feat(ui): comprehensive ui improvements and persona system enhancement #128
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Multiple enhancements across the application: - LFXV2-670: Relocate footer to main layout for consistency - LFXV2-671: Replace "old-ui" with "home" persona and simplify routing - LFXV2-672: Add scrollable containers and refactor dashboard charts - LFXV2-673: Add meeting attachments display with file type icons - LFXV2-674: Enhance meeting join with password support and new tab - LFXV2-675: Improve error handling and null safety across services Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the persona system by replacing the "old-ui" persona with "projects", improves dashboard UI/UX with scrollable containers and optimized chart rendering, enhances meeting functionality with attachment displays and password-protected joins, and strengthens error handling across services.
Key Changes:
- Renamed "old-ui" persona to "projects" and simplified routing by removing the dedicated /old-ui route
- Added scrollable containers to dashboard components and refactored chart data generation for better performance
- Implemented meeting attachments display with dynamic file type icons and enhanced join functionality with password support
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/persona.interface.ts | Updated PersonaType to replace 'old-ui' with 'projects' |
| packages/shared/src/interfaces/components.interface.ts | Changed chart type definitions to use Chart.js ChartType |
| packages/shared/src/constants/persona.constants.ts | Updated persona option label from "Old UI" to "Projects" |
| apps/lfx-one/src/server/server.ts | Changed error handling to logout users when user info fetch fails |
| apps/lfx-one/src/app/shared/services/persona.service.ts | Removed localStorage initialization and old-ui routing logic |
| apps/lfx-one/src/app/shared/services/analytics.service.ts | Added null safety checks for user and window objects |
| apps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.ts | Simplified form initialization by removing effect-based sync |
| apps/lfx-one/src/app/shared/components/header/header.component.ts | Added error handling with catchError for user profile fetching |
| apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.ts | Added dynamic meeting attachments fetching and display |
| apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html | Replaced static file icons with dynamic attachment links |
| apps/lfx-one/src/app/shared/components/button/button.component.ts | Added queryParams input support |
| apps/lfx-one/src/app/shared/components/button/button.component.html | Added queryParams and target binding to p-button |
| apps/lfx-one/src/app/modules/pages/home/home.component.html | Removed top padding from hero section |
| apps/lfx-one/src/app/modules/dashboards/dashboard.component.ts | Updated to route projects persona to HomeComponent |
| apps/lfx-one/src/app/modules/dashboards/dashboard.component.html | Added case for projects persona |
| apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html | Added max-height and overflow-y-auto for scrolling |
| apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts | Refactored to pre-generate chart data in constructor |
| apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html | Simplified chart rendering by using pre-generated data |
| apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html | Added max-height and overflow-y-auto for scrolling |
| apps/lfx-one/src/app/layouts/main-layout/main-layout.component.ts | Added CUSTOM_ELEMENTS_SCHEMA support |
| apps/lfx-one/src/app/layouts/main-layout/main-layout.component.html | Added footer component to main layout |
| apps/lfx-one/src/app/app.routes.ts | Removed dedicated /old-ui route |
| apps/lfx-one/src/app/app.component.ts | Removed CUSTOM_ELEMENTS_SCHEMA from app component |
| apps/lfx-one/src/app/app.component.html | Removed footer from app component template |
Comments suppressed due to low confidence (1)
apps/lfx-one/src/app/shared/services/persona.service.ts:37
- Extra closing brace on line 32. This appears to be a syntax error where line 32 should be removed as the method already closes properly on line 31.
}
}
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
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. WalkthroughThis PR refactors the application by replacing the deprecated 'old-ui' persona type with a new 'projects' type, relocating the footer component from the root app to the main layout, adding meeting attachment display with file icons, enhancing the projects dashboard with computed chart data, and introducing query parameter support to button navigation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MeetingCard as Dashboard Meeting Card
participant MeetingService
participant Backend
User->>MeetingCard: Component renders with meeting signal
MeetingCard->>MeetingCard: Detect meeting change via toObservable
MeetingCard->>MeetingService: getMeetingAttachments(meeting.uid)
alt Meeting UID exists
MeetingService->>Backend: Fetch attachments
alt Success
Backend-->>MeetingService: MeetingAttachment[]
MeetingService-->>MeetingCard: attachments Signal updated
else Error
Backend-->>MeetingService: Error
MeetingService-->>MeetingCard: attachments = []
end
else No UID
MeetingService-->>MeetingCard: attachments = []
end
MeetingCard->>User: Render attachment icons or empty state
sequenceDiagram
participant PersonaSelector as Persona Selector
participant PersonaService
participant Dashboard
PersonaSelector->>PersonaService: User selects new persona
PersonaService->>PersonaService: setPersona(newPersona)
PersonaService->>PersonaService: Navigate to '/'
PersonaService->>Dashboard: Dashboard signal triggers re-evaluation
Dashboard->>Dashboard: dashboardType computed with new persona
Dashboard->>Dashboard: Switch case renders appropriate component (projects | core-developer | maintainer)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple areas (routing, persona system, dashboard components, services) with moderate complexity. While many changes follow a consistent pattern (old-ui → projects migration), the projects dashboard refactoring introduces new chart logic requiring careful review. Meeting attachments feature adds reactive signal-based data loading. Most changes are cohesive and localized within their domains. Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/lfx-one/src/app/shared/services/persona.service.ts (1)
17-20: Don't drop the persisted persona on reloadBy removing the localStorage initialization we now always boot with
'core-developer', butpersistPersonastill writes the user’s choice. After a refresh the stored value is ignored, so the UI silently reverts to the default persona. That’s a regression in behavior and breaks the promise of persisting the selection. Please restore the stored value before creating the signal (and keep a safe fallback), e.g.:- // Initialize with default value - this.currentPersona = signal<PersonaType>('core-developer'); + const storedPersona = typeof window !== 'undefined' + ? (localStorage.getItem(this.storageKey) as PersonaType | null) + : null; + this.currentPersona = signal<PersonaType>(storedPersona ?? 'core-developer');
🧹 Nitpick comments (6)
packages/shared/src/interfaces/components.interface.ts (1)
4-4: Consider narrowing chart typings to dashboard-supported typesUsing ChartType broadens to all charts and may mask config/data mismatches. Suggest a bounded alias and reuse it:
// shared, near interfaces export type DashboardChartType = Extract<ChartType, 'line' | 'bar'>; // then chartType: DashboardChartType; chartData: ChartData<DashboardChartType>; chartOptions: ChartOptions<DashboardChartType>;Also applies to: 323-328
apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html (1)
20-20: Scrollable container looks goodMeets the UX goal. Optionally add a data-testid on the scroll wrapper for e2e assertions (e.g., dashboard-pending-actions-scroll).
packages/shared/src/interfaces/persona.interface.ts (1)
8-8: Persona value update aligns with routing changesSwap to 'projects' is consistent. For maintainability, consider replacing the string union with either:
export enum PersonaTypeEnum { CoreDeveloper = 'core-developer', Maintainer = 'maintainer', Projects = 'projects', } // and use PersonaTypeEnum where applicableOr the as-const pattern:
export const PERSONA_TYPES = ['core-developer','maintainer','projects'] as const; export type PersonaType = typeof PERSONA_TYPES[number];apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.ts (1)
4-14: Attachments reactive stream is well-structuredGood use of toObservable/switchMap/catchError and toSignal. Consider deduping by uid to prevent redundant fetches when the same meeting instance re-emits:
-const attachments$ = meeting$.pipe( - switchMap((meeting) => { +const attachments$ = meeting$.pipe( + // Dedup by UID + distinctUntilChanged((a, b) => a?.uid === b?.uid), + switchMap((meeting) => { if (meeting.uid) { return this.meetingService.getMeetingAttachments(meeting.uid).pipe(catchError(() => of([]))); } return of([]); }) );Also applies to: 23-24, 27-34, 148-161
apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html (1)
73-88: Improve accessibility and testability for attachment icons.
- Add an aria-label on the anchor for SR users.
- Mark the decorative as aria-hidden.
- Expose a stable container test id for the attachments row.
- <div class="flex items-center gap-1 flex-shrink-0 mt-0.5"> + <div class="flex items-center gap-1 flex-shrink-0 mt-0.5" + data-testid="dashboard-meeting-card-attachments"> @@ - <a + <a [href]="attachment.file_url" target="_blank" rel="noopener noreferrer" class="w-6 h-6 rounded bg-gray-100 flex items-center justify-center hover:bg-gray-200 transition-colors" [attr.data-testid]="'dashboard-meeting-card-file-icon-' + (index + 1)" + [attr.aria-label]="'Open attachment: ' + (attachment.file_name || 'file')" [pTooltip]="attachment.file_name" tooltipPosition="top"> - <i [class]="(((attachment.mime_type ?? '') | fileTypeIcon) + ' text-xs text-gray-500')"></i> + <i aria-hidden="true" + [class]="(((attachment.mime_type ?? '') | fileTypeIcon) + ' text-xs text-gray-500')"></i> </a>apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts (1)
31-39: Chart options: mark as constant for clarity and potential inlining.Minor: declare as a const to signal immutability.
- protected readonly chartOptions: ChartOptions<'line'> = { + protected readonly chartOptions: ChartOptions<'line'> = { responsive: true, maintainAspectRatio: false, plugins: { legend: { display: false }, tooltip: { enabled: false } }, scales: { x: { display: false }, y: { display: false }, }, - }; + } as const;
📜 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 (24)
apps/lfx-one/src/app/app.component.html(0 hunks)apps/lfx-one/src/app/app.component.ts(1 hunks)apps/lfx-one/src/app/app.routes.ts(0 hunks)apps/lfx-one/src/app/layouts/main-layout/main-layout.component.html(1 hunks)apps/lfx-one/src/app/layouts/main-layout/main-layout.component.ts(2 hunks)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-projects/my-projects.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/dashboard.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/dashboard.component.ts(3 hunks)apps/lfx-one/src/app/modules/pages/home/home.component.html(1 hunks)apps/lfx-one/src/app/shared/components/button/button.component.html(1 hunks)apps/lfx-one/src/app/shared/components/button/button.component.ts(1 hunks)apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.html(2 hunks)apps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.ts(3 hunks)apps/lfx-one/src/app/shared/components/header/header.component.ts(2 hunks)apps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.ts(2 hunks)apps/lfx-one/src/app/shared/services/analytics.service.ts(2 hunks)apps/lfx-one/src/app/shared/services/persona.service.ts(2 hunks)apps/lfx-one/src/server/server.ts(1 hunks)packages/shared/src/constants/persona.constants.ts(1 hunks)packages/shared/src/interfaces/components.interface.ts(2 hunks)packages/shared/src/interfaces/persona.interface.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/lfx-one/src/app/app.routes.ts
- apps/lfx-one/src/app/app.component.html
🧰 Additional context used
📓 Path-based instructions (6)
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/layouts/main-layout/main-layout.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.htmlapps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.htmlapps/lfx-one/src/app/modules/dashboards/dashboard.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.htmlapps/lfx-one/src/app/modules/pages/home/home.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.htmlpackages/shared/src/interfaces/persona.interface.tsapps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.tsapps/lfx-one/src/app/layouts/main-layout/main-layout.component.htmlapps/lfx-one/src/app/shared/components/button/button.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.htmlapps/lfx-one/src/app/shared/components/header/header.component.tsapps/lfx-one/src/app/app.component.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/app/modules/dashboards/dashboard.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/layouts/main-layout/main-layout.component.tspackages/shared/src/constants/persona.constants.tsapps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.htmlapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/app/modules/dashboards/dashboard.component.htmlapps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.tsapps/lfx-one/src/app/shared/services/persona.service.tsapps/lfx-one/src/app/shared/components/button/button.component.htmlapps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.htmlapps/lfx-one/src/app/modules/pages/home/home.component.html
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all TypeScript interfaces in the shared package at packages/shared/src/interfaces
Files:
packages/shared/src/interfaces/persona.interface.tspackages/shared/src/interfaces/components.interface.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:
packages/shared/src/interfaces/persona.interface.tsapps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.tsapps/lfx-one/src/app/shared/components/button/button.component.tsapps/lfx-one/src/app/shared/components/header/header.component.tsapps/lfx-one/src/app/app.component.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/app/modules/dashboards/dashboard.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/layouts/main-layout/main-layout.component.tspackages/shared/src/constants/persona.constants.tsapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.tsapps/lfx-one/src/app/shared/services/persona.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
packages/shared/src/interfaces/persona.interface.tsapps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.tsapps/lfx-one/src/app/shared/components/button/button.component.tsapps/lfx-one/src/app/shared/components/header/header.component.tsapps/lfx-one/src/app/app.component.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/app/modules/dashboards/dashboard.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/layouts/main-layout/main-layout.component.tspackages/shared/src/constants/persona.constants.tsapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/app/shared/components/dashboard-meeting-card/dashboard-meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.tsapps/lfx-one/src/app/shared/services/persona.service.ts
packages/shared/src/constants/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all reusable constants in the shared package at packages/shared/src/constants
Files:
packages/shared/src/constants/persona.constants.ts
🧬 Code graph analysis (4)
apps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.ts (1)
packages/shared/src/interfaces/persona.interface.ts (1)
PersonaType(8-8)
apps/lfx-one/src/app/modules/dashboards/dashboard.component.ts (1)
packages/shared/src/interfaces/persona.interface.ts (1)
PersonaType(8-8)
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/meeting-attachment.interface.ts (1)
MeetingAttachment(8-27)
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts (1)
packages/shared/src/interfaces/components.interface.ts (1)
ProjectItem(368-383)
🔇 Additional comments (5)
apps/lfx-one/src/app/shared/components/button/button.component.html (1)
57-59: Router bindings look good.Passing
queryParamsandtargetthroughp-buttonkeeps router navigation consistent with the new input.apps/lfx-one/src/app/shared/components/header/header.component.ts (1)
164-171: Confirm logout still triggers when profile fetch fails.By catching every error here and converting it to
null, no downstream code will ever observe the rejection fromgetCurrentUserProfile(). Please double-check that our “logout on user info fetch failure” flow still runs (e.g., redirecting to/logoutor clearing auth state) when this observable errors; if it doesn’t, we might need to invoke the logout logic explicitly before returningof(null).apps/lfx-one/src/app/app.component.ts (1)
5-5: SSR context injection looks correctImporting REQUEST_CONTEXT aligns with server.handle context (auth) and TransferState hydration. LGTM.
apps/lfx-one/src/app/modules/pages/home/home.component.html (1)
4-4: Layout spacing tweak is fineTop padding removal is safe; test IDs intact.
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
85-88: Robustness improvements to analytics are good
- Early-return in identifyUser prevents invalid identify calls.
- Optional chaining in waitForAnalytics avoids SSR/window issues.
Also applies to: 139-139
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts
Show resolved
Hide resolved
Comment on lines
+29
to
32
| protected readonly dashboardType: Signal<PersonaType> = computed(() => { | ||
| const persona = this.personaService.currentPersona(); | ||
| // Filter out 'old-ui' as it has its own route | ||
| return persona === 'old-ui' ? 'core-developer' : persona; | ||
| return persona; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Typed Signal looks good; add a safe default to harden against bad persona values.
Defensively default to 'projects' if the service ever returns null/unknown.
- protected readonly dashboardType: Signal<PersonaType> = computed(() => {
- const persona = this.personaService.currentPersona();
- return persona;
- });
+ protected readonly dashboardType: Signal<PersonaType> = computed(() => {
+ const persona = this.personaService.currentPersona();
+ return (persona ?? 'projects') as PersonaType;
+ });To ensure alignment with objectives, please confirm the set of persona literals used across the app:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 "PersonaType|currentPersona|setPersona|\bprojects\b|\bhome\b"Length of output: 111653
Remove unnecessary fallback and add license header
- The
dashboardTypecomputed signal safely returns aPersonaTypeand cannot be null/undefined, so the proposed?? 'projects'fallback is redundant. - Add the required SPDX license header at the top of
apps/lfx-one/src/app/modules/dashboards/dashboard.component.ts.
🤖 Prompt for AI Agents
In apps/lfx-one/src/app/modules/dashboards/dashboard.component.ts around lines
29 to 32, remove the unnecessary fallback used when computing dashboardType (the
computed signal should directly return the PersonaType from
personaService.currentPersona() with no '?? "projects"' or similar fallback) and
add the required SPDX license header at the very top of the file (e.g., the
appropriate SPDX short identifier and copyright line) so the file begins with
the licensed header followed by the existing imports and code.
| public readonly href = input<string | undefined>(undefined); | ||
| public readonly target = input<string | undefined>(undefined); | ||
| public readonly rel = input<string | undefined>(undefined); | ||
| public readonly queryParams = input<Record<string, string>>({}); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use Angular Params type for router query params.
The shared button should accept the full RouterLink Params contract (numbers, booleans, arrays). Record<string, string> rejects those valid shapes and forces consumers to cast or stringify. Switch to Params and keep the input optional to match Angular’s expectations.
-import { RouterModule } from '@angular/router';
+import { Params, RouterModule } from '@angular/router';
…
- public readonly queryParams = input<Record<string, string>>({});
+ public readonly queryParams = input<Params | undefined>(undefined);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public readonly queryParams = input<Record<string, string>>({}); | |
| // Before: | |
| import { RouterModule } from '@angular/router'; | |
| // After: | |
| import { Params, RouterModule } from '@angular/router'; | |
| … | |
| // Before: | |
| public readonly queryParams = input<Record<string, string>>({}); | |
| // After: | |
| public readonly queryParams = input<Params | undefined>(undefined); |
🤖 Prompt for AI Agents
In apps/lfx-one/src/app/shared/components/button/button.component.ts around line
58, replace the current input type Record<string, string> with Angular's Params
type and make the input optional; update imports to include Params from
@angular/router and change the input declaration to use Params (or Params |
undefined) with an optional/default value so that numbers, booleans, arrays and
other valid RouterLink shapes are accepted without consumer casting.
...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
Comment on lines
+241
to
243
| res.oidc.logout(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Avoid unconditional logout on userinfo fetch failures
Risk of logout loops and poor UX on transient/network errors. Only logout on definite auth errors (401/403/invalid_token); otherwise fall back to unauthenticated render.
Apply this diff locally to scope the behavior:
- res.oidc.logout();
- return;
+ const status = (error as any)?.statusCode ?? (error as any)?.status;
+ const code = (error as any)?.code;
+ if (status === 401 || status === 403 || code === 'invalid_token') {
+ res.oidc.logout();
+ return;
+ }
+ // Non-auth failure: render as unauthenticated to avoid loops
+ auth.authenticated = false;
+ auth.user = null;Also update the preceding warning message to reflect the new behavior (it currently mentions “using basic user data”).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| res.oidc.logout(); | |
| return; | |
| } | |
| const status = (error as any)?.statusCode ?? (error as any)?.status; | |
| const code = (error as any)?.code; | |
| if (status === 401 || status === 403 || code === 'invalid_token') { | |
| res.oidc.logout(); | |
| return; | |
| } | |
| // Non-auth failure: render as unauthenticated to avoid loops | |
| auth.authenticated = false; | |
| auth.user = null; | |
| } |
🤖 Prompt for AI Agents
In apps/lfx-one/src/server/server.ts around lines 241-243, the code
unconditionally calls res.oidc.logout() when fetching userinfo fails; change
this so only definite auth errors (HTTP 401, 403, or an invalid_token error)
trigger logout, while other failures (network/timeouts/5xx) fall back to
rendering the page as unauthenticated. Update the surrounding catch/if logic to
inspect the error/response status or error code and call res.oidc.logout() only
for 401/403/invalid_token; for all other errors log a warning and proceed with
unauthenticated render. Also update the preceding warning message text to
reflect this behavior (mention falling back to unauthenticated render on
transient errors rather than saying “using basic user data”).
dealako
approved these changes
Oct 23, 2025
mauriciozanettisalomao
pushed a commit
that referenced
this pull request
Oct 24, 2025
#128) Multiple enhancements across the application: - LFXV2-670: Relocate footer to main layout for consistency - LFXV2-671: Replace "old-ui" with "home" persona and simplify routing - LFXV2-672: Add scrollable containers and refactor dashboard charts - LFXV2-673: Add meeting attachments display with file type icons - LFXV2-674: Enhance meeting join with password support and new tab - LFXV2-675: Improve error handling and null safety across services Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
mauriciozanettisalomao
pushed a commit
that referenced
this pull request
Oct 24, 2025
#128) Multiple enhancements across the application: - LFXV2-670: Relocate footer to main layout for consistency - LFXV2-671: Replace "old-ui" with "home" persona and simplify routing - LFXV2-672: Add scrollable containers and refactor dashboard charts - LFXV2-673: Add meeting attachments display with file type icons - LFXV2-674: Enhance meeting join with password support and new tab - LFXV2-675: Improve error handling and null safety across services Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
mauriciozanettisalomao
pushed a commit
that referenced
this pull request
Oct 24, 2025
#128) Multiple enhancements across the application: - LFXV2-670: Relocate footer to main layout for consistency - LFXV2-671: Replace "old-ui" with "home" persona and simplify routing - LFXV2-672: Add scrollable containers and refactor dashboard charts - LFXV2-673: Add meeting attachments display with file type icons - LFXV2-674: Enhance meeting join with password support and new tab - LFXV2-675: Improve error handling and null safety across services Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
mauriciozanettisalomao
pushed a commit
that referenced
this pull request
Oct 24, 2025
#128) Multiple enhancements across the application: - LFXV2-670: Relocate footer to main layout for consistency - LFXV2-671: Replace "old-ui" with "home" persona and simplify routing - LFXV2-672: Add scrollable containers and refactor dashboard charts - LFXV2-673: Add meeting attachments display with file type icons - LFXV2-674: Enhance meeting join with password support and new tab - LFXV2-675: Improve error handling and null safety across services Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
mauriciozanettisalomao
pushed a commit
that referenced
this pull request
Oct 24, 2025
#128) Multiple enhancements across the application: - LFXV2-670: Relocate footer to main layout for consistency - LFXV2-671: Replace "old-ui" with "home" persona and simplify routing - LFXV2-672: Add scrollable containers and refactor dashboard charts - LFXV2-673: Add meeting attachments display with file type icons - LFXV2-674: Enhance meeting join with password support and new tab - LFXV2-675: Improve error handling and null safety across services Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]> Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR includes multiple UI improvements and persona system enhancements across the application:
Changes
LFXV2-670: Relocate footer to main layout for consistency
LFXV2-671: Replace "old-ui" with "home" persona and simplify routing
LFXV2-672: Add scrollable containers and refactor dashboard charts
LFXV2-673: Add meeting attachments display with file type icons
LFXV2-674: Enhance meeting join with password support and new tab
LFXV2-675: Improve error handling and null safety across services
Test Plan
Related Issues
Generated with Claude Code