-
Notifications
You must be signed in to change notification settings - Fork 0
fix(ui): replace contrast severity with secondary for consistent styling #208
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
- Created LoggerService singleton with WeakMap-based operation tracking - Prevents duplicate logs and memory leaks - CloudWatch-optimized structured JSON logging - Updated 10 controllers, 2 middleware, 7 services, 2 utils - Improved error handling with centralized error middleware logging - Removed redundant error logs before next(error) calls - Net reduction of 706 lines LFXV2-903 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
Extended LoggerService to support both request-scoped and infrastructure operations by making req parameter optional. Migrated all 41 direct serverLogger calls across services, utilities, and routes to use the unified logger service pattern. Changes: - Extended LoggerService methods to accept Request | undefined - Migrated 11 request-scoped operations to pass req parameter - Migrated 30 infrastructure operations to use logger with undefined - Updated ai.service, user.service, project.service method signatures - Replaced serverLogger calls in nats, snowflake, lock-manager services - Updated CLAUDE.md with comprehensive logging documentation - Removed serverLogger imports from all service/utility files Benefits: - Unified logging interface for all operations - Better request correlation when context available - Consistent error handling with err field - Infrastructure operations now use same pattern LFXV2-903 Signed-off-by: Asitha de Silva <[email protected]>
Updated logging-monitoring.md to reflect the new dual-mode logger service that supports both request-scoped and infrastructure operations. Added documentation for the new server-logger.ts file that breaks the circular dependency between server.ts and logger.service.ts. Changes: - Added logging architecture layers diagram showing server-logger.ts - Updated all method signatures to show req | undefined pattern - Added infrastructure logging examples for all methods - Documented circular dependency resolution - Clarified when to use request-scoped vs infrastructure logging LFXV2-903 Signed-off-by: Asitha de Silva <[email protected]>
Moved AI agenda generation logic from inline route handler to MeetingController.generateAgenda method for better separation of concerns and consistency with other route patterns. Also improved logger usage in middleware and services: - auth.middleware: better error tracking with startTime - error-handler.middleware: use operation startTime when available - access-check.service: move startOperation before try block - etag.service: use debug instead of silent startOperation - persona-helper: use success/error instead of warning LFXV2-903 Signed-off-by: Asitha de Silva <[email protected]>
Updated CLAUDE.md logging architecture diagram to reflect the new server-logger.ts file that breaks the circular dependency between server.ts and logger.service.ts. LFXV2-903 Signed-off-by: Asitha de Silva <[email protected]>
Signed-off-by: Asitha de Silva <[email protected]>
LFXV2-903 Use ServiceValidationError.fromFieldErrors() and next() for validation errors instead of manual 400 response, consistent with other controller methods. Provides field-level error details and centralized error handling through middleware. Signed-off-by: Asitha de Silva <[email protected]>
Signed-off-by: Asitha de Silva <[email protected]>
Replace 'contrast' severity with 'secondary' across meeting and committee components for improved visual consistency and better theme compatibility. Changes: - Member card: secretary role badge severity - Dashboard meeting card: button severities and meeting type badges - Meeting card: join button severities and meeting type badges - Meeting join: button styling and meeting type badges LFXV2-905 Signed-off-by: Asitha de Silva <[email protected]>
WalkthroughBadge severity mappings are updated from 'contrast' to 'secondary' for the secretary role and technical meeting type across member and meeting components. Button styling is refactored to use component inputs (severity, size, styleClass) instead of inline classes across multiple templates. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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 implements a comprehensive logging system refactor, replacing the previous Logger helper class with a new singleton LoggerService that provides operation lifecycle tracking, CloudWatch optimization, and duplicate error prevention. Additionally, it updates UI components to use secondary severity instead of contrast for consistent styling.
Key changes:
- New singleton
LoggerServicewith operation tracking (startOperation → success/error) - Breaking circular dependency by extracting
serverLoggertoserver-logger.ts - Migration from
Logger.start/success/errortologger.startOperation/success/erroracross all controllers and services - UI consistency improvements replacing
contrastwithsecondaryseverity - Turbo dependency bump from 2.5.5 to 2.6.3
Reviewed changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
apps/lfx-one/src/server/services/logger.service.ts |
New singleton LoggerService with operation lifecycle tracking |
apps/lfx-one/src/server/server-logger.ts |
Extracted base Pino logger to break circular dependencies |
apps/lfx-one/src/server/server.ts |
Updated to use new logger architecture |
apps/lfx-one/src/server/helpers/logger.ts |
Deleted old Logger helper class |
apps/lfx-one/src/server/services/*.ts |
Migrated all services to use new logger |
apps/lfx-one/src/server/controllers/*.ts |
Migrated all controllers to use new logger |
apps/lfx-one/src/server/middleware/*.ts |
Updated middleware to use new logger |
apps/lfx-one/src/app/modules/**/**.component.ts |
Changed contrast to secondary severity |
docs/architecture/backend/logging-monitoring.md |
Comprehensive documentation rewrite |
package.json, yarn.lock |
Updated turbo from 2.5.5 to 2.6.3 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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: 0
🧹 Nitpick comments (3)
apps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.html (1)
117-129: Consider consolidating full-width styling.The button wrapper has
class="w-full h-[40px]"(line 118) while the Button component receivesstyleClass="w-full"(line 119). IfstyleClassapplies the full-width style to the inner button element, the wrapper'sw-fullclass might be redundant. Consider verifying whether both are necessary or if the wrapper class can be simplified to justclass="h-[40px]".apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.html (1)
199-230: Button styling updated correctly; consider consolidating width classes.The Join Meeting buttons have been correctly updated with
severity="success"andstyleClass="w-full"(lines 211-212, 225-226), aligning with the PR objectives for consistent button styling. However, note that the wrapper divs already haveclass="w-full"(lines 205, 219). Similar to the meeting-join component, consider verifying whether both the wrapper'sw-fullclass and the button'sstyleClass="w-full"are both necessary, or if one can be simplified.apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.html (1)
99-133: Button presentation inputs updated correctly; consider consolidating width classes.Both buttons have been correctly updated with presentation inputs (
severity,size,styleClass) that align with the PR objectives for consistent button styling. The Join Meeting button'sseverity="success"(line 128) is appropriate. However, similar to other components in this PR, both buttons haveclass="w-full"on their wrapper divs (lines 102, 120) andstyleClass="w-full"on the button components (lines 105, 129). Consider verifying if this duplication is intentional or if the wrapper classes can be simplified.
📜 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 (7)
apps/lfx-one/src/app/modules/committees/components/member-card/member-card.component.ts(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.html(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts(1 hunks)apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.html(2 hunks)apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.ts(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-join/meeting-join.component.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
apps/lfx-one/src/**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.ts: Use zoneless change detection and signals in Angular 19 components
All PrimeNG components must be wrapped in LFX components for UI library independence
Reference PrimeNG's component interface when defining types for PrimeNG wrappers
Files:
apps/lfx-one/src/app/modules/committees/components/member-card/member-card.component.tsapps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts
apps/lfx-one/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.ts: Use direct imports for standalone components - no barrel exports
Shared package uses direct source imports during development for hot reloading
Files:
apps/lfx-one/src/app/modules/committees/components/member-card/member-card.component.tsapps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Always use async/await pattern for promises
Use TypeScript interfaces instead of union types for better maintainability
Files:
apps/lfx-one/src/app/modules/committees/components/member-card/member-card.component.tsapps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts
**/*.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,js}: License headers are required on all source files - run ./check-headers.sh to verify
Do not nest ternary expressions
Prepend 'Generated with Claude Code (https://claude.ai/code)' when you assisted with the code
Files:
apps/lfx-one/src/app/modules/committees/components/member-card/member-card.component.tsapps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts
apps/lfx-one/src/**/*.component.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.html: Always add data-testid attributes when creating new components for reliable test targeting
Use data-testid naming convention - [section]-[component]-[element] for hierarchical structure
Files:
apps/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/components/meeting-card/meeting-card.component.html
🧠 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/committees/components/member-card/member-card.component.tsapps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts
🔇 Additional comments (4)
apps/lfx-one/src/app/modules/committees/components/member-card/member-card.component.ts (1)
51-51: LGTM! Severity mapping updated consistently.The secretary role badge severity has been correctly updated from 'contrast' to 'secondary', aligning with the PR objectives and maintaining consistency with the default fallback on line 52.
apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.ts (1)
574-574: LGTM! Technical meeting type badge severity updated.The technical meeting type badge severity has been correctly changed from 'contrast' to 'secondary', consistent with similar changes across meeting-join.component.ts and dashboard-meeting-card.component.ts.
apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts (1)
106-106: LGTM! Color-to-severity mapping updated consistently.The purple textColor mapping has been correctly changed from 'contrast' to 'secondary', aligning with the technical meeting type badge changes in other components.
apps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.ts (1)
339-339: LGTM! Badge severity standardized across components.The technical meeting type badge severity has been correctly updated from 'contrast' to 'secondary', maintaining consistency with the identical change in meeting-card.component.ts (line 574).
Description
Replace
contrastseverity withsecondaryacross meeting and committee components for improved visual consistency and better theme compatibility.Changes
contrasttosecondaryAffected Components
member-card.component.ts- Role badge severitydashboard-meeting-card.component.*- Button and badge severitiesmeeting-card.component.*- Join button and badge severitiesmeeting-join.component.*- Button styling and badge severitiesImpact
JIRA Ticket
LFXV2-905
Generated with Claude Code