-
Notifications
You must be signed in to change notification settings - Fork 0
feat(ui): enhance shadows, empty states, and committee categories #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
LFXV2-800 - Add custom shadow-md to Tailwind config for consistent styling - Update empty state designs for committees and meetings dashboards - Remove colored borders from dashboard meeting cards - Update pending actions to use shadow-md styling - Add FILTERED_COMMITTEE_CATEGORIES for form selections - Split Technical Oversight/Advisory Committee into separate options - Remove Data Copilot from board member dashboard - Fix project context usage in meeting-manage component Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
|
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. WalkthroughUI styling updates across multiple dashboard and meeting components, committee category constants refactored (new FILTERED_COMMITTEE_CATEGORIES and separate TAC/TOC entries), DataCopilot removed from a board dashboard, a new project computed property added to meeting-manage, and a Tailwind boxShadow.md theme entry added. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as MeetingManage UI
participant Comp as MeetingManageComponent
participant PCtx as ProjectContextService
UI->>Comp: prepareMeetingData(formValue)
alt formValue.selectedProjectUid set
Comp->>Comp: use formValue.selectedProjectUid
else fallback
Comp->>Comp: evaluate project() computed
Comp->>PCtx: selectedProject() / selectedFoundation()
PCtx-->>Comp: project or foundation object
Comp->>Comp: use project()?.uid for project_uid
end
Comp-->>UI: prepared payload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html (1)
145-145: Reconsider the hardcoded color value for consistency.Line 145 replaces the semantic
text-muted-foregroundwith a hardcodedtext-gray-500. However, other similar text in this component (lines 81, 84, 128, 51) continues to usetext-muted-foreground. This creates a styling inconsistency and diverges from the semantic design system.If the intent is to update all muted-foreground text to use a specific gray shade, apply this change consistently across the component. If this is a targeted update for the subtitle only, consider whether the hardcoded value should remain or revert to the semantic class for maintainability.
Can you clarify the design intent here? Should all
text-muted-foregrounduses in this component be updated totext-gray-500, or does the subtitle require a different treatment?apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.html (1)
63-63: Optional: Remove unusedtransition-colorsclass if hover state is intentionally removed.The
transition-colorsutility class is present but appears to be orphaned now that the hover border styling has been removed. If this simplification is intentional, consider cleaning up the unused transition class for consistency.- class="p-4 bg-white rounded-lg border border-slate-200 transition-colors cursor-pointer flex-shrink-0 w-80" + class="p-4 bg-white rounded-lg border border-slate-200 cursor-pointer flex-shrink-0 w-80"apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html (1)
61-79: Consider aligning empty state heading styling for visual consistency.The two empty-state sections use different heading structures and styles:
- New state (line 66):
text-slate-600 mt-2, positioned inside the icon container- Filtered state (line 76):
text-xl font-semibold text-slate-900 mt-4, positioned below the icon containerThis creates different visual hierarchies. For consistency across the dashboard, consider adopting a unified heading style and structure for both empty states.
packages/shared/src/constants/committees.constants.ts (1)
52-63: Document the purpose and usage of FILTERED_COMMITTEE_CATEGORIES.The new
FILTERED_COMMITTEE_CATEGORIESconstant provides a restricted subset of categories, but the criteria for inclusion vs. exclusion is not immediately clear. Consider adding a JSDoc comment explaining when to useFILTERED_COMMITTEE_CATEGORIESvs.COMMITTEE_CATEGORIES(e.g., "Use for form selections" vs. "Use for display/filtering all existing committees").Apply this diff to add clarifying documentation:
/** * Filtered committee categories for specific UI contexts - * @description Subset of categories for restricted selection (e.g., forms, dashboards) + * @description Restricted subset of categories for user-facing selections in forms. + * Use this for creating/editing committees to limit category choices. + * Use COMMITTEE_CATEGORIES for displaying or filtering existing committees. */ export const FILTERED_COMMITTEE_CATEGORIES = [
📜 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 (15)
apps/lfx-one/src/app/layouts/main-layout/main-layout.component.html(1 hunks)apps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.html(1 hunks)apps/lfx-one/src/app/modules/committees/components/committee-form/committee-form.component.ts(2 hunks)apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html(0 hunks)apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts(0 hunks)apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html(1 hunks)apps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.html(1 hunks)apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts(2 hunks)apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html(1 hunks)apps/lfx-one/tailwind.config.js(1 hunks)packages/shared/src/constants/committees.constants.ts(3 hunks)
💤 Files with no reviewable changes (2)
- apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts
- apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html
🧰 Additional context used
🧠 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/committees/committee-dashboard/committee-dashboard.component.htmlapps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.tsapps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.htmlapps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html
🧬 Code graph analysis (1)
apps/lfx-one/src/app/modules/committees/components/committee-form/committee-form.component.ts (1)
packages/shared/src/constants/committees.constants.ts (1)
FILTERED_COMMITTEE_CATEGORIES(56-63)
⏰ 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 (14)
apps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.html (1)
13-13: LGTM!The image height adjustment from
md:h-20tomd:h-14is a clean styling refinement that aligns with the UI enhancement objectives. No behavioral or logic changes.apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html (2)
61-69: Well-structured empty state for upcoming tab with no meetings.The new condition correctly isolates the "no upcoming meetings" case from general filter mismatches, and the centered empty state with dashed border and eyes icon aligns well with the PR's visual standardization objectives.
74-74: Icon standardization approved.The replacement of the previous icon with
fa-eyesmaintains consistency with the new empty state pattern (line 65) and aligns with the PR's visual standardization objectives.apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html (2)
15-22: Style update correctly replaces colored borders with shadow-md.The change removes the solid border and colored border classes in favor of shadow-md, aligning with the PR objectives to introduce consistent shadow styling. The ngClass simplification is correct—background colors remain for visual distinction while border color classes are appropriately removed.
Please confirm that the custom
shadow-mdutility is properly defined intailwind.config.jswith the specified values:0px 1px 2px -1px rgba(0,0,0,0.10), 0px 1px 3px 0px rgba(0,0,0,0.10).
92-94: Empty state styling aligns with PR objectives.The dashed border (
border-2 border-dashed border-gray-300) and centered text (text-center) match the intended design for empty-state components throughout the PR. Well-executed.apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
20-24: Data Copilot removal fromimportsis clean and safeThe
importsarray now only lists components actually used by this dashboard, and removing the Data Copilot component here is consistent with the described HTML cleanup. No functional or typing issues introduced.apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.html (1)
5-5: LGTM! Card styling simplified effectively.The card wrapper styling has been successfully simplified from complex multi-border and dynamic overlay styling to a clean, consistent design using
shadow-md(defined in the Tailwind config). This aligns with the removal ofborderColorClassfrom the component logic.apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts (1)
392-392: LGTM! Project context usage improved.The change from
this.projectContextService.getProjectUid()tothis.project()?.uiduses the new computed property, providing better reactivity and consistency. The optional chaining correctly handles cases where no project is selected.packages/shared/src/constants/committees.constants.ts (2)
127-128: LGTM! Color mappings updated consistently.The color mappings have been properly updated to handle the split Technical Advisory Committee and Technical Oversight Committee as separate entities, with both using
bg-brand-100 text-brand-700. The explicit check for "technical advisory" ingetCommitteeTypeColor(line 189) ensures correct color assignment for both exact matches and partial string matches.Also applies to: 189-189
44-50: Updatetag.constants.tsto remove orphaned old combined committee value and add separate entries matching new structure.The restructuring creates a backward compatibility issue. The old combined value "Technical Oversight Committee/Technical Advisory Committee" is still mapped in
packages/shared/src/constants/tag.constants.ts:71, but the newcommittees.constants.tssplits it into separate entries. Additionally, theCOMMITTEE_TYPE_COLORSobject incommittees.constants.tsintentionally added mappings for the new separate values but did not include a mapping for the old combined value—this is inconsistent with how other legacy combined values (e.g., "Marketing Oversight Committee/Marketing Advisory Committee") are handled in the same object.The concern is real: existing database records may contain the old combined value, which will not match queries using the new separate categories, causing filtering and display issues. No migration or mapping logic was found to bridge old and new values.
Recommended actions:
- Add the old combined value to
COMMITTEE_TYPE_COLORSincommittees.constants.tsas a fallback or add a migration script- Update or clarify
tag.constants.tsto reflect the new structure and confirm old value handling- Document any database migration strategy for existing committee records
apps/lfx-one/src/app/modules/committees/components/committee-form/committee-form.component.ts (1)
13-13: LGTM! Form correctly uses filtered categories.The committee form now uses
FILTERED_COMMITTEE_CATEGORIESinstead of the fullCOMMITTEE_CATEGORIESlist, appropriately restricting user selections to the approved subset. This aligns with the PR objectives to improve UX by limiting form options.Also applies to: 41-42
apps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.html (2)
82-89: LGTM! Empty state design improved.The empty state for committees has been effectively redesigned with a centered card layout, dashed border, and clear messaging. The use of the eyes icon and helpful text provides a better user experience.
92-100: LGTM! Consistent empty state design for filtered results.The "no results found" empty state follows the same design pattern as the main empty state, providing consistency and helpful guidance to adjust search or filter criteria.
apps/lfx-one/tailwind.config.js (1)
69-71: Verification confirms proper shadow utility adoption across components.The custom
shadow-mdutility is correctly defined and actively used across multiple components including pending-actions, dashboard-meeting-card, meeting-card, member-card, project-card, and fullcalendar. Both direct application and hover state enhancements demonstrate consistent adoption of the new shadow styling standard.
apps/lfx-one/src/app/layouts/main-layout/main-layout.component.html
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the UI styling and user experience across the LFX One application by introducing consistent shadow styling, improving empty state designs, refining committee category management, and fixing a project context bug. The changes focus on visual consistency and better form UX while cleaning up unused components.
Key changes include:
- Custom shadow-md definition in Tailwind for consistent styling across components
- Split Technical Oversight/Advisory Committee into separate categories for better granularity
- Enhanced empty state designs with dashed borders and centered content
- Fixed project context usage in meeting-manage component to use computed signal
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
tailwind.config.js |
Added custom shadow-md definition for consistent styling |
committees.constants.ts |
Split Technical Oversight/Advisory Committee and added FILTERED_COMMITTEE_CATEGORIES |
meetings-dashboard.component.html |
Updated empty states with new design pattern |
meeting-manage.component.ts |
Fixed project context to use computed signal instead of method call |
meeting-join.component.html |
Minor adjustment to logo height |
pending-actions.component.html |
Updated to use shadow-md instead of colored borders |
organization-involvement.component.html |
Removed hover border effect from membership tier cards |
foundation-health.component.html |
Changed text-muted-foreground to text-gray-500 |
dashboard-meeting-card.component.ts |
Removed borderColorClass signal |
dashboard-meeting-card.component.html |
Simplified card styling, removed colored border |
board-member-dashboard.component.ts |
Removed Data Copilot import |
board-member-dashboard.component.html |
Removed Data Copilot component |
committee-form.component.ts |
Updated to use FILTERED_COMMITTEE_CATEGORIES |
committee-dashboard.component.html |
Updated empty states with new design pattern |
main-layout.component.html |
Modified footer padding/margin classes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Changes
Styling Enhancements
shadow-md:0px 1px 2px -1px rgba(0, 0, 0, 0.10), 0px 1px 3px 0px rgba(0, 0, 0, 0.10)Committee Categories
FILTERED_COMMITTEE_CATEGORIESconstant for restricted form selectionsComponent Cleanup
Bug Fixes
Files Modified
tailwind.config.js- Custom shadow-md configurationpackages/shared/src/constants/committees.constants.ts- Category updatesmeeting-manage.component.ts- Project context fixcommittee-form.component.ts- Use filtered categoriesRelated Ticket
LFXV2-800
Generated with Claude Code