feat(dashboard): enhance meeting card UI and add loading states #141
Conversation
LFXV2-702 - Add colored left border to meeting cards matching meeting type - Implement join URL pre-fetching for today's meetings - Move feature icons from header to under title with time - Update attachment icon styling with blue theme - Add skeleton loading to My Meetings component - Fix button component ngClass and type attribute support Signed-off-by: Asitha de Silva <[email protected]>
github-actions
bot
requested review from
andrest50,
dealako and
mauriciozanettisalomao
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a loading signal and skeleton UI for MyMeetings, removes event-based "see meeting" navigation in favor of computed join URLs on meeting cards, introduces meeting-type styling constants and a MeetingWithOccurrence interface, and extends the shared button template with type/style bindings. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant MyMeetings
participant MeetingCard as DashboardMeetingCard
participant UserService
participant JoinURL as joinUrl Signal
participant Router
rect `#E8F0FF`
Note over MyMeetings: Data fetch with loading signal
MyMeetings->>MyMeetings: call meetingService.getMeetings()
MyMeetings->>MyMeetings: set loading = true
MyMeetings-->>MyMeetings: toSignal(..., initialValue: [])
MyMeetings->>MyMeetings: finalize -> set loading = false
end
rect `#F0FFF0`
Note over MeetingCard,JoinURL: joinUrl computation
MeetingCard->>UserService: request authenticated user info
MeetingCard->>JoinURL: combine meeting + user + auth -> compute joinUrl
JoinURL-->>MeetingCard: joinUrl (string|null)
end
rect `#FFF8E6`
Note over User,MeetingCard: Interaction
User->>MeetingCard: Click action
alt joinUrl exists
MeetingCard->>User: navigate via href (external join link)
else joinUrl null
MeetingCard->>Router: routerLink to internal meeting details
Router-->>User: show meeting details page
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (6)apps/lfx-one/src/**/*.html📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
packages/shared/src/constants/**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
packages/shared/src/interfaces/**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (4)📚 Learning: 2025-10-21T21:19:13.599ZApplied to files:
📚 Learning: 2025-09-16T03:32:46.518ZApplied to files:
📚 Learning: 2025-09-16T03:32:46.518ZApplied to files:
📚 Learning: 2025-09-16T03:32:46.518ZApplied to files:
🧬 Code graph analysis (1)apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.ts (4)
🔇 Additional comments (5)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the dashboard meeting card component by adding direct meeting join functionality, visual improvements, and loading states. Key changes include fetching join URLs directly for today's meetings, redesigning the card layout with colored borders and improved button actions, and adding loading skeleton UI.
- Integrated direct join URL fetching for authenticated users on today's meetings
- Redesigned meeting card UI with meeting-type colored borders and reorganized layout
- Added loading state skeleton UI to the my-meetings dashboard component
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| dashboard-meeting-card.component.ts | Added join URL fetching logic, removed output event, added color and styling computed signals |
| dashboard-meeting-card.component.html | Redesigned layout with colored borders, direct join buttons, and improved visual hierarchy |
| button.component.html | Added type attribute and ngClass binding support for anchor elements |
| my-meetings.component.ts | Added loading signal and removed unused handleSeeMeeting method |
| my-meetings.component.html | Added skeleton loading state UI |
Comments suppressed due to low confidence (3)
apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html:134
- This attribute is duplicated later.
This attribute is duplicated later.
type="button"
apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html:142
- This attribute is duplicated later.
rel="noopener noreferrer"
apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html:125
- This attribute is duplicated later.
rel="noopener noreferrer"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...x-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html
Show resolved
Hide resolved
...x-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html
Show resolved
Hide resolved
...x-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html
Show resolved
Hide resolved
...lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.ts
Outdated
Show resolved
Hide resolved
![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
🧹 Nitpick comments (3)
apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.ts (1)
218-226: Replace deprecatedunescape()with modern encoding approach.The method uses the deprecated
unescape()function in the encoding chain. While the comment indicates this matches the meeting-join component logic, consider using a modern approach for Unicode-safe base64 encoding.Replace the deprecated
unescape()with a TextEncoder-based approach:private buildJoinUrlWithParams(joinUrl: string, user: User): string { const displayName = user.name || user.email; - const encodedName = btoa(unescape(encodeURIComponent(displayName))); + // Modern Unicode-safe base64 encoding + const encodedName = btoa( + String.fromCharCode(...new TextEncoder().encode(displayName)) + ); const queryParams = new HttpParams().set('uname', displayName).set('un', encodedName); const separator = joinUrl.includes('?') ? '&' : '?'; return `${joinUrl}${separator}${queryParams.toString()}`; }Note: If you update this, ensure the meeting-join component uses the same encoding logic for consistency.
apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html (2)
5-7: Clarify border styling: combineborderandborder-l-4creates unintended borders.Line 7 applies both
border(1px on all sides) andborder-l-4(4px on left), resulting in 1px borders on top/right/bottom plus a 4px left border. If the intent is only a colored left border, remove theborderclass.- <div aria-hidden="true" class="absolute border border-l-4 border-solid inset-0 pointer-events-none rounded-xl" [ngClass]="borderColorClass()"></div> + <div aria-hidden="true" class="absolute border-l-4 inset-0 pointer-events-none rounded-xl" [ngClass]="borderColorClass()"></div>Additionally,
border-solidis redundant in Tailwind (borders are solid by default).
102-113: Remove duplicatew-fullclass and reconsidertype="button"usage.Minor issues in the See Meeting Details button:
- Line 103: The wrapper has
class="w-full"and line 110 includesw-fullinstyleClass, creating duplication.- Line 108:
type="button"may not apply correctly when usingrouterLink, as the button component likely renders an anchor tag for navigation.<lfx-button - class="w-full" label="See Meeting Details" [routerLink]="['/meetings', meeting().uid]" [queryParams]="{ password: meeting().password || '' }" target="_blank" - type="button" rel="noopener noreferrer" [styleClass]="'border border-gray-300 text-gray-900 hover:bg-gray-50 h-8 text-sm font-medium w-full'" [outlined]="true" icon="fa-light fa-up-right-from-square text-xs" data-testid="dashboard-meeting-card-see-button" />
📜 Review details
Configuration used: CodeRabbit 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 (5)
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts(2 hunks)apps/lfx-one/src/app/shared/components/button/button.component.html(2 hunks)apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html(1 hunks)apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/lfx-one/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]
Files:
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.htmlapps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.htmlapps/lfx-one/src/app/shared/components/button/button.component.html
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.htmlapps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.htmlapps/lfx-one/src/app/shared/components/button/button.component.htmlapps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts
🧠 Learnings (1)
📚 Learning: 2025-10-21T21:19:13.599Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-ui PR: 125
File: apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts:345-350
Timestamp: 2025-10-21T21:19:13.599Z
Learning: In the Angular meeting card component (apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts), when selecting between `summary.summary_data.edited_content` and `summary.summary_data.content`, the logical OR operator (`||`) is intentionally used instead of nullish coalescing (`??`) because empty string edited_content should fall back to the original content rather than being displayed as empty.
Applied to files:
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.htmlapps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.htmlapps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.ts (2)
packages/shared/src/interfaces/meeting.interface.ts (2)
Meeting(75-150)MeetingOccurrence(156-167)packages/shared/src/interfaces/auth.interface.ts (1)
User(8-41)
🔇 Additional comments (6)
apps/lfx-one/src/app/shared/components/button/button.component.html (1)
26-27: LGTM! Style and class bindings added correctly.The addition of
[style]="style()"and[ngClass]="styleClass()"bindings enables dynamic styling for anchor buttons, consistent with the button element variant.apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html (2)
21-26: LGTM! Loading skeleton implementation is clean.The loading state with two 140px skeleton loaders matches the expected card dimensions and provides good visual feedback during data fetch.
27-65: LGTM! Meeting list structure is well-organized.The conditional rendering flow (loading → meeting list → empty state) is intuitive, and removing the
onSeeMeetingbindings aligns with the new navigation approach. Thedata-testidattributes follow the correct naming convention.apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html (2)
10-19: LGTM! Badge layout is clean and well-structured.The meeting type and project badges in the header provide clear visual hierarchy. The use of
data-testidattributes enables reliable testing.
22-97: LGTM! Title area layout is well-organized.The consolidated layout with title, time, feature icons, and attachment icons provides a clean and intuitive card structure. The use of the
fileTypeIconpipe withngClassfor dynamic icon styling is implemented correctly.apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts (1)
5-12: LGTM! Imports correctly support the loading state feature.The additions of
signal,SkeletonModule, and rxjs operators (tap,finalize) properly support the new skeleton loading UI functionality.Also applies to: 25-25
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts
Outdated
Show resolved
Hide resolved
...x-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html
Show resolved
Hide resolved
...lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.ts
Outdated
Show resolved
Hide resolved
...lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.ts
Outdated
Show resolved
Hide resolved
- Move MeetingTypeConfig and related constants to shared package - Add comprehensive meeting type color mappings with all shade variations - Move MeetingTypeBadge interface to shared library - Move MeetingWithOccurrence interface to shared library - Eliminate duplicate switch statements in dashboard-meeting-card component - Update my-meetings component to use shared MeetingWithOccurrence interface LFXV2-702 Signed-off-by: Asitha de Silva <[email protected]>
Summary
Changes
Dashboard Meeting Card
My Meetings Component
Button Component
Test Plan
References