feat(my-activity): implement vote details drawer with selectable cards #226
Conversation
- Add vote-details-drawer component for viewing and submitting votes - Add selectable-card shared component for vote options - Support single-choice and multiple-choice questions - Add confirmation and success modals for vote submission - Add VoteDetails, PollQuestion, UserChoice interfaces to shared package - Add mock poll questions and descriptions for development - Wire up votes table to open drawer on action button click LFXV2-975 Signed-off-by: Asitha de Silva <[email protected]>
github-actions
bot
requested review from
andrest50,
dealako and
mauriciozanettisalomao
WalkthroughAdds a vote details drawer feature with UI, form-driven voting flow, selectable card control, poll interfaces/constants, and integration into the votes table; also includes several related UI and type refactors across activity, surveys, mailing-list, and shared interfaces. Changes
Sequence DiagramsequenceDiagram
participant User
participant VotesTable
participant VoteDetailsDrawer
participant FormState as Form/State
participant Modal as Confirmation Modal
participant Dialog as Success Dialog
User->>VotesTable: Click "Vote" action
VotesTable->>VotesTable: onVoteClick(vote) -> set selectedVote, drawerVisible = true
VotesTable->>VoteDetailsDrawer: render with VoteDetails input
User->>VoteDetailsDrawer: Select option(s)
VoteDetailsDrawer->>FormState: update voteForm / checkboxForms
VoteDetailsDrawer->>VoteDetailsDrawer: compute allQuestionsAnswered
User->>VoteDetailsDrawer: Click "Submit Vote"
VoteDetailsDrawer->>Modal: open confirmation modal
User->>Modal: Confirm
Modal->>VoteDetailsDrawer: confirm result
VoteDetailsDrawer->>VotesTable: emit voteSubmitted { pollId, answers }
VotesTable->>VotesTable: onVoteSubmitted -> apply local modifications
VoteDetailsDrawer->>Dialog: open success dialog
User->>Dialog: Click "Done"
Dialog->>VoteDetailsDrawer: close drawer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
@apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html:
- Line 65: In the template for VoteDetailsDrawer (the div with data-testid
"vote-details-drawer-content" in vote-details-drawer.component.html) fix the
Tailwind typo by replacing the invalid class "p6" with the correct "p-6" so the
padding utility applies correctly.
In
@apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts:
- Around line 62-75: The computed allQuestionsAnswered currently reads
non-reactive form values (this.voteForm.get(...).value) so it only recomputes
when this.vote() changes; make it reactive by introducing a signal (e.g.,
formValuesChanged) that you update on form value changes (subscribe to
this.voteForm.valueChanges inside the constructor/effect after buildForm) and
reference that signal at the top of allQuestionsAnswered so the computed re-runs
when the user updates controls; keep existing logic using vote(), voteForm, and
checkboxForms but ensure formValuesChanged() is touched inside the computed to
trigger recomputation.
- Line 32: Remove the unnecessary CUSTOM_ELEMENTS_SCHEMA from the @Component
decorator in VoteDetailsDrawerComponent (it’s redundant because the lfx-*
components are already declared/imported), and modify the template to wrap the
PrimeNG <p-drawer> in the LFX wrapper component (e.g., replace or nest p-drawer
inside the corresponding <lfx-drawer> wrapper component) ensuring the wrapper
component is present in the component's imports array; update any template
inputs/events to pass through the LFX wrapper so the original p-drawer behavior
is preserved.
In
@apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts:
- Around line 127-131: getVotesSource currently prefers internalVotes over the
votes input which can ignore parent updates after a submission; fix by syncing
internalVotes from the votes() input when the input changes—add an effect in the
component constructor that watches this.votes() and resets or merges into
this.internalVotes (or reinitialize internalVotes) when the incoming array
changes, or choose to always read from internalVotes but initialize it from
votes() so getVotesSource, internalVotes, and votes() remain consistent with
parent updates.
In
@apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.html:
- Around line 17-20: The selectable-card components use role="option" which
requires their parent container to have role="listbox" for proper ARIA
accessibility. Locate the parent div wrapping selectable-card components in
vote-details-drawer.component.html around lines 151 and 164, and add the
attribute role="listbox" to these container divs to establish the correct
accessibility hierarchy.
🧹 Nitpick comments (8)
apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.scss (1)
4-17: Consider alternatives to::ng-deepfor future-proofing.
::ng-deepis deprecated and may be removed in future Angular versions. While it still works, consider these alternatives:
- Use PrimeNG's
styleClassinput to apply classes directly- Define global styles in a shared stylesheet (if these styles are reused)
- Use CSS custom properties for theming
For now, this approach is functional and acceptable for component-scoped overrides of third-party component styles.
packages/shared/src/interfaces/poll.interface.ts (1)
54-55: Consider using a string literal type or enum fortypefield.The
typefield is typed asstringbut appears to have a known set of values ('single_choice','multiple_choice'). A more specific type would provide better type safety and IDE autocompletion.♻️ Suggested improvement
// Option 1: String literal type type QuestionType = 'single_choice' | 'multiple_choice'; // Then in PollQuestion: type: QuestionType; // Option 2: Add to existing enums in poll.enum.ts export enum QuestionType { SINGLE_CHOICE = 'single_choice', MULTIPLE_CHOICE = 'multiple_choice', }apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html (2)
17-22: Empty div used as a spacer.Line 18 has an empty
<div></div>used for spacing in a flexbox withjustify-between. Consider usingml-autoon the button instead for cleaner markup.♻️ Suggested refactor
- <div class="flex items-center justify-between"> - <div></div> - <button type="button" (click)="onClose()" class="text-slate-400 hover:text-slate-600 transition-colors p-1" data-testid="vote-details-drawer-close"> + <div class="flex items-center"> + <button type="button" (click)="onClose()" class="ml-auto text-slate-400 hover:text-slate-600 transition-colors p-1" data-testid="vote-details-drawer-close">
216-239: Consider extracting modals into reusable components.The confirmation and success modals are implemented inline with manual backdrop handling and z-index management. Consider extracting these into reusable modal components (or using an existing LFX modal wrapper) to ensure consistent behavior across the app and reduce template complexity.
apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts (1)
133-167: Mock data implementation should be clearly marked for replacement.The
getMockVoteDetailsmethod hardcodes mock data including descriptions, creator, and discussion links. While acceptable for development, add a TODO or consider injecting a service that can be swapped for real API calls.♻️ Suggested improvement
+ // TODO: Replace with real API call to fetch vote details + // This mock implementation is for development purposes only private getMockVoteDetails(vote: UserVote): VoteDetails {apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.ts (1)
96-99: Remove redundant identity map.The
.map((value) => value)on line 98 is a no-op and can be removed.♻️ Proposed fix
return formControl.valueChanges.pipe( - startWith(formControl.value), - map((value) => value) + startWith(formControl.value) );apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts (2)
45-48: Consider usingprotectedfor template-bound properties.
voteFormandcheckboxFormsare only used in the template and should be marked asprotectedrather thanpublicto limit their API surface. Based on Angular 19+ zoneless patterns with signals, this maintains encapsulation.♻️ Proposed fix
// Form for vote responses - dynamically built based on questions - public voteForm = new FormGroup<Record<string, FormControl<string | null>>>({}); + protected voteForm = new FormGroup<Record<string, FormControl<string | null>>>({}); // Checkbox forms - one FormGroup per multi-choice question - public checkboxForms: Record<string, FormGroup<Record<string, FormControl<boolean>>>> = {}; + protected checkboxForms: Record<string, FormGroup<Record<string, FormControl<boolean>>>> = {};
51-52: Consider using signals for modal state consistency.
showConfirmModalandshowSuccessModalare plain boolean properties while the rest of the component uses signals. For consistency with the zoneless change detection pattern mentioned in guidelines, consider using signals.♻️ Suggested refactor
// Modal states - protected showConfirmModal = false; - protected showSuccessModal = false; + protected readonly showConfirmModal = signal(false); + protected readonly showSuccessModal = signal(false);Then update usages to use
.set()and()accessor patterns.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.htmlapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.scssapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.htmlapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.tsapps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.htmlapps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.tspackages/shared/src/constants/poll.constants.tspackages/shared/src/interfaces/poll.interface.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript interfaces instead of union types for better maintainability
Files:
packages/shared/src/constants/poll.constants.tspackages/shared/src/interfaces/poll.interface.tsapps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All shared types, interfaces, and constants must be centralized in @lfx-one/shared package
Files:
packages/shared/src/constants/poll.constants.tspackages/shared/src/interfaces/poll.interface.ts
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}: License headers are required on all source files
Always prepend 'Generated with Claude Code (https://claude.ai/code)' if Claude Code assisted with the code
Files:
packages/shared/src/constants/poll.constants.tsapps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.htmlpackages/shared/src/interfaces/poll.interface.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.scssapps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.htmlapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.htmlapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Do not nest ternary expressions
Always use async/await for promises in TypeScript/JavaScript
Use camelCase for variable and function names
Files:
packages/shared/src/constants/poll.constants.tspackages/shared/src/interfaces/poll.interface.tsapps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
apps/lfx-one/src/**/*.component.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.html: Use data-testid naming convention: [section]-[component]-[element] for hierarchical structure
Use flex + flex-col + gap-* instead of space-y-* for spacing
Files:
apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.htmlapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.htmlapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.html
apps/lfx-one/**/*.{ts,tsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier configuration with Tailwind integration in lfx-one app
Files:
apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.htmlapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.scssapps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.htmlapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.htmlapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
apps/lfx-one/src/**/*.{html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind CSS with PrimeUI plugin and LFX colors for styling
Files:
apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.htmlapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.scssapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.htmlapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.html
apps/lfx-one/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.{ts,tsx}: Use zoneless change detection with signals in Angular 19
Always use direct imports for standalone components - no barrel exports
Use ESLint and Prettier for code quality, following Angular-specific ESLint rules
Files:
apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
apps/lfx-one/src/**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.ts: All PrimeNG components must be wrapped in LFX components for UI library independence
Add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
🧠 Learnings (1)
📚 Learning: 2025-12-10T23:20:29.281Z
Learnt from: asithade
Repo: linuxfoundation/lfx-v2-ui PR: 206
File: apps/lfx-one/src/app/modules/meetings/components/agenda-template-selector/agenda-template-selector.component.ts:11-15
Timestamp: 2025-12-10T23:20:29.281Z
Learning: For Angular 19+ projects, standalone defaults to true for components, directives, and pipes when not explicitly declared. Therefore, components with an imports array in the Component decorator do not require an explicit standalone: true declaration to be standalone. In reviews, verify that a component without an explicit standalone flag and with an imports array will behave as a standalone component according to Angular 19+ defaults. If you need to enforce explicit standalone behavior for clarity or consistency, you can still add standalone: true, but it is not required by default in Angular 19+. Ensure this guidance is considered when reviewing any component.ts files across the codebase.
Applied to files:
apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
🧬 Code graph analysis (2)
packages/shared/src/constants/poll.constants.ts (1)
packages/shared/src/interfaces/poll.interface.ts (1)
PollQuestion(49-60)
apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts (3)
packages/shared/src/interfaces/poll.interface.ts (3)
UserVote(11-28)VoteDetails(93-114)PollAnswer(66-79)packages/shared/src/constants/poll.constants.ts (2)
MOCK_POLL_DESCRIPTIONS(167-173)MOCK_POLL_QUESTIONS(73-161)packages/shared/src/utils/poll.utils.ts (1)
CombinedVoteStatus(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (16)
apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.html (2)
121-123: LGTM!The drawer integration follows Angular best practices:
- Two-way binding
[(visible)]for drawer visibility state- Signal accessor
selectedVote()for reactive data flow- Event binding for submission handling
The placement outside
<lfx-card>is appropriate for a modal/drawer overlay component.
103-103: LGTM!The click handler correctly passes the vote object to
onVoteClick()to populate the drawer with vote details.packages/shared/src/constants/poll.constants.ts (2)
73-161: LGTM! Mock data structure is well-organized.The mock data correctly implements the
PollQuestioninterface with properUserChoiceobjects. Good use ofMapfor efficient lookups by poll ID, and the JSDoc comments clearly indicate this is temporary mock data.Consider adding a TODO comment or tracking issue for removing/replacing this mock data when the API is ready.
167-173: LGTM!Descriptions are properly keyed to match the poll IDs in
MOCK_POLL_QUESTIONS, ensuring data consistency.apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.html (1)
4-22: LGTM! Well-structured accessible component.The implementation correctly handles:
- Visual states (selected, disabled, hover) via
ngClass- Keyboard accessibility with tabindex management
- ARIA attributes for screen reader support
- Event binding for click and keyboard interactions
packages/shared/src/interfaces/poll.interface.ts (2)
66-79: LGTM!The
PollAnswerinterface clearly separates regular selections (user_choice) from ranked choices (ranked_user_choice), supporting both plurality and ranked-choice voting scenarios. The required arrays ensure consistent structure even when one is empty.
93-114: LGTM!The
VoteDetailsinterface is well-designed:
- Extends
UserVoteto inherit base poll properties- Required
poll_questionsensures questions are always available for rendering- Optional
poll_answerscorrectly models the "not yet voted" state- Clear JSDoc documentation for each property
apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html (2)
130-177: Well-structured question rendering with proper accessibility.The questions section properly handles both single and multiple choice types, uses
trackfor performance, and includes appropriate data-testid attributes for testing. The conditional rendering for empty choices is a good defensive pattern.
4-11: No action required. The project architecture explicitly supports direct PrimeNG component integration as documented in the README. Direct usage ofp-draweris consistent with the project's stated approach and is used throughout the codebase, including in shared components like data-copilot.apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts (2)
100-104: Good implementation of drawer opening flow.The
onVoteClickmethod correctly constructs vote details and updates the drawer state signals in the proper order.
110-125: Vote submission handler updates state correctly.The
onVoteSubmittedmethod properly updates the vote status toRESPONDEDand sets the creation time. The immutable update pattern usingmapis correct.apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.ts (3)
86-104: Complex RxJS chain for control value tracking.The
initControlValuemethod usestoObservable→switchMap→toSignalto track the form control's value. This approach handles dynamic form/control changes well, but consider documenting why this pattern is needed versus a simpler approach.
59-76: Well-implemented click handler with proper guard clauses.The
onCardClickmethod correctly checks for disabled state, handles both toggle and value modes, and has proper null checks for the form control.
78-84: Good keyboard accessibility implementation.The
onKeydownhandler properly supports Enter and Space keys for activation, and correctly prevents default behavior to avoid page scrolling on Space.apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts (2)
145-153: Effect-based form building is appropriate.Using
effect()to rebuild the form when the vote input changes is the correct Angular signals pattern for side effects that need to react to signal changes.
223-270: Well-structured form building with pre-population support.The
buildFormmethod correctly handles both single and multiple choice questions, pre-populates values from existing answers, and properly disables forms in read-only mode.
...rc/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html
Outdated
Show resolved
Hide resolved
.../src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts
Outdated
Show resolved
Hide resolved
.../src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.html
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive vote details drawer with selectable card components, enabling users to view vote information and submit their responses through an interactive UI.
Changes:
- Added vote details drawer component with vote submission flow including confirmation and success modals
- Implemented reusable selectable card component supporting both single-choice (radio) and multiple-choice (checkbox) selection modes
- Extended shared interfaces to support detailed vote data including questions, choices, and answers
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/poll.interface.ts | Added VoteDetails, PollQuestion, PollAnswer, UserChoice interfaces for vote drawer functionality |
| packages/shared/src/constants/poll.constants.ts | Added mock poll questions and descriptions for testing the vote drawer |
| apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.ts | New reusable component for selectable cards with form integration |
| apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.html | Template for selectable card with accessibility attributes |
| apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts | Wired up vote click handler, drawer state management, and vote submission logic |
| apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.html | Added vote details drawer component reference |
| apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts | Core drawer logic with form building, validation, and submission handling |
| apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html | Drawer UI with question display, selection options, and modal dialogs |
| apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.scss | Drawer styling with custom PrimeNG drawer overrides |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...rc/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.ts
Outdated
Show resolved
Hide resolved
...rc/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html
Outdated
Show resolved
Hide resolved
...rc/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html
Show resolved
Hide resolved
...rc/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html
Outdated
Show resolved
Hide resolved
.../src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts
Outdated
Show resolved
Hide resolved
...rc/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html
Outdated
Show resolved
Hide resolved
.../src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts
Outdated
Show resolved
Hide resolved
...rc/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html
Show resolved
Hide resolved
- Add VoteSubmittedDialogComponent loaded via DialogService - Fix votes-table to use local modifications pattern for proper input sync - Use PollStatus enum in template instead of string literal - Handle mixed question types in selectionTypeText - Show vote choice text only for single-question polls - Update Your Vote section for multi-question polls LFXV2-975 Signed-off-by: Asitha de Silva <[email protected]>
LFXV2-975 Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts:
- Around line 103-112: onVoteSubmitted currently only saves vote_status and
vote_creation_time into the localModifications map, so the submitted answers are
not preserved; update the handler in votes-table.component (onVoteSubmitted) to
also store result.answers as poll_answers when setting modifications for
result.pollId, and update the localModifications map type (or UserVote
interface) to allow poll_answers: PollAnswer[] (e.g., Map<string,
Partial<UserVote & { poll_answers?: PollAnswer[] }>> or extend UserVote) so the
drawer will render the actual submitted answers before parent sync.
🧹 Nitpick comments (6)
apps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.ts (1)
42-44: Consider simplifying to inline computed signals.The private initializer methods add indirection without providing additional value. The standard Angular pattern for simple computed signals is more concise and equally readable:
protected readonly isVotesTab = computed(() => this.activeTab() === 'votes'); protected readonly isSurveysTab = computed(() => this.activeTab() === 'surveys');The
computed()function already returnsSignal<T>, so explicit typing is redundant, and the initializer methods don't add testability or reusability benefits for these one-liners.♻️ Suggested simplification
- // === Computed Signals === - protected readonly isVotesTab: Signal<boolean> = this.initIsVotesTab(); - protected readonly isSurveysTab: Signal<boolean> = this.initIsSurveysTab(); + // === Computed Signals === + protected readonly isVotesTab = computed(() => this.activeTab() === 'votes'); + protected readonly isSurveysTab = computed(() => this.activeTab() === 'surveys');And remove the initializer methods (lines 51-58).
Also applies to: 51-58
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html (1)
30-30: Consider usingoverflow-autoinstead ofoverflow-scroll.
overflow-scrollalways renders scrollbars regardless of content height (on some browsers/OS), whereasoverflow-autoonly shows scrollbars when content overflows. This would provide a cleaner appearance when the content fits withinmax-h-[30rem].Suggested change
- <div lfxScrollShadow class="flex flex-col gap-1 overflow-scroll max-h-[30rem] py-1 -mt-1" data-testid="dashboard-my-meetings-list"> + <div lfxScrollShadow class="flex flex-col gap-1 overflow-auto max-h-[30rem] py-1 -mt-1" data-testid="dashboard-my-meetings-list">apps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.html (1)
6-10: Consider extracting inline SVG to an icon component.The inline SVG checkmark works correctly, but for consistency with the rest of the codebase, consider using a FontAwesome icon class (e.g.,
fa-light fa-check) or an existing icon component if available.💡 Optional: Use FontAwesome icon
- <div class="w-12 h-12 bg-green-100 rounded-full flex items-center justify-center mx-auto"> - <svg class="w-6 h-6 text-green-600" fill="none" viewBox="0 0 24 24" stroke="currentColor"> - <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M5 13l4 4L19 7" /> - </svg> - </div> + <div class="w-12 h-12 bg-green-100 rounded-full flex items-center justify-center mx-auto"> + <i class="fa-light fa-check text-xl text-green-600"></i> + </div>apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts (1)
239-258: Consider improving the "0 days left" edge case.When
diffDaysis 0 (poll closes today), the text displays "0 days left" which may confuse users. Consider using "Closes today" for better clarity.💡 Suggested improvement
if (diffDays < 0) { return { text: 'Closed', color: 'text-slate-500' }; + } else if (diffDays === 0) { + return { text: 'Closes today', color: 'text-red-600' }; } else if (diffDays <= 3) { return { text: `${diffDays} day${diffDays !== 1 ? 's' : ''} left`, color: 'text-red-600' };apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts (2)
73-74: Consider clearing localModifications when parent input updates.The
localModificationsmap accumulates optimistic updates but is never cleared. If the parent component eventually syncs the actual vote data (e.g., after an API call), the stale modifications will persist and continue to be merged unnecessarily. Consider implementing a mechanism to clear modifications when the parent'svotesinput updates with the synced data.♻️ Suggested approach using effect to clear stale modifications
import { effect } from '@angular/core'; // In constructor or field initializer: constructor() { // Clear local modifications when input votes change (parent synced) effect(() => { const inputVotes = this.votes(); const modifications = this.localModifications(); if (modifications.size > 0) { // Check if any modified vote now has the expected status in input const updatedModifications = new Map(modifications); let hasChanges = false; for (const [pollId, mod] of modifications) { const inputVote = inputVotes.find(v => v.poll_id === pollId); if (inputVote && inputVote.vote_status === mod.vote_status) { updatedModifications.delete(pollId); hasChanges = true; } } if (hasChanges) { this.localModifications.set(updatedModifications); } } }); }
115-125: Non-null assertion on form control access.Using
this.searchForm.get('search')!assumes the control always exists. While safe here since the form is defined in the same class, consider using a direct reference for better type safety.♻️ Alternative using direct control reference
public searchForm = new FormGroup({ search: new FormControl<string>(''), status: new FormControl<CombinedVoteStatus | null>(null), committee: new FormControl<string | null>(null), }); + +// Direct reference for type safety +private readonly searchControl = this.searchForm.controls.search; private initSearchTerm(): Signal<string> { return toSignal( - this.searchForm.get('search')!.valueChanges.pipe( + this.searchControl.valueChanges.pipe( startWith(''), debounceTime(300), distinctUntilChanged(), map((value) => value ?? '') ), { initialValue: '' } ); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
CLAUDE.mdapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.tsapps/lfx-one/src/app/modules/my-activity/components/activity-top-bar/activity-top-bar.component.tsapps/lfx-one/src/app/modules/my-activity/components/surveys-table/surveys-table.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.htmlapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.htmlapps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.tsapps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.tspackages/shared/src/interfaces/components.interface.tspackages/shared/src/interfaces/mailing-list.interface.ts
💤 Files with no reviewable changes (1)
- packages/shared/src/interfaces/mailing-list.interface.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.html
- apps/lfx-one/src/app/shared/components/selectable-card/selectable-card.component.ts
🧰 Additional context used
📓 Path-based instructions (9)
apps/lfx-one/src/**/*.component.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.html: Use data-testid naming convention: [section]-[component]-[element] for hierarchical structure
Use flex + flex-col + gap-* instead of space-y-* for spacing
Files:
apps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}: License headers are required on all source files
Always prepend 'Generated with Claude Code (https://claude.ai/code)' if Claude Code assisted with the code
Files:
apps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.htmlpackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.tsapps/lfx-one/src/app/modules/my-activity/components/activity-top-bar/activity-top-bar.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/modules/my-activity/components/surveys-table/surveys-table.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
apps/lfx-one/**/*.{ts,tsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier configuration with Tailwind integration in lfx-one app
Files:
apps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.tsapps/lfx-one/src/app/modules/my-activity/components/activity-top-bar/activity-top-bar.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/modules/my-activity/components/surveys-table/surveys-table.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
apps/lfx-one/src/**/*.{html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind CSS with PrimeUI plugin and LFX colors for styling
Files:
apps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript interfaces instead of union types for better maintainability
Files:
packages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.tsapps/lfx-one/src/app/modules/my-activity/components/activity-top-bar/activity-top-bar.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/modules/my-activity/components/surveys-table/surveys-table.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All shared types, interfaces, and constants must be centralized in @lfx-one/shared package
Files:
packages/shared/src/interfaces/components.interface.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Do not nest ternary expressions
Always use async/await for promises in TypeScript/JavaScript
Use camelCase for variable and function names
Files:
packages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.tsapps/lfx-one/src/app/modules/my-activity/components/activity-top-bar/activity-top-bar.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/modules/my-activity/components/surveys-table/surveys-table.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
apps/lfx-one/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.{ts,tsx}: Use zoneless change detection with signals in Angular 19
Always use direct imports for standalone components - no barrel exports
Use ESLint and Prettier for code quality, following Angular-specific ESLint rules
Files:
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.tsapps/lfx-one/src/app/modules/my-activity/components/activity-top-bar/activity-top-bar.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/modules/my-activity/components/surveys-table/surveys-table.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
apps/lfx-one/src/**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.ts: All PrimeNG components must be wrapped in LFX components for UI library independence
Add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.tsapps/lfx-one/src/app/modules/my-activity/components/activity-top-bar/activity-top-bar.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/modules/my-activity/components/surveys-table/surveys-table.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
🧠 Learnings (1)
📚 Learning: 2025-12-10T23:20:29.281Z
Learnt from: asithade
Repo: linuxfoundation/lfx-v2-ui PR: 206
File: apps/lfx-one/src/app/modules/meetings/components/agenda-template-selector/agenda-template-selector.component.ts:11-15
Timestamp: 2025-12-10T23:20:29.281Z
Learning: For Angular 19+ projects, standalone defaults to true for components, directives, and pipes when not explicitly declared. Therefore, components with an imports array in the Component decorator do not require an explicit standalone: true declaration to be standalone. In reviews, verify that a component without an explicit standalone flag and with an imports array will behave as a standalone component according to Angular 19+ defaults. If you need to enforce explicit standalone behavior for clarity or consistency, you can still add standalone: true, but it is not required by default in Angular 19+. Ensure this guidance is considered when reviewing any component.ts files across the codebase.
Applied to files:
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.tsapps/lfx-one/src/app/modules/my-activity/components/activity-top-bar/activity-top-bar.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/modules/my-activity/components/surveys-table/surveys-table.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.tsapps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.tsapps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.tsapps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts
🧬 Code graph analysis (8)
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.ts (1)
packages/shared/src/interfaces/committee.interface.ts (1)
CommitteeReference(11-18)
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts (1)
packages/shared/src/interfaces/committee.interface.ts (1)
CommitteeReference(11-18)
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts (2)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (1)
Component(25-639)apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (1)
Component(37-1067)
apps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.ts (1)
apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts (1)
Component(23-329)
apps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.ts (2)
packages/shared/src/interfaces/committee.interface.ts (1)
CommitteeReference(11-18)packages/shared/src/interfaces/components.interface.ts (1)
CommitteeSelectorOption(574-581)
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.ts (1)
packages/shared/src/interfaces/committee.interface.ts (1)
CommitteeReference(11-18)
apps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.ts (4)
packages/shared/src/constants/my-activity.constants.ts (1)
MY_ACTIVITY_TAB_OPTIONS(24-27)packages/shared/src/interfaces/my-activity.interface.ts (1)
MyActivityTab(8-8)packages/shared/src/interfaces/poll.interface.ts (1)
UserVote(11-28)packages/shared/src/interfaces/survey.interface.ts (1)
UserSurvey(11-26)
apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts (4)
packages/shared/src/interfaces/poll.interface.ts (3)
UserVote(11-28)VoteDetails(93-114)PollAnswer(66-79)packages/shared/src/utils/poll.utils.ts (2)
CombinedVoteStatus(11-11)getCombinedVoteStatus(22-32)packages/shared/src/constants/my-activity.constants.ts (1)
MY_ACTIVITY_FILTER_LABELS(34-37)packages/shared/src/constants/poll.constants.ts (3)
COMBINED_VOTE_STATUS_LABELS(53-57)MOCK_POLL_DESCRIPTIONS(167-173)MOCK_POLL_QUESTIONS(73-161)
🔇 Additional comments (29)
apps/lfx-one/src/app/modules/my-activity/components/surveys-table/surveys-table.component.ts (5)
48-66: LGTM! Clean signal-based reactive architecture.The refactoring to use Angular signals (
input.required,computed,toSignal) follows modern Angular 19+ patterns. The separation of form controls from filter signals with explicitonChangehandlers provides clear control flow.
77-88: Well-structured signal-based search debouncing.The RxJS pipeline correctly handles the form-to-signal conversion with appropriate debouncing and null-safety.
90-115: LGTM!The computed signal correctly derives status options from the input surveys with appropriate counting and ordering logic.
117-143: LGTM!Committee options computation follows the same clean pattern as status options, with appropriate fallback to
uidwhennameis unavailable.
145-169: LGTM!The filtering logic correctly chains multiple filter criteria with case-insensitive search and proper signal dependencies. The defensive null handling on line 149 is slightly redundant given the signal's
initialValue, but doesn't cause harm.apps/lfx-one/src/app/modules/my-activity/my-activity-dashboard/my-activity-dashboard.component.ts (3)
4-4: LGTM!The
Signaltype import is correctly added to support the explicit typing ofisVotesTabandisSurveysTabproperties.
32-35: LGTM!Clean inline initialization of
tabFormwith proper typing. Removing constructor-based initialization simplifies the component.
28-29: Section organization looks clean.The section comments provide good visual organization for the component's members.
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts (1)
4-4: LGTM! ScrollShadowDirective integration follows established patterns.The implementation is consistent with
recent-progress.component.ts(usingprotectedaccess modifier). The directive import and@ViewChildwiring are correctly set up.Also applies to: 11-11, 19-19, 24-25
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html (2)
69-83: Shadow fade implementation is well-structured.Good use of
aria-hidden="true"andpointer-events-nonefor accessibility. The defensivescrollShadowDirective &&check handles potential timing issues with@ViewChildinitialization.Minor note: The hardcoded color
#f9fafb(gray-50) in the gradients could be extracted to a CSS variable if theme consistency becomes important, but this is fine for now.
31-66: Content structure is clean and follows guidelines.
- Proper use of
@forwithtrackfor efficient rendering- Conditional section rendering avoids empty containers
data-testidattributes follow the[section]-[component]-[element]naming convention- Uses
flex+gap-*for spacing per coding guidelinesCLAUDE.md (1)
88-197: Well-structured documentation for Angular signal patterns.The component organization pattern documentation is comprehensive and provides clear guidance on:
- WritableSignals vs Model Signals usage
- Private initializer functions for complex computed signals
- Consistent component structure ordering
- Interface centralization in the shared package
This aligns well with the Angular 19 patterns used throughout the codebase and will help maintain consistency.
packages/shared/src/interfaces/components.interface.ts (1)
582-603: Well-designed shared interfaces with proper generics.The new
RelativeDateInfoandTabOption<T>interfaces are:
- Properly documented with JSDoc comments consistent with the file's style
- Using TypeScript generics appropriately (
TabOption<T = string>)- Centralized in the shared package as per coding guidelines
This enables type-safe usage across components like
ActivityTopBarComponent.apps/lfx-one/src/app/modules/my-activity/components/activity-top-bar/activity-top-bar.component.ts (2)
7-7: Good refactor to use shared TabOption interface.Importing
TabOptionfrom@lfx-one/shared/interfacescentralizes the interface definition as per coding guidelines.
14-25: Clean component structure with proper organization.The component follows good practices:
readonlymodifiers on input signals- Generic typing with
TabOption<MyActivityTab>[]for type safety- Clear section comments for code organization
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts (2)
12-12: Type alignment with CommitteeReference.Good refactor to use the authoritative
CommitteeReferencetype instead of the deprecatedMailingListCommitteealias, maintaining consistency across mailing-list components.
226-226: Consistent FormControl typing.The
committeesFormControl is correctly typed asCommitteeReference[]to match the imported type.apps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.html (1)
1-25: Well-structured success dialog template.The template follows coding guidelines:
- Uses
flex + flex-col + gap-*instead ofspace-y-*- Includes
data-testidattribute for testing- Proper license header
- Uses wrapped
lfx-buttoncomponentapps/lfx-one/src/app/modules/my-activity/components/vote-submitted-dialog/vote-submitted-dialog.component.ts (1)
1-20: LGTM!Clean, minimal dialog component implementation. License header is present, and the component correctly leverages Angular 19+ standalone defaults (no explicit
standalone: trueneeded with theimportsarray). The injectedDynamicDialogConfigprovides access to thepollNamedata passed fromshowSuccessDialog()in the drawer component.apps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.ts (1)
21-21: LGTM!Type migration from
MailingListCommitteetoCommitteeReferenceis consistent with the broader PR refactoring across mailing list components. TheCommitteeReferenceinterface provides the requireduidandnameproperties used in the computed signal at line 103.Also applies to: 72-72
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.ts (1)
11-11: LGTM!Consistent type migration to
CommitteeReference. The internalMap<string, CommitteeReference>correctly stores committee data with the expecteduidandnameproperties used in the filtering and options generation logic.Also applies to: 177-177
apps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.ts (1)
8-8: LGTM!Comprehensive type migration from
MailingListCommitteetoCommitteeReference. The bidirectional mapping betweenCommitteeReference(usinguid) andCommitteeSelectorOption(usingid) is correctly implemented in bothmapFormValueToSelectedItemsandupdateFormControl. Theallowed_voting_statusespopulation usingVOTING_STATUSESin multi-select mode aligns with theCommitteeReferenceinterface definition.Also applies to: 169-170, 182-196, 220-224
apps/lfx-one/src/app/modules/my-activity/components/vote-details-drawer/vote-details-drawer.component.ts (2)
1-40: Well-structured vote details drawer component.The component follows Angular 19+ patterns with signals, computed properties, and reactive forms. The separation of concerns is clear with distinct sections for injections, inputs/outputs, forms, computed signals, and helper methods. The form handling correctly supports both single-choice and multiple-choice questions with pre-population for existing votes.
Also applies to: 41-82
103-143: This review comment is based on incorrect assumptions about the system architecture.The code does not make actual API calls. The my-activity-dashboard component uses mock data only (lines 61–149 in my-activity-dashboard.component.ts), with no HTTP service or API integration. The
voteSubmittedevent emission in vote-details-drawer triggers the success dialog immediately (line 142), and the parent component (votes-table) only updates local state via thelocalModificationssignal when the event is received. There is no parent API call to handle, no API error to catch, and no reason to defer the success dialog display.Likely an incorrect or invalid review comment.
apps/lfx-one/src/app/modules/my-activity/components/votes-table/votes-table.component.ts (5)
1-34: LGTM - Imports and license header are properly structured.The imports are well-organized and appropriate for the component's functionality. The license header follows the required format.
35-66: LGTM - Component structure follows Angular 19 patterns.The component correctly uses Angular 19's standalone defaults (implicit with
importsarray), signal-based input withinput.required<UserVote[]>(), and properly typed form controls. Based on learnings, the explicitstandalone: trueis not required in Angular 19+.
207-230: LGTM - Merged votes implementation is well-structured.The optimistic update pattern with
initMergedVotesis correctly implemented:
- Early return optimization when no modifications exist
- Proper spread operator usage to apply partial updates
- Clean abstraction via
getVotesSource()for future flexibility
232-263: Mock data only handles single-question poll results.The
generic_choice_votesis only populated for the first question (poll_questions[0].choices). For multi-question polls likepoll-005(which has 2 questions per the mock data), the results for additional questions won't be generated. This is acceptable for development but should be noted for future real implementation.Additionally, the user's answer is hardcoded to always be the first choice (
q.choices[0]), which may make testing different scenarios difficult.Is this mock data scope sufficient for the current development phase, or should it be expanded to handle multi-question scenarios?
271-286: LGTM - Sorting implementation is correct and immutable.The sorting logic properly:
- Creates a new array to avoid mutating the original
- Prioritizes actionable votes (open → submitted → closed)
- Uses end_time as secondary sort for equal statuses
Summary
Changes
Vote Details Drawer: Slide-over panel showing full vote information
Selectable Card Component: Reusable shared component
Shared Interfaces: Added VoteDetails, PollQuestion, UserChoice, PollAnswer
JIRA
LFXV2-975
🤖 Generated with Claude Code