Closed
Conversation
- Add 64px lens switcher with per-lens icons, active state, tooltips, LFX Insights shortcut, Changelog button, and external link indicators - Foundation lens: foundation/project selector with click-outside close, Foundation/Project type labels, sticky Insights card linking to insights.linuxfoundation.org/collection/details/<slug> - Me lens: user profile card at top of nav, 'My...' prefixed engagement items, lens-aware page titles (My Meetings, My Committees, etc.) - Organization lens: Maintainer-only nav with Portfolio/Membership/ Administration sections, org avatar with deterministic color - Dev toolbar unified via AppService as single source of truth; z-index stacking fixed (sidebar z-60, backdrop z-50, toolbar z-100) - Board Member and Executive Director can now switch foundations - Me lens Overview shows 'My LFX Overview' header with subtitle; Recent Progress section removed - All page h1 headers use text-xl (20px) per LFX font scale Co-Authored-By: Claude Sonnet 4.6 <[email protected]> Signed-off-by: Nuno Eufrasio <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Builds out the complete Org Lens experience with 7 sections, role-based nav visibility, and user-type-aware content throughout. Pages added: - Overview: role-differentiated home (Employee vs Admin content) - Memberships: Active/Expired/Discover tabs + membership detail with 5 sub-tabs (Documentation, Key Contacts, Board & Committees, Onboarding, ROI) - Key Projects: influence level summary + workspace filter + project table - Activities: 3 tabs (Code Contributions, Training & Certifications, Events) with Employee/Admin content split - People: 3 tabs (LFX Access, Board & Committee Members, Request Data Correction) - Company Data: 2 tabs (Company Profile, Company Data with domains & conglomerate structure) - OSPO Resources: topic-based OSPO guidance with resource cards and external links Infrastructure: - OrgUserType (Employee / Admin View / Admin Edit / Conglomerate Admin) added to shared interfaces and constants - AppService extended with orgUserType signal and setOrgUserType() - Dev toolbar shows User Type selector when in Org lens - Org nav is now a computed signal — Employee sees 4 items, Admin roles see all 7 - All 9 routes lazy-loaded via ORG_LENS_ROUTES LFXV2-XXX Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Contributor
WalkthroughAdds a multi-lens UI (me, foundation, org): new AppService lens/orgUserType state, lens-switcher and sidebar adjustments, new org-lens route module and many org-lens standalone components/templates, project-selector refactor (popover → fixed panel), and placeholder routes for several me/foundation pages. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LensSwitcher
participant AppService
participant Router
participant MainLayout
participant OrgModule
rect rgba(200,200,255,0.5)
User->>LensSwitcher: click lens button
end
LensSwitcher->>AppService: setLens('org'|'me'|'foundation')
AppService-->>LensSwitcher: activeLens updated
AppService->>Router: navigate(default route for lens)
Router->>MainLayout: route change (NavigationEnd)
MainLayout->>AppService: read activeLens, projectSelectorOpen, showDevToolbar
AppService->>OrgModule: (on-demand) lazy-load org routes/components
OrgModule-->>MainLayout: render org component
MainLayout->>User: updated sidebar/layout and content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
github-actions
bot
requested review from
andrest50,
asithade,
dealako,
jordane and
mauriciozanettisalomao
🚀 Deployment StatusYour branch has been deployed to: https://ui-pr-391.dev.v2.cluster.linuxfound.info Deployment Details:
The deployment will be automatically removed when this PR is closed. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new multi-“lens” navigation model and adds the Organization Lens (/org/*) as a standalone, lazy-loaded module with role-aware UI.
Changes:
- Added Organization Lens routes/components and introduced
OrgUserTypewith dev-toolbar selection. - Implemented lens switcher + sidebar rework (me/foundation/org nav variants) and updated page titles based on the active lens.
- Rebuilt the project selector UI into a custom fixed-position panel and added new branding assets.
Reviewed changes
Copilot reviewed 46 out of 49 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/persona.interface.ts | Adds OrgUserType type definition for org-lens access level. |
| packages/shared/src/constants/persona.constants.ts | Adds ORG_USER_TYPE_OPTIONS for dev-toolbar selection. |
| apps/lfx-one/src/app/shared/services/persona.service.ts | Imports updated; (currently includes an unused AppService import). |
| apps/lfx-one/src/app/shared/services/app.service.ts | Adds global lens state, org user type state, and project-selector open state. |
| apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts | Adds org/me selectors, insights card logic, and scroll-to-top effect on nav change. |
| apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html | Updates sidebar layout, adds org/me selectors, adds fixed insights card. |
| apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.ts | Replaces PrimeNG popover behavior with custom open/close + outside-click handling. |
| apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.scss | Updates project selector typography and removes popover styling overrides. |
| apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.html | Implements fixed-position right-side selector panel UI. |
| apps/lfx-one/src/app/shared/components/placeholder-page/placeholder-page.component.ts | Adds simple placeholder page component for new routes. |
| apps/lfx-one/src/app/shared/components/placeholder-page/placeholder-page.component.html | Placeholder “Coming Soon” template. |
| apps/lfx-one/src/app/shared/components/lens-switcher/lens-switcher.component.ts | Adds lens switcher component + footer actions with tooltips. |
| apps/lfx-one/src/app/shared/components/lens-switcher/lens-switcher.component.scss | Host layout styling for lens switcher. |
| apps/lfx-one/src/app/shared/components/lens-switcher/lens-switcher.component.html | Lens switcher UI and footer links. |
| apps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.ts | Adds org projects page with influence filtering and table rendering. |
| apps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.html | Org projects UI (summary, filters, table). |
| apps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.ts | Adds org profile/company data page with admin/edit gating. |
| apps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.html | Org profile/company data UI (tabs, domains, conglomerate structure). |
| apps/lfx-one/src/app/modules/org-lens/permissions/org-permissions.component.ts | Adds org permissions page (static data). |
| apps/lfx-one/src/app/modules/org-lens/permissions/org-permissions.component.html | Org permissions UI (stats + table). |
| apps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.ts | Adds org overview page with admin vs employee sections. |
| apps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.html | Org overview UI (summary, activities, admin-only cards, projects). |
| apps/lfx-one/src/app/modules/org-lens/org-lens.routes.ts | Adds lazy-loaded org lens route map. |
| apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.ts | Adds org membership list/detail pages with edit gating. |
| apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html | Org membership UI (tabs, list, detail tabs, onboarding, ROI). |
| apps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.ts | Adds org people/groups page with tabbed views and admin/edit gating. |
| apps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.html | Org people/groups UI (access, board seats, correction request). |
| apps/lfx-one/src/app/modules/org-lens/code/org-code.component.ts | Adds org activities page (code/training/events tabs); currently has an unused import. |
| apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html | Org activities UI with admin-only tables vs employee banners. |
| apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.ts | Adds org CLA management page (static data). |
| apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.html | Org CLA UI (stats, alert, table). |
| apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.ts | Adds OSPO resources page (topic/resource cards + links). |
| apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.html | OSPO resources UI layout. |
| apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts | Updates meetings title based on activeLens. |
| apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html | Renders computed meetings page title. |
| apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts | Adds activeLens awareness to show “My LFX Overview” header. |
| apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html | Adds me-lens header and adjusts conditional header rendering. |
| apps/lfx-one/src/app/modules/dashboards/dashboard.component.ts | Adds activeLens awareness (used for me-lens gating). |
| apps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.html | Formatting change to <lfx-select> markup. |
| apps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.ts | Updates committee title based on activeLens. |
| apps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.html | Renders computed committee page title. |
| apps/lfx-one/src/app/layouts/main-layout/main-layout.component.ts | Reworks sidebar items per lens and wires lens switcher + project selector backdrop. |
| apps/lfx-one/src/app/layouts/main-layout/main-layout.component.html | Updates desktop/mobile layout to include lens switcher and new sidebar variants. |
| apps/lfx-one/src/app/layouts/dev-toolbar/dev-toolbar.component.ts | Adds org user type selector and ties toolbar visibility to AppService.showDevToolbar. |
| apps/lfx-one/src/app/layouts/dev-toolbar/dev-toolbar.component.html | Adds org user type selector UI and adjusts org selector visibility logic. |
| apps/lfx-one/src/app/app.routes.ts | Adds placeholder routes and lazy-loads the org lens routes under /org. |
| apps/lfx-one/public/images/logo_lfx_white.svg | Adds white LFX logo asset. |
| apps/lfx-one/public/images/logo_lf_white.svg | Adds white Linux Foundation logo asset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
4
to
8
| import { computed, inject, Injectable, Signal, signal, WritableSignal } from '@angular/core'; | ||
| import { isBoardScopedPersona, PersonaType } from '@lfx-one/shared/interfaces'; | ||
|
|
||
| import { AppService } from './app.service'; | ||
| import { ProjectContextService } from './project-context.service'; |
There was a problem hiding this comment.
AppService is imported but not used in this service. With the repo's ESLint setup, this will be flagged as an unused import; please remove it or use it (if persona changes are meant to update app-level lens state).
Comment on lines
+27
to
+33
| // Active lens state — defaults to 'me' (user at the center) | ||
| private readonly activeLensSignal = signal<Lens>('me'); | ||
| public readonly activeLens = this.activeLensSignal.asReadonly(); | ||
|
|
||
| // Project selector panel state | ||
| private readonly projectSelectorOpenSignal = signal(false); | ||
| public readonly projectSelectorOpen = this.projectSelectorOpenSignal.asReadonly(); |
There was a problem hiding this comment.
activeLens defaults to 'me', but there’s no syncing between the current URL (e.g. /org/*) and activeLens. Direct navigation/refresh on /org will leave activeLens === 'me', resulting in the wrong sidebar items and dev-toolbar selectors. Consider initializing/updating activeLens from router navigation (or persisting it) so route and lens stay consistent.
Comment on lines
+8
to
+10
| <!-- Sidebar - Desktop: lens switcher (72px) + nav panel (280px) = 352px total --> | ||
| <div class="hidden lg:flex flex-row flex-shrink-0 fixed left-0 bottom-0 w-[344px] z-[60]" [ngClass]="{ 'top-[48px]': showDevToolbar(), 'top-0': !showDevToolbar() }"> | ||
| <!-- Lens Switcher Column (72px) --> |
There was a problem hiding this comment.
The width math in the comment doesn’t match the actual Tailwind widths: the lens switcher column is w-[64px] and the wrapper is w-[344px], but the comment says 72px + 280px = 352px. This makes it harder to keep layout-aligned (and is likely related to the project selector panel using left: 352px). Please update the comment (and any dependent positioning) to the actual values.
Suggested change
| <!-- Sidebar - Desktop: lens switcher (72px) + nav panel (280px) = 352px total --> | |
| <div class="hidden lg:flex flex-row flex-shrink-0 fixed left-0 bottom-0 w-[344px] z-[60]" [ngClass]="{ 'top-[48px]': showDevToolbar(), 'top-0': !showDevToolbar() }"> | |
| <!-- Lens Switcher Column (72px) --> | |
| <!-- Sidebar - Desktop: lens switcher (64px) + nav panel (280px) = 344px total --> | |
| <div class="hidden lg:flex flex-row flex-shrink-0 fixed left-0 bottom-0 w-[344px] z-[60]" [ngClass]="{ 'top-[48px]': showDevToolbar(), 'top-0': !showDevToolbar() }"> | |
| <!-- Lens Switcher Column (64px) --> |
Comment on lines
+34
to
+38
| <div | ||
| class="fixed z-[60] w-[28rem] max-h-[480px] bg-white rounded-xl shadow-2xl border border-gray-200 flex flex-col overflow-hidden" | ||
| [style.top.px]="panelTop()" | ||
| [style.left]="'352px'" | ||
| data-testid="project-selector-panel"> |
There was a problem hiding this comment.
The selector panel uses a hard-coded left: 352px. This doesn’t match the actual desktop sidebar width (w-[344px] in main layout) and will also position the panel off-screen on mobile (the project selector is rendered in the mobile drawer too). Consider calculating left from the trigger element’s getBoundingClientRect() (e.g. rect.right + gap) and clamping to the viewport so it works across layouts/screen sizes.
Comment on lines
+163
to
+168
| public constructor() { | ||
| // Scroll sidebar to top whenever nav items change (e.g. on lens switch) | ||
| effect(() => { | ||
| this.items(); | ||
| this.scrollContainer?.nativeElement?.scrollTo({ top: 0, behavior: 'instant' }); | ||
| }); |
There was a problem hiding this comment.
scrollTo({ behavior: 'instant' }) is not a valid ScrollBehavior value (supported values are typically 'auto' and 'smooth'). This will be ignored in many browsers. Use 'auto' (or omit behavior) if you want an immediate jump-to-top.
|
|
||
| import { DecimalPipe } from '@angular/common'; | ||
| import { Component, computed, inject, signal } from '@angular/core'; | ||
| import { OrgUserType } from '@lfx-one/shared/interfaces'; |
There was a problem hiding this comment.
OrgUserType is imported but not used in this component. With the repo’s ESLint configuration, this will be reported as an unused import; please remove it or use it.
Suggested change
| import { OrgUserType } from '@lfx-one/shared/interfaces'; |
Comment on lines
+28
to
+38
| path: 'groups', | ||
| loadComponent: () => import('./groups/org-groups.component').then((m) => m.OrgGroupsComponent), | ||
| }, | ||
| { | ||
| path: 'cla', | ||
| loadComponent: () => import('./cla/org-cla.component').then((m) => m.OrgClaComponent), | ||
| }, | ||
| { | ||
| path: 'permissions', | ||
| loadComponent: () => import('./permissions/org-permissions.component').then((m) => m.OrgPermissionsComponent), | ||
| }, |
There was a problem hiding this comment.
PR description states the Org lens has 7 sections/pages, but routes here also include /org/cla and /org/permissions (2 additional pages) that aren’t listed. Please align the PR description with the implemented routes (or remove/feature-flag these routes if they’re not intended to ship yet).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (10)
apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.ts (1)
24-97: Consider de-duplicating presentation class fields in mock data.
typeClass/statusClassare repeated in every row; deriving them fromtypeandstatuswould reduce drift and make updates safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.ts` around lines 24 - 97, The mock data in the protected readonly agreements array duplicates presentation classes (typeClass and statusClass) per item; remove those hardcoded fields and instead compute them from the type and status values when rendering or when building the mock (e.g., add a small helper like getTypeClass(type) and getStatusClass(status) used by the OrgClaComponent template or component logic); update usages that reference agreements[].typeClass and agreements[].statusClass to call the new helpers so the visual classes are derived from agreements[].type and agreements[].status only.apps/lfx-one/src/app/modules/org-lens/org-lens.routes.ts (1)
6-6: Add JSDoc for exported route module constant.Please add a short JSDoc block above
ORG_LENS_ROUTESto document intent/ownership of this route tree.♻️ Suggested update
+/** + * Route definitions for the Organization lens area under `/org`. + */ export const ORG_LENS_ROUTES: Routes = [As per coding guidelines, "Add JSDoc comments for public functions and exported modules".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/org-lens.routes.ts` at line 6, Add a short JSDoc block immediately above the exported constant ORG_LENS_ROUTES describing the purpose of this route tree (what area of the app it serves), ownership/maintainer, and any notes about its public usage; include standard tags such as `@constant` (or `@module`), `@public`, and `@remarks` or `@owner` to make ownership clear, and keep the description to one or two sentences so readers can quickly understand intent and responsibility.apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html (1)
43-50: Consider consistent null handling for user() access.Line 44 uses
user()!(non-null assertion) while line 52 usesuser()?.name(optional chaining). Since both are within the same conditional block checkinguser()?.picture, the inconsistency is minor but could be unified for clarity.♻️ Suggested consistency fix
`@if` (user()?.picture) { - <img [src]="user()!.picture" [alt]="user()!.name" class="size-full object-cover" /> + <img [src]="user()?.picture" [alt]="user()?.name" class="size-full object-cover" /> } `@else` {Or alternatively, use non-null assertions consistently when inside the guard:
<div class="flex flex-col gap-1 grow min-w-0"> - <p class="font-semibold text-base text-gray-900 leading-5 tracking-tight line-clamp-2">{{ user()?.name }}</p> + <p class="font-semibold text-base text-gray-900 leading-5 tracking-tight line-clamp-2">{{ user()!.name }}</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html` around lines 43 - 50, The template mixes non-null assertions and optional chaining when accessing user() inside the same guard; update the bindings in the image element to use optional chaining consistently (replace user()! with user()?) so both [src] and [alt] use user()?.picture and user()?.name (the conditional remains `@if` (user()?.picture)); this keeps null-safe access consistent with userInitials() usage.apps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.ts (1)
148-159: Use the union type fromProjectInfluencefor type safety.The
levelClassmethod acceptsstringbut thelevelproperty onProjectInfluenceis already a union type. Using the specific type prevents invalid level values from being passed.♻️ Proposed fix
- protected levelClass(level: string): string { + protected levelClass(level: ProjectInfluence['level']): string { switch (level) { case 'Leading': return 'bg-green-50 text-green-700 border border-green-200'; case 'Contributing': return 'bg-blue-50 text-blue-700 border border-blue-200'; case 'Participating': return 'bg-amber-50 text-amber-700 border border-amber-200'; default: return 'bg-slate-100 text-slate-500'; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.ts` around lines 148 - 159, Change the levelClass method signature to accept the exact union type used by ProjectInfluence.level instead of a plain string: update the parameter type of levelClass (in org-overview.component.ts) to the ProjectInfluence level union (import ProjectInfluence or its type alias as needed) so callers are type-checked against valid levels; keep the switch cases and return values unchanged but ensure the method signature references the ProjectInfluence level union for type safety.apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts (2)
46-79: Consider extracting duplicate initials computation logic.Both
orgInitialsanduserInitialsshare nearly identical logic for computing initials from a name string. Consider extracting this to a reusable helper function.♻️ Proposed refactor to reduce duplication
+ private computeInitials(name: string | undefined | null): string { + if (!name) return '?'; + return name + .split(' ') + .map((n: string) => n[0]) + .filter(Boolean) + .slice(0, 2) + .join('') + .toUpperCase(); + } + protected readonly orgInitials = computed(() => { - const name = this.selectedAccount().accountName; - if (!name) return '?'; - return name - .split(' ') - .map((n: string) => n[0]) - .filter(Boolean) - .slice(0, 2) - .join('') - .toUpperCase(); + return this.computeInitials(this.selectedAccount().accountName); }); // ... orgAvatarColor stays the same ... protected readonly userInitials = computed(() => { - const u: User | null = this.user(); - if (!u?.name) return '?'; - return u.name - .split(' ') - .map((n: string) => n[0]) - .slice(0, 2) - .join('') - .toUpperCase(); + return this.computeInitials(this.user()?.name); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts` around lines 46 - 79, Both orgInitials and userInitials duplicate the same initials-extraction logic; extract that logic into a shared helper (e.g., a function getInitials(name: string | null, maxChars = 2): string) and replace computed bodies to call it. Update orgInitials to call getInitials(this.selectedAccount().accountName) and userInitials to call getInitials(u?.name) (keep fallback '?' behavior), and ensure the helper handles null/empty names, splitting, taking first letters, slicing to the max count, joining and uppercasing; leave orgAvatarColor and other code unchanged.
68-68: Redundant type cast onasReadonly()return value.The
asReadonly()method already returns a properly typed readonly signal; theas ReturnType<...>cast is unnecessary.♻️ Simplified declaration
- protected readonly user = this.userService.user.asReadonly() as ReturnType<typeof this.userService.user.asReadonly>; + protected readonly user = this.userService.user.asReadonly();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts` at line 68, The declaration of the protected property 'user' redundantly casts the result of this.userService.user.asReadonly() to ReturnType<typeof this.userService.user.asReadonly>; remove the unnecessary type assertion and simply assign the readonly signal directly (i.e., replace the current initialization of the 'user' property in SidebarComponent with a direct assignment from this.userService.user.asReadonly()) so the type inferred from asReadonly() is used without the extra cast.apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html (2)
362-383: Nested@forloops with repeated filtering could be optimized.The template iterates over
detailOnboardingStepsfor each category, filtering bystep.category === category. Consider pre-grouping steps by category in the component for cleaner template logic.♻️ Pre-group onboarding steps by category
In
org-membership.component.ts:protected readonly onboardingByCategory = computed(() => { const grouped: Record<string, OnboardingStep[]> = {}; for (const step of this.detailOnboardingSteps) { (grouped[step.category] ??= []).push(step); } return grouped; });Then simplify template:
`@for` (category of ['Documentation', 'Key Contacts', 'Board & Committees', 'Communications']; track category) { <div class="bg-white rounded-xl border border-gray-200 p-5"> <div class="text-sm font-semibold text-slate-700 mb-3">{{ category }}</div> <div class="flex flex-col gap-2"> `@for` (step of onboardingByCategory()[category]; track step.label) { <!-- step rendering --> } </div> </div> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html` around lines 362 - 383, The template currently loops over detailOnboardingSteps for each category causing repeated filtering; move the grouping logic into the component by adding a computed property (e.g., onboardingByCategory) that builds a Record<string, OnboardingStep[]> keyed by step.category from detailOnboardingSteps, then update the template loops to iterate onboardingByCategory()[category] instead of filtering detailOnboardingSteps each time; keep existing rendering logic (including canEdit() checks) but replace the inner `@for` source with the grouped collection to improve clarity and performance.
192-206: Consider moving tab definitions to the component.The inline tab array definition works but duplicates what could be a component property, improving testability and reducing template complexity.
♻️ Move to component
In
org-membership.component.ts:protected readonly detailTabs = [ {id: 'docs', label: 'Documentation', icon: 'fa-light fa-file-lines'}, {id: 'contacts', label: 'Key Contacts', icon: 'fa-light fa-address-book'}, {id: 'board', label: 'Board & Committees', icon: 'fa-light fa-people-roof'}, {id: 'onboarding', label: 'Onboarding', icon: 'fa-light fa-list-check'}, {id: 'roi', label: 'ROI', icon: 'fa-light fa-chart-line'} ];Then in template:
`@for` (tab of detailTabs; track tab.id) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html` around lines 192 - 206, The template currently in org-membership.component.html defines the tab array inline; move that array into the component class by adding a protected readonly property (e.g., detailTabs) in org-membership.component.ts containing the five tab objects, then update the template loop to iterate over detailTabs (use the same track tab.id and existing bindings like setDetailTab, activeDetailTab, and tab.icon/tab.label) so the data lives on the component for easier testing and reduced template complexity.apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.ts (1)
73-81: MultipleonDestroycallbacks registered on repeated toggle.Each call to
attachOutsideClickListener()registers a newdestroyRef.onDestroy()callback. While the callbacks are effectively no-ops afterdetachOutsideClickListener()nulls the listener, this creates unnecessary callback accumulation. Consider moving the cleanup registration to the constructor.♻️ Register cleanup once in constructor
+ public constructor() { + this.destroyRef.onDestroy(() => this.detachOutsideClickListener()); + } + private attachOutsideClickListener(): void { this.outsideClickListener = (event: MouseEvent) => { if (!this.elementRef.nativeElement.contains(event.target as Node)) { this.closePanel(); } }; document.addEventListener('click', this.outsideClickListener); - this.destroyRef.onDestroy(() => this.detachOutsideClickListener()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.ts` around lines 73 - 81, The attachOutsideClickListener function currently calls this.destroyRef.onDestroy(() => this.detachOutsideClickListener()) each time it's invoked, causing multiple onDestroy callbacks to accumulate; move the one-time registration into the component constructor so the cleanup is registered only once: add a single this.destroyRef.onDestroy(() => this.detachOutsideClickListener()) in the constructor (or an initialization block) and remove the onDestroy registration from attachOutsideClickListener, keeping attachOutsideClickListener responsible only for setting this.outsideClickListener and document.addEventListener, and ensure detachOutsideClickListener still nulls and removes the listener.apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.ts (1)
277-279: Type assertion bypasses compile-time safety.Casting
tabfromstringtoMembershipDetailTabwithout validation could allow invalid tab values. Consider validating or using a type guard.♻️ Add type guard for tab validation
+ private readonly validDetailTabs = new Set<MembershipDetailTab>(['docs', 'contacts', 'board', 'onboarding', 'roi']); + protected setDetailTab(tab: string): void { - this.activeDetailTab.set(tab as MembershipDetailTab); + if (this.validDetailTabs.has(tab as MembershipDetailTab)) { + this.activeDetailTab.set(tab as MembershipDetailTab); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.ts` around lines 277 - 279, The setDetailTab method is unsafely casting a string to MembershipDetailTab; replace the direct assertion in setDetailTab and validate the incoming tab before setting activeDetailTab by implementing a type guard (e.g., isMembershipDetailTab) or checking against a known enum/array of allowed MembershipDetailTab values, and only call this.activeDetailTab.set(validTab) when validation succeeds (handle or log invalid values otherwise).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/lfx-one/src/app/app.routes.ts`:
- Around line 62-65: The ORG_LENS_ROUTES currently rely on component-level role
checks (membership, code, groups, profile) so add a route guard to enforce
admin-only access at the routing layer: implement an OrgAdminGuard (implementing
CanActivate) that checks the current user's org role via your auth/user service
and returns true only for org admins, register it in the org-lens module
providers, and add canActivate: [OrgAdminGuard] to the sensitive child route
entries inside ORG_LENS_ROUTES (membership, groups, profile, code or any
admin-only paths) so unauthorized users are blocked before the component loads.
In `@apps/lfx-one/src/app/layouts/main-layout/main-layout.component.ts`:
- Around line 35-37: The component does not initialize activeLens from the
current route or update it on navigation, so deep links render the wrong
sidebar; update the initialization and NavigationEnd handling in
main-layout.component.ts to derive lens from the URL and call
appService.setLens(...) accordingly: on component init (where activeLens is
declared/used) read the initial route path and call setLens('org') for paths
matching /org/**, setLens('foundation') for /foundation/** (fallback to existing
behavior for 'me'), and extend the existing Router NavigationEnd subscription to
parse the new URL each navigation and call appService.setLens(...) to keep the
appService.activeLens in sync with the route. Ensure you reference the
appService.setLens method and the current NavigationEnd handler when making
changes.
In `@apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.html`:
- Around line 22-27: The hero CTA anchors using href="#" (the two <a> elements
containing "OSPO 101 Guide" and "TODO Group") are placeholders and should be
removed or replaced: either wire them to real URLs or Angular router links
(e.g., [routerLink]="/path") or hide the CTA until a valid resource exists; also
replace the "TODO Group" label with the actual resource name or remove that
anchor; apply the same fixes for the topic card anchors referenced around lines
52-54 so no visible links use href="#" or placeholder text.
In `@apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.ts`:
- Around line 69-76: The externalLinks array in org-benefits component uses
placeholder urls ('#'); update the entries in the externalLinks constant to
either real URLs or clear TODO placeholders (e.g., a TODO_URL constant or null)
so consumers know they are incomplete; locate the externalLinks definition
(protected readonly externalLinks) in OrgBenefitsComponent and replace each url:
'#' with the correct target URLs or a documented placeholder and, if choosing
null/placeholder, ensure any template or link-rendering code gracefully handles
non-URL values.
In `@apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.html`:
- Around line 68-75: The Renew/Sign buttons are shown based only on
agreement.status (agreement.status === 'Expired' / 'Pending') which exposes
write actions to view-only roles; update the template to also check the
component's canEdit() guard before rendering those buttons so only editable
roles see them (combine agreement.status checks with a canEdit() condition in
the same conditional blocks that render the buttons), locating the checks around
the existing agreement.status conditionals in org-cla.component.html and using
the canEdit() method from the component class.
In `@apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html`:
- Around line 169-172: The search input lacks data binding and filtering logic;
bind it to a component property (e.g., add [(ngModel)]="searchQuery" or
(ngModelChange)="onSearchChange($event)" on the input in
org-code.component.html), add a searchQuery string property to the
OrgCodeComponent class, and implement a filter method (e.g., onSearchChange or
filterTrainingRecords) or a computed getter (e.g., get filteredTrainings()) that
returns the training/employee list filtered by searchQuery; also ensure the
containing module imports FormsModule so ngModel works. Ensure the template uses
the filtered array (filteredTrainings) wherever the list is rendered instead of
the raw array.
- Around line 243-247: The template buttons lack click handlers and a reactive
signal to track the selected filter; update the OrgCodeComponent class to add a
signal (e.g., selectedFilter = signal<'all'|'past'|'upcoming'>('all')) and a
setter method setFilter(value: 'all'|'past'|'upcoming') that updates the signal,
then replace the static "All" styling in org-code.component.html with
(click)="setFilter('all')" / (click)="setFilter('past')" /
(click)="setFilter('upcoming')" and bind active styling via [ngClass] or
class.active using selectedFilter() (or a computed signal) to conditionally
apply the active classes; also use a computed filteredCodes (or filteredList)
signal that derives from selectedFilter to actually filter the displayed items.
In `@apps/lfx-one/src/app/modules/org-lens/code/org-code.component.ts`:
- Around line 83-89: The topContributors seed uses hard-coded relative labels
like 'Today'/'Yesterday' and the events fixtures use upcoming dates; replace
these hard-coded strings with absolute date fields (e.g., ISO date strings) in
the CodeContributor objects (topContributors) and in the events fixture, remove
any relative or future dates and set real past ISO dates; then import and use
the shared utilities (direct imports from `@lfx-one/shared/utils` —
getRelativeDate and formatDate) in org-code.component.ts to derive the display
string at render time (e.g., convert the contributor.lastActive ISO date to a
relative label with getRelativeDate or formatDate when rendering). Also update
the similar fixture block referenced (lines ~114-120) to follow the same pattern
so all display labels are computed at render time rather than stored in the
fixtures.
In `@apps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.ts`:
- Around line 105-107: searchCorrection() currently ignores the user's input and
always sets correctionSearched, causing empty queries to appear successful;
update searchCorrection to read the entered query from the correctionSearch
control (e.g., this.correctionSearch or this.correctionSearch.value),
trim/validate it, and if empty set correctionSearched to false and return;
otherwise use that query to perform the correction lookup (trigger the existing
lookup logic) and then set correctionSearched to true so the template reflects
real results.
In
`@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html`:
- Around line 147-149: The anchor element that renders the "Learn More" action
(the <a> with class "flex-shrink-0 px-3 py-1.5 border border-blue-200
text-blue-600 text-xs font-semibold rounded-lg hover:bg-blue-50 cursor-pointer")
uses href="#" which causes a page jump and is not accessible; replace it with
either a semantic <button> (if it opens a modal) wired to the component method
(e.g., (click)="openLearnMoreModal()" in OrgMembershipComponent) or an Angular
router link (e.g., [routerLink]="['/desired/path']") if it navigates, and add
appropriate ARIA attributes (role/aria-label) to keep the same styling and
keyboard accessibility.
In
`@apps/lfx-one/src/app/modules/org-lens/permissions/org-permissions.component.html`:
- Around line 40-43: The search input is currently unwired so typing does
nothing; add a searchTerm property and either bind the input to it (e.g.,
[(ngModel)]="searchTerm") or call an handler (e.g.,
(input)="onSearch($event.target.value)"), implement onSearch(value: string) or
filterUsers() in the OrgPermissionsComponent to compute a filteredUsers array
from the existing users array, and change the template's *ngFor to iterate over
filteredUsers (or use a filter pipe) instead of users; reference the input
element, the users array, filteredUsers, searchTerm, and onSearch/filterUsers in
your changes.
- Around line 10-13: The Invite button and other mutation controls in
org-permissions.component.html are rendered unconditionally; wrap these
write-affordance elements with the component's edit permission guard (e.g.
*ngIf="canEdit()" or an equivalent boolean like canEdit()) so only users with
edit rights see them; apply the same guard to the other write controls
referenced (lines ~84-86) to enforce the org's canEdit() visibility model.
In `@apps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.html`:
- Around line 79-96: Replace the placeholder anchors so they open the real URLs:
bind the anchor hrefs to profile.website and profile.linkedIn (instead of "#"),
add target="_blank" and rel="noopener noreferrer" for external links, and only
render the anchor (or fall back to plain text) when
profile.website/profile.linkedIn exists (use *ngIf or equivalent). If your
stored URLs may omit a scheme, normalize them before binding (e.g., ensure
"https://" prefix via your existing utility or a small helper) to avoid broken
links.
- Around line 190-210: The Link Org button and each Unlink button inside the
conglomerate block are visible to view-only users; wrap the Link Org button and
the per-org Unlink button rendering in the same edit guard used elsewhere by
checking canEdit() (the same condition used in other templates) so they only
render when canEdit() returns true; locate the conglomerate block driven by
isConglomerate(), iterate over linkedOrgs, and conditionally render the Link Org
control and the Unlink control based on canEdit() to prevent showing mutation
actions to read-only users.
In
`@apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.html`:
- Around line 34-38: The panel currently uses a hardcoded left offset ('352px');
instead, anchor it to the trigger element by computing its left position at
render/open. Add or use an anchor reference (e.g., the trigger element queried
via `@ViewChild` or an `@Input` anchor element) and replace the hardcoded left with
a method like panelLeft() analogous to panelTop() that calls
anchor.getBoundingClientRect().left (or center) and clamps the value to the
viewport width so the panel never overflows; update the template to bind
[style.left.px]="panelLeft()" and ensure the logic lives next to panelTop() and
uses the same panel sizing/viewport constraints.
- Around line 7-10: The trigger and option rows are non-semantic clickable divs
and the panel uses a hardcoded left offset; change the trigger element with id
"#selectorTrigger" and each project row to semantic <button> elements (preserve
classes, data-testid="project-selector", (click)="togglePanel()" and any ARIA
attributes) so they are keyboard-focusable and activatable, and update any
(keydown) handlers to handle Enter/Space if present; remove the hardcoded
[style.left]="'352px'" on the panel and instead compute the panel's left offset
in the ProjectSelectorComponent (use the selectorTrigger element reference and
its getBoundingClientRect() inside the togglePanel() or an updatePanelPosition()
method to set a component property like panelLeft which the template binds as
[style.left]="panelLeft + 'px'"). Ensure project rows use the same button change
and that focus/aria roles are preserved when toggling the panel.
In `@apps/lfx-one/src/app/shared/services/app.service.ts`:
- Around line 35-37: The orgUserTypeSignal currently defaults to an elevated
role ('admin-edit'), causing privileged UI to flash before real role data loads;
change its initial value to a least-privilege fallback (e.g., 'view-only' or a
neutral value) and ensure the real role from session/org data hydrates
orgUserTypeSignal as soon as the session loader (where org role is retrieved)
completes; update code paths that read orgUserTypeSignal (orgUserType,
isAdmin(), canEdit()) to rely on this safe default until the hydrate call
replaces it.
In `@packages/shared/src/constants/persona.constants.ts`:
- Around line 6-11: The JSDoc comment "Persona options available for user
selection" is misaligned above ORG_USER_TYPE_OPTIONS; move or reassign the
docblock so it sits directly above the correct symbol (e.g., PERSONA_OPTIONS or
whichever constant represents user-selectable personas) and ensure the
ORG_USER_TYPE_OPTIONS block has its own accurate comment "Organization lens user
type options" immediately above it; update the two JSDoc blocks so each
documents the correct exported constant name (ORG_USER_TYPE_OPTIONS and the
persona options constant) to avoid misleading documentation.
---
Nitpick comments:
In `@apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.ts`:
- Around line 24-97: The mock data in the protected readonly agreements array
duplicates presentation classes (typeClass and statusClass) per item; remove
those hardcoded fields and instead compute them from the type and status values
when rendering or when building the mock (e.g., add a small helper like
getTypeClass(type) and getStatusClass(status) used by the OrgClaComponent
template or component logic); update usages that reference
agreements[].typeClass and agreements[].statusClass to call the new helpers so
the visual classes are derived from agreements[].type and agreements[].status
only.
In
`@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html`:
- Around line 362-383: The template currently loops over detailOnboardingSteps
for each category causing repeated filtering; move the grouping logic into the
component by adding a computed property (e.g., onboardingByCategory) that builds
a Record<string, OnboardingStep[]> keyed by step.category from
detailOnboardingSteps, then update the template loops to iterate
onboardingByCategory()[category] instead of filtering detailOnboardingSteps each
time; keep existing rendering logic (including canEdit() checks) but replace the
inner `@for` source with the grouped collection to improve clarity and
performance.
- Around line 192-206: The template currently in org-membership.component.html
defines the tab array inline; move that array into the component class by adding
a protected readonly property (e.g., detailTabs) in org-membership.component.ts
containing the five tab objects, then update the template loop to iterate over
detailTabs (use the same track tab.id and existing bindings like setDetailTab,
activeDetailTab, and tab.icon/tab.label) so the data lives on the component for
easier testing and reduced template complexity.
In
`@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.ts`:
- Around line 277-279: The setDetailTab method is unsafely casting a string to
MembershipDetailTab; replace the direct assertion in setDetailTab and validate
the incoming tab before setting activeDetailTab by implementing a type guard
(e.g., isMembershipDetailTab) or checking against a known enum/array of allowed
MembershipDetailTab values, and only call this.activeDetailTab.set(validTab)
when validation succeeds (handle or log invalid values otherwise).
In `@apps/lfx-one/src/app/modules/org-lens/org-lens.routes.ts`:
- Line 6: Add a short JSDoc block immediately above the exported constant
ORG_LENS_ROUTES describing the purpose of this route tree (what area of the app
it serves), ownership/maintainer, and any notes about its public usage; include
standard tags such as `@constant` (or `@module`), `@public`, and `@remarks` or `@owner` to
make ownership clear, and keep the description to one or two sentences so
readers can quickly understand intent and responsibility.
In `@apps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.ts`:
- Around line 148-159: Change the levelClass method signature to accept the
exact union type used by ProjectInfluence.level instead of a plain string:
update the parameter type of levelClass (in org-overview.component.ts) to the
ProjectInfluence level union (import ProjectInfluence or its type alias as
needed) so callers are type-checked against valid levels; keep the switch cases
and return values unchanged but ensure the method signature references the
ProjectInfluence level union for type safety.
In
`@apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.ts`:
- Around line 73-81: The attachOutsideClickListener function currently calls
this.destroyRef.onDestroy(() => this.detachOutsideClickListener()) each time
it's invoked, causing multiple onDestroy callbacks to accumulate; move the
one-time registration into the component constructor so the cleanup is
registered only once: add a single this.destroyRef.onDestroy(() =>
this.detachOutsideClickListener()) in the constructor (or an initialization
block) and remove the onDestroy registration from attachOutsideClickListener,
keeping attachOutsideClickListener responsible only for setting
this.outsideClickListener and document.addEventListener, and ensure
detachOutsideClickListener still nulls and removes the listener.
In `@apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html`:
- Around line 43-50: The template mixes non-null assertions and optional
chaining when accessing user() inside the same guard; update the bindings in the
image element to use optional chaining consistently (replace user()! with
user()?) so both [src] and [alt] use user()?.picture and user()?.name (the
conditional remains `@if` (user()?.picture)); this keeps null-safe access
consistent with userInitials() usage.
In `@apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts`:
- Around line 46-79: Both orgInitials and userInitials duplicate the same
initials-extraction logic; extract that logic into a shared helper (e.g., a
function getInitials(name: string | null, maxChars = 2): string) and replace
computed bodies to call it. Update orgInitials to call
getInitials(this.selectedAccount().accountName) and userInitials to call
getInitials(u?.name) (keep fallback '?' behavior), and ensure the helper handles
null/empty names, splitting, taking first letters, slicing to the max count,
joining and uppercasing; leave orgAvatarColor and other code unchanged.
- Line 68: The declaration of the protected property 'user' redundantly casts
the result of this.userService.user.asReadonly() to ReturnType<typeof
this.userService.user.asReadonly>; remove the unnecessary type assertion and
simply assign the readonly signal directly (i.e., replace the current
initialization of the 'user' property in SidebarComponent with a direct
assignment from this.userService.user.asReadonly()) so the type inferred from
asReadonly() is used without the extra cast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25bb03d2-21cf-436e-9704-5af081c62772
⛔ Files ignored due to path filters (3)
apps/lfx-one/public/images/logo_lf_white.svgis excluded by!**/*.svgapps/lfx-one/public/images/logo_lfx_white.svgis excluded by!**/*.svgapps/lfx-one/public/images/org-placeholder-logo.pngis excluded by!**/*.png
📒 Files selected for processing (46)
apps/lfx-one/src/app/app.routes.tsapps/lfx-one/src/app/layouts/dev-toolbar/dev-toolbar.component.htmlapps/lfx-one/src/app/layouts/dev-toolbar/dev-toolbar.component.tsapps/lfx-one/src/app/layouts/main-layout/main-layout.component.htmlapps/lfx-one/src/app/layouts/main-layout/main-layout.component.tsapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.htmlapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-one/src/app/modules/committees/components/committee-members/committee-members.component.htmlapps/lfx-one/src/app/modules/dashboards/dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.htmlapps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.tsapps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.htmlapps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.tsapps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.htmlapps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.tsapps/lfx-one/src/app/modules/org-lens/code/org-code.component.htmlapps/lfx-one/src/app/modules/org-lens/code/org-code.component.tsapps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.htmlapps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.tsapps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.htmlapps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.tsapps/lfx-one/src/app/modules/org-lens/org-lens.routes.tsapps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.htmlapps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.tsapps/lfx-one/src/app/modules/org-lens/permissions/org-permissions.component.htmlapps/lfx-one/src/app/modules/org-lens/permissions/org-permissions.component.tsapps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.htmlapps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.tsapps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.htmlapps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.tsapps/lfx-one/src/app/shared/components/lens-switcher/lens-switcher.component.htmlapps/lfx-one/src/app/shared/components/lens-switcher/lens-switcher.component.scssapps/lfx-one/src/app/shared/components/lens-switcher/lens-switcher.component.tsapps/lfx-one/src/app/shared/components/placeholder-page/placeholder-page.component.htmlapps/lfx-one/src/app/shared/components/placeholder-page/placeholder-page.component.tsapps/lfx-one/src/app/shared/components/project-selector/project-selector.component.htmlapps/lfx-one/src/app/shared/components/project-selector/project-selector.component.scssapps/lfx-one/src/app/shared/components/project-selector/project-selector.component.tsapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.htmlapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.tsapps/lfx-one/src/app/shared/services/app.service.tsapps/lfx-one/src/app/shared/services/persona.service.tspackages/shared/src/constants/persona.constants.tspackages/shared/src/interfaces/persona.interface.ts
Comment on lines
+62
to
+65
| { | ||
| path: 'org', | ||
| loadChildren: () => import('./modules/org-lens/org-lens.routes').then((m) => m.ORG_LENS_ROUTES), | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -p 'org-lens.routes.ts' apps/lfx-one/src/app/modules/org-lens | while read -r file; do
echo "== $file =="
sed -n '1,220p' "$file"
done
echo
rg -nP --type=ts 'canActivate|canMatch|orgUserType|employee|admin-edit|conglomerate-admin' \
apps/lfx-one/src/app/modules/org-lens \
apps/lfx-one/src/app/shared \
apps/lfx-one/src/app/layoutsRepository: linuxfoundation/lfx-v2-ui
Length of output: 7444
Add route-level guards to admin-only org child paths.
The ORG_LENS_ROUTES lack canActivate guards on sensitive routes. Access control is currently implemented at the component level (orgUserType computed properties in membership, code, groups, and profile components), which only hides UI features but does not prevent the route from activating. Routes such as membership, groups, and profile are directly reachable by any authenticated user without role validation at the routing layer.
Move role checks to route guards using canActivate to prevent unauthorized access before the component loads.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/app.routes.ts` around lines 62 - 65, The ORG_LENS_ROUTES
currently rely on component-level role checks (membership, code, groups,
profile) so add a route guard to enforce admin-only access at the routing layer:
implement an OrgAdminGuard (implementing CanActivate) that checks the current
user's org role via your auth/user service and returns true only for org admins,
register it in the org-lens module providers, and add canActivate:
[OrgAdminGuard] to the sensitive child route entries inside ORG_LENS_ROUTES
(membership, groups, profile, code or any admin-only paths) so unauthorized
users are blocked before the component loads.
Comment on lines
+35
to
+37
| protected readonly activeLens = this.appService.activeLens; | ||
| protected readonly projectSelectorOpen = this.appService.projectSelectorOpen; | ||
| protected readonly showDevToolbar = this.appService.showDevToolbar; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -p 'lens-switcher.component.ts' apps/lfx-one/src/app | while read -r file; do
echo "== $file =="
sed -n '1,220p' "$file"
done
echo
echo "== setLens call sites =="
rg -nP --type=ts '\bsetLens\s*\(' apps/lfx-one/src/app
echo
echo "== route/lens sync candidates =="
rg -nP --type=ts 'NavigationEnd|router\.url|urlAfterRedirects|activeLens' apps/lfx-one/src/appRepository: linuxfoundation/lfx-v2-ui
Length of output: 7010
🏁 Script executed:
cat -n apps/lfx-one/src/app/layouts/main-layout/main-layout.component.tsRepository: linuxfoundation/lfx-v2-ui
Length of output: 9269
🏁 Script executed:
cat -n apps/lfx-one/src/app/shared/services/app.service.tsRepository: linuxfoundation/lfx-v2-ui
Length of output: 2980
🏁 Script executed:
fd -p 'routes\.ts|app\.config' apps/lfx-one/src/app | head -20 | xargs -I {} sh -c 'echo "== {} ==" && head -100 {}'Repository: linuxfoundation/lfx-v2-ui
Length of output: 14797
Add route-based lens initialization to sync activeLens with URL on load and navigation.
Deep linking to /org/** or /foundation/** routes renders the wrong sidebar because activeLens defaults to 'me' and is only set via user interaction with lens-switcher. The NavigationEnd handler here only closes overlays; it does not restore the lens from the current URL. When users reload at /org/projects, they see the org page with the me-lens sidebar, creating a mismatch.
Routes should establish the matching lens on startup and after navigation (e.g., /org/** → call setLens('org'), /foundation/** → call setLens('foundation')).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/layouts/main-layout/main-layout.component.ts` around
lines 35 - 37, The component does not initialize activeLens from the current
route or update it on navigation, so deep links render the wrong sidebar; update
the initialization and NavigationEnd handling in main-layout.component.ts to
derive lens from the URL and call appService.setLens(...) accordingly: on
component init (where activeLens is declared/used) read the initial route path
and call setLens('org') for paths matching /org/**, setLens('foundation') for
/foundation/** (fallback to existing behavior for 'me'), and extend the existing
Router NavigationEnd subscription to parse the new URL each navigation and call
appService.setLens(...) to keep the appService.activeLens in sync with the
route. Ensure you reference the appService.setLens method and the current
NavigationEnd handler when making changes.
apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.html
Outdated
Show resolved
Hide resolved
Comment on lines
+69
to
+76
| protected readonly externalLinks = [ | ||
| { label: 'TODO Group', description: 'Industry community for OSPOs', url: '#' }, | ||
| { label: 'LF Research', description: 'Open source industry reports', url: '#' }, | ||
| { label: 'OpenSSF Best Practices', description: 'Security best practices for OSS', url: '#' }, | ||
| { label: 'SPDX License List', description: 'Standard OSS license identifiers', url: '#' }, | ||
| { label: 'LF Training Catalog', description: 'Open source courses & certifications', url: '#' }, | ||
| { label: 'LF Events Calendar', description: 'Upcoming open source events', url: '#' }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
External links have placeholder URLs.
All externalLinks entries use url: '#' which will not navigate anywhere useful. Consider adding real URLs or marking these as TODOs.
Would you like me to open an issue to track adding the real external URLs?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.ts`
around lines 69 - 76, The externalLinks array in org-benefits component uses
placeholder urls ('#'); update the entries in the externalLinks constant to
either real URLs or clear TODO placeholders (e.g., a TODO_URL constant or null)
so consumers know they are incomplete; locate the externalLinks definition
(protected readonly externalLinks) in OrgBenefitsComponent and replace each url:
'#' with the correct target URLs or a documented placeholder and, if choosing
null/placeholder, ensure any template or link-rendering code gracefully handles
non-URL values.
Comment on lines
+68
to
+75
| <div class="flex items-center gap-2"> | ||
| <span class="text-xs font-semibold px-2 py-0.5 rounded {{ agreement.statusClass }}">{{ agreement.status }}</span> | ||
| @if (agreement.status === 'Expired') { | ||
| <button class="text-xs text-blue-600 font-medium hover:text-blue-700 cursor-pointer">Renew</button> | ||
| } | ||
| @if (agreement.status === 'Pending') { | ||
| <button class="text-xs text-blue-600 font-medium hover:text-blue-700 cursor-pointer">Sign</button> | ||
| } |
Contributor
There was a problem hiding this comment.
Hide Renew/Sign for view-only org roles.
This block is keyed only off agreement.status, so employee and admin-view-only users still see write affordances. It needs the same canEdit() guard the rest of the org lens uses.
Suggested guard
- `@if` (agreement.status === 'Expired') {
- <button class="text-xs text-blue-600 font-medium hover:text-blue-700 cursor-pointer">Renew</button>
+ `@if` (canEdit() && agreement.status === 'Expired') {
+ <button type="button" class="text-xs text-blue-600 font-medium hover:text-blue-700 cursor-pointer">Renew</button>
}
- `@if` (agreement.status === 'Pending') {
- <button class="text-xs text-blue-600 font-medium hover:text-blue-700 cursor-pointer">Sign</button>
+ `@if` (canEdit() && agreement.status === 'Pending') {
+ <button type="button" class="text-xs text-blue-600 font-medium hover:text-blue-700 cursor-pointer">Sign</button>
}📝 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
| <div class="flex items-center gap-2"> | |
| <span class="text-xs font-semibold px-2 py-0.5 rounded {{ agreement.statusClass }}">{{ agreement.status }}</span> | |
| @if (agreement.status === 'Expired') { | |
| <button class="text-xs text-blue-600 font-medium hover:text-blue-700 cursor-pointer">Renew</button> | |
| } | |
| @if (agreement.status === 'Pending') { | |
| <button class="text-xs text-blue-600 font-medium hover:text-blue-700 cursor-pointer">Sign</button> | |
| } | |
| <div class="flex items-center gap-2"> | |
| <span class="text-xs font-semibold px-2 py-0.5 rounded {{ agreement.statusClass }}">{{ agreement.status }}</span> | |
| `@if` (canEdit() && agreement.status === 'Expired') { | |
| <button type="button" class="text-xs text-blue-600 font-medium hover:text-blue-700 cursor-pointer">Renew</button> | |
| } | |
| `@if` (canEdit() && agreement.status === 'Pending') { | |
| <button type="button" class="text-xs text-blue-600 font-medium hover:text-blue-700 cursor-pointer">Sign</button> | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.html` around
lines 68 - 75, The Renew/Sign buttons are shown based only on agreement.status
(agreement.status === 'Expired' / 'Pending') which exposes write actions to
view-only roles; update the template to also check the component's canEdit()
guard before rendering those buttons so only editable roles see them (combine
agreement.status checks with a canEdit() condition in the same conditional
blocks that render the buttons), locating the checks around the existing
agreement.status conditionals in org-cla.component.html and using the canEdit()
method from the component class.
Comment on lines
+190
to
+210
| @if (isConglomerate()) { | ||
| <div class="bg-white rounded-xl border border-gray-200 overflow-hidden" data-testid="conglomerate-structure"> | ||
| <div class="px-6 py-4 border-b border-gray-100 flex items-center justify-between"> | ||
| <div> | ||
| <div class="text-sm font-semibold text-slate-700">Conglomerate Structure</div> | ||
| <div class="text-xs text-slate-400 mt-0.5">Parent and linked subsidiary organizations sharing memberships.</div> | ||
| </div> | ||
| <button class="flex items-center gap-1.5 px-3 py-1.5 bg-blue-600 text-white text-xs font-medium rounded-lg hover:bg-blue-700 cursor-pointer"> | ||
| <i class="fa-light fa-plus"></i> Link Org | ||
| </button> | ||
| </div> | ||
| <div class="flex flex-col divide-y divide-slate-50"> | ||
| @for (org of linkedOrgs; track org.name) { | ||
| <div class="flex items-center gap-4 px-6 py-3.5"> | ||
| <div class="w-9 h-9 rounded-full bg-indigo-100 text-indigo-700 flex items-center justify-center text-sm font-bold flex-shrink-0">{{ org.initials }}</div> | ||
| <div class="flex-1 min-w-0"> | ||
| <div class="text-sm font-medium text-slate-900">{{ org.name }}</div> | ||
| <div class="text-xs text-slate-400">{{ org.memberships }} memberships</div> | ||
| </div> | ||
| <span class="text-xs px-2 py-0.5 rounded font-medium" [class]="org.relationship === 'Parent' ? 'bg-purple-50 text-purple-700 border border-purple-200' : 'bg-slate-100 text-slate-600'">{{ org.relationship }}</span> | ||
| <button class="text-xs text-red-400 hover:text-red-600 cursor-pointer ml-2">Unlink</button> |
Contributor
There was a problem hiding this comment.
Gate conglomerate mutations with canEdit().
Link Org and Unlink are still rendered for view-only users, while the rest of the page correctly hides write actions behind canEdit().
Wrap the mutation buttons in the same edit guard used elsewhere
- <button class="flex items-center gap-1.5 px-3 py-1.5 bg-blue-600 text-white text-xs font-medium rounded-lg hover:bg-blue-700 cursor-pointer">
- <i class="fa-light fa-plus"></i> Link Org
- </button>
+ `@if` (canEdit()) {
+ <button class="flex items-center gap-1.5 px-3 py-1.5 bg-blue-600 text-white text-xs font-medium rounded-lg hover:bg-blue-700 cursor-pointer">
+ <i class="fa-light fa-plus"></i> Link Org
+ </button>
+ }
@@
- <button class="text-xs text-red-400 hover:text-red-600 cursor-pointer ml-2">Unlink</button>
+ `@if` (canEdit()) {
+ <button class="text-xs text-red-400 hover:text-red-600 cursor-pointer ml-2">Unlink</button>
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.html`
around lines 190 - 210, The Link Org button and each Unlink button inside the
conglomerate block are visible to view-only users; wrap the Link Org button and
the per-org Unlink button rendering in the same edit guard used elsewhere by
checking canEdit() (the same condition used in other templates) so they only
render when canEdit() returns true; locate the conglomerate block driven by
isConglomerate(), iterate over linkedOrgs, and conditionally render the Link Org
control and the Unlink control based on canEdit() to prevent showing mutation
actions to read-only users.
Comment on lines
+7
to
10
| #selectorTrigger | ||
| class="box-border content-stretch flex gap-3 items-center overflow-clip p-3 relative w-full cursor-pointer hover:bg-gray-50 transition-colors rounded-lg" | ||
| (click)="togglePanel($event, popover)" | ||
| (click)="togglePanel()" | ||
| data-testid="project-selector"> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.htmlRepository: linuxfoundation/lfx-v2-ui
Length of output: 7449
🏁 Script executed:
head -50 apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.tsRepository: linuxfoundation/lfx-v2-ui
Length of output: 2141
🏁 Script executed:
grep -n "togglePanel\|closePanel\|openPanel" apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.tsRepository: linuxfoundation/lfx-v2-ui
Length of output: 223
Use semantic buttons for the selector trigger and options; fix responsive positioning of the panel.
The trigger (lines 6-10) and option rows (lines 60-63, 87-90) are clickable <div>s without button semantics. Keyboard users cannot Tab to, activate with Enter/Space, or navigate these interactive elements, and assistive tech receives no semantic information about their purpose.
Additionally, the panel positioning at line 37 uses a hardcoded [style.left]="'352px'", which will misalign the dropdown on mobile devices and narrower viewports where the sidebar width differs.
Accessible markup sketch
- <div
+ <button
+ type="button"
`#selectorTrigger`
class="box-border content-stretch flex gap-3 items-center overflow-clip p-3 relative w-full cursor-pointer hover:bg-gray-50 transition-colors rounded-lg"
(click)="togglePanel()"
+ [attr.aria-expanded]="isOpen()"
+ aria-haspopup="dialog"
data-testid="project-selector">
...
- </div>
+ </button>
...
- <div
- class="flex items-center gap-3 p-2 cursor-pointer hover:bg-gray-100 rounded-lg transition-colors"
+ <button
+ type="button"
+ class="flex w-full items-center gap-3 p-2 cursor-pointer hover:bg-gray-100 rounded-lg transition-colors text-left"
(click)="selectProject(foundation)"
[attr.data-testid]="'foundation-' + foundation.slug">
...
- </div>
+ </button>Apply the same button change to the project rows (lines 87-90). For responsive positioning, calculate the panel's left offset dynamically based on the trigger's bounds using getBoundingClientRect() in the component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.html`
around lines 7 - 10, The trigger and option rows are non-semantic clickable divs
and the panel uses a hardcoded left offset; change the trigger element with id
"#selectorTrigger" and each project row to semantic <button> elements (preserve
classes, data-testid="project-selector", (click)="togglePanel()" and any ARIA
attributes) so they are keyboard-focusable and activatable, and update any
(keydown) handlers to handle Enter/Space if present; remove the hardcoded
[style.left]="'352px'" on the panel and instead compute the panel's left offset
in the ProjectSelectorComponent (use the selectorTrigger element reference and
its getBoundingClientRect() inside the togglePanel() or an updatePanelPosition()
method to set a component property like panelLeft which the template binds as
[style.left]="panelLeft + 'px'"). Ensure project rows use the same button change
and that focus/aria roles are preserved when toggling the panel.
Comment on lines
+34
to
+38
| <div | ||
| class="fixed z-[60] w-[28rem] max-h-[480px] bg-white rounded-xl shadow-2xl border border-gray-200 flex flex-col overflow-hidden" | ||
| [style.top.px]="panelTop()" | ||
| [style.left]="'352px'" | ||
| data-testid="project-selector-panel"> |
Contributor
There was a problem hiding this comment.
Anchor the floating panel to the trigger instead of a magic number.
Line 37 hardcodes a desktop-only offset. This component is also rendered inside the mobile drawer in this PR, so the 28rem panel can open detached from the trigger or mostly off-screen on smaller viewports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.html`
around lines 34 - 38, The panel currently uses a hardcoded left offset
('352px'); instead, anchor it to the trigger element by computing its left
position at render/open. Add or use an anchor reference (e.g., the trigger
element queried via `@ViewChild` or an `@Input` anchor element) and replace the
hardcoded left with a method like panelLeft() analogous to panelTop() that calls
anchor.getBoundingClientRect().left (or center) and clamps the value to the
viewport width so the panel never overflows; update the template to bind
[style.left.px]="panelLeft()" and ensure the logic lives next to panelTop() and
uses the same panel sizing/viewport constraints.
Comment on lines
+35
to
+37
| // Org lens user type — controls access level displayed in org lens | ||
| private readonly orgUserTypeSignal = signal<OrgUserType>('admin-edit'); | ||
| public readonly orgUserType = this.orgUserTypeSignal.asReadonly(); |
Contributor
There was a problem hiding this comment.
Don't default org sessions into an elevated role.
orgUserTypeSignal backs isAdmin()/canEdit() across the org lens, so starting it at 'admin-edit' makes privileged nav and action buttons render before real role data is loaded. Use a least-privilege fallback or hydrate this value before the lens paints.
Safer fallback until the real org role is loaded
- private readonly orgUserTypeSignal = signal<OrgUserType>('admin-edit');
+ private readonly orgUserTypeSignal = signal<OrgUserType>('employee');📝 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
| // Org lens user type — controls access level displayed in org lens | |
| private readonly orgUserTypeSignal = signal<OrgUserType>('admin-edit'); | |
| public readonly orgUserType = this.orgUserTypeSignal.asReadonly(); | |
| // Org lens user type — controls access level displayed in org lens | |
| private readonly orgUserTypeSignal = signal<OrgUserType>('employee'); | |
| public readonly orgUserType = this.orgUserTypeSignal.asReadonly(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/shared/services/app.service.ts` around lines 35 - 37,
The orgUserTypeSignal currently defaults to an elevated role ('admin-edit'),
causing privileged UI to flash before real role data loads; change its initial
value to a least-privilege fallback (e.g., 'view-only' or a neutral value) and
ensure the real role from session/org data hydrates orgUserTypeSignal as soon as
the session loader (where org role is retrieved) completes; update code paths
that read orgUserTypeSignal (orgUserType, isAdmin(), canEdit()) to rely on this
safe default until the hydrate call replaces it.
Comment on lines
6
to
+11
| /** | ||
| * Persona options available for user selection | ||
| */ | ||
| /** | ||
| * Organization lens user type options | ||
| */ |
Contributor
There was a problem hiding this comment.
Fix misaligned JSDoc comments.
The “Persona options available for user selection” docblock currently sits above ORG_USER_TYPE_OPTIONS, which is misleading.
📝 Suggested fix
-/**
- * Persona options available for user selection
- */
/**
* Organization lens user type options
*/
export const ORG_USER_TYPE_OPTIONS: { value: OrgUserType; label: string }[] = [
@@
+/**
+ * Persona options available for user selection
+ */
export const PERSONA_OPTIONS: PersonaOption[] = [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/constants/persona.constants.ts` around lines 6 - 11, The
JSDoc comment "Persona options available for user selection" is misaligned above
ORG_USER_TYPE_OPTIONS; move or reassign the docblock so it sits directly above
the correct symbol (e.g., PERSONA_OPTIONS or whichever constant represents
user-selectable personas) and ensure the ORG_USER_TYPE_OPTIONS block has its own
accurate comment "Organization lens user type options" immediately above it;
update the two JSDoc blocks so each documents the correct exported constant name
(ORG_USER_TYPE_OPTIONS and the persona options constant) to avoid misleading
documentation.
- Rewrote all org-lens pages (overview, projects, project-detail, code, membership, groups, benefits, CLA, profile, permissions) to match the HTML prototype layout and structure - Added project-detail page with charts and leaderboard matching the prototype - Replaced raw <button> elements with <lfx-button>, status spans with <lfx-tag>, and info banners with <lfx-message> across all pages - Standardised Tailwind classes: slate-* → gray-*, rounded-xl → rounded-lg, tab active colour text-blue-700 → text-blue-600, dropped explicit border-gray-200 from tab container borders - Added Open Source Strategy page (/org/strategy) with the "How to Get More Involved" section moved from the Overview page - Added Open Source Strategy nav item under Settings & Resources Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
🧹 Deployment RemovedThe deployment for PR #391 has been removed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (9)
apps/lfx-one/src/app/modules/org-lens/permissions/org-permissions.component.html (2)
10-13:⚠️ Potential issue | 🟠 MajorGuard mutation controls with
canEdit()before rendering.Line 10 and Line 84 still expose write affordances to non-edit org roles, which breaks the role-visibility contract for this lens.
Suggested template guard
- <button class="flex items-center gap-2 px-4 py-2 bg-blue-600 text-white text-sm font-medium rounded-lg hover:bg-blue-700 cursor-pointer"> - <i class="fa-light fa-user-plus"></i> - Invite User - </button> + `@if` (canEdit()) { + <button type="button" class="flex items-center gap-2 px-4 py-2 bg-blue-600 text-white text-sm font-medium rounded-lg hover:bg-blue-700 cursor-pointer"> + <i class="fa-light fa-user-plus"></i> + Invite User + </button> + } ... - <button class="text-xs text-gray-400 hover:text-gray-600 cursor-pointer"> - <i class="fa-light fa-ellipsis"></i> - </button> + `@if` (canEdit()) { + <button type="button" class="text-xs text-gray-400 hover:text-gray-600 cursor-pointer" aria-label="Open user actions for {{ user.name }}"> + <i class="fa-light fa-ellipsis"></i> + </button> + }Also applies to: 84-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/permissions/org-permissions.component.html` around lines 10 - 13, The "Invite User" button and other mutation controls in the org-permissions template are rendered unconditionally; wrap those elements with a guard that checks the component's canEdit() permission method so non-edit roles do not see write affordances—locate the template elements (e.g., the Invite User button and the controls around the block referenced by the reviewer) and only render them when canEdit() returns true (use an *ngIf or equivalent referencing the component's canEdit() method in org-permissions component class).
40-43:⚠️ Potential issue | 🟡 MinorWire the search input to filtered rows (or remove it for now).
Line 42 currently updates nothing, and Line 57 still renders
users, so search has no effect.Minimal wiring sketch
- <input type="text" placeholder="Search users..." class="text-sm outline-none bg-transparent text-gray-700 w-40" /> + <input + `#searchInput` + type="text" + (input)="searchQuery.set(searchInput.value)" + placeholder="Search users..." + class="text-sm outline-none bg-transparent text-gray-700 w-40" /> ... - `@for` (user of users; track user.email) { + `@for` (user of filteredUsers(); track user.email) {Also applies to: 57-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/permissions/org-permissions.component.html` around lines 40 - 43, The search input is unhooked and the template still renders the raw users array; wire it to a search model and filter the rendered list: add a component property like searchQuery (or use an existing one), implement a filterUsers(search: string) method or a filteredUsers getter that returns users.filter(u => matches search), and bind the input to it (e.g., [(ngModel)]="searchQuery" with (ngModelChange)="filterUsers($event)" or (input)="onSearch($event.target.value)"); then change the template iteration to use filteredUsers instead of users (references: users, filteredUsers or filterUsers/onSearch, searchQuery).apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html (2)
43-43:⚠️ Potential issue | 🟡 MinorSearch inputs are not wired to filtering logic.
Line 43 and Line 182 render search boxes, but there’s no binding or input handler, so typing won’t affect displayed rows.
💡 Suggested template wiring
- <input type="text" placeholder="Search" class="pl-8 pr-3 py-1.5 text-sm border border-gray-200 rounded-md outline-none text-gray-700 w-44" /> + <input + type="text" + placeholder="Search" + class="pl-8 pr-3 py-1.5 text-sm border border-gray-200 rounded-md outline-none text-gray-700 w-44" + [value]="contributorSearch()" + (input)="setContributorSearch(($any($event.target).value ?? '').trim())" /> - <input type="text" placeholder="Search" class="pl-8 pr-3 py-1.5 text-sm border border-gray-200 rounded-md outline-none text-gray-700 w-44" /> + <input + type="text" + placeholder="Search" + class="pl-8 pr-3 py-1.5 text-sm border border-gray-200 rounded-md outline-none text-gray-700 w-44" + [value]="trainingSearch()" + (input)="setTrainingSearch(($any($event.target).value ?? '').trim())" />Also applies to: 182-182
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html` at line 43, The search inputs in org-code.component.html are not bound to any component state or handlers, so they do nothing; wire them to the component by adding an input binding like (input)="onSearchChange($event.target.value)" or [(ngModel)]="searchTerm" on both search boxes and implement an onSearchChange(searchTerm: string) method in org-code.component.ts that updates a component property (e.g., searchTerm), calls a filterRows() method, and sets filteredRows from rows by matching the searchTerm against the relevant row fields; ensure filterRows and the properties (rows, filteredRows, searchTerm) are defined and used by the template to render the displayed list (optionally debounce input inside onSearchChange if needed).
190-193:⚠️ Potential issue | 🟡 MinorTraining/Event filter chips are static and don’t update state.
These buttons are styled as filters, but without click handlers and active-state bindings they remain presentational only.
💡 Suggested pattern for interactive filter chips
- <button class="px-4 py-1.5 text-sm border border-blue-400 rounded-md bg-blue-50 text-blue-600 font-medium cursor-pointer">All (82)</button> + <button + class="px-4 py-1.5 text-sm border rounded-md cursor-pointer" + [class]="trainingTypeFilter() === 'all' + ? 'border-blue-400 bg-blue-50 text-blue-600 font-medium' + : 'border-gray-200 bg-white text-gray-700 hover:border-gray-400'" + (click)="setTrainingTypeFilter('all')">All (82)</button>Also applies to: 272-275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html` around lines 190 - 193, The filter chip buttons in org-code.component.html are static presentational elements; make them interactive by binding them to a component state property (e.g., selectedFilter) and handling clicks (e.g., (click)="setFilter('All')", (click)="setFilter('Training')", (click)="setFilter('Certification')"); implement setFilter(filter: string) and selectedFilter: string in the OrgCodeComponent class, update the templates to use [ngClass] or conditional class bindings to toggle the active styling when selectedFilter matches the chip, and add accessible attributes like [attr.aria-pressed]="selectedFilter === '...'" so the chips update visually and functionally (apply the same changes for the duplicated chips block).apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.html (2)
27-32:⚠️ Potential issue | 🟡 MinorDocumentation links are not navigable.
The
<a>elements on Line 29 have nohrefor[routerLink], making them inaccessible to keyboard navigation and non-functional for all users.🔧 Proposed fix
<div class="py-3.5"> - <a class="text-sm font-semibold text-blue-500 hover:text-blue-600 cursor-pointer">{{ doc.title }}</a> + <a [href]="doc.url" target="_blank" rel="noopener" class="text-sm font-semibold text-blue-500 hover:text-blue-600 cursor-pointer">{{ doc.title }}</a> <div class="text-xs text-gray-500 mt-1">{{ doc.description }}</div> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.html` around lines 27 - 32, The anchor elements inside the for-loop over keyDocs in org-benefits.component.html (the <a> showing doc.title) lack href/routerLink, so they are not keyboard-navigable; update the template to bind a real navigation target—use an href bound to doc.url for external links or a [routerLink] bound to a route property (e.g., doc.route) for internal navigation, and handle external links with target="_blank" rel="noopener noreferrer" as needed; if some docs intentionally have no link, replace the <a> with a focusable element (add role="button" and tabindex="0") and wire a click handler that performs navigation or opens a dialog, ensuring doc.title remains accessible.
13-19:⚠️ Potential issue | 🟡 Minor"Learn More" button has no action or navigation.
The
<lfx-button>renders a clickable button but lacks a(click)handler,routerLink, or any interaction. Users will click expecting navigation but nothing happens.Either wire to actual resource URLs/routes or disable the button until real targets exist.
🔧 Proposed fix using routerLink (if route exists)
- <lfx-button label="Learn More" severity="info" size="small"></lfx-button> + <lfx-button label="Learn More" severity="info" size="small" [routerLink]="card.route"></lfx-button>Or with external URL:
- <lfx-button label="Learn More" severity="info" size="small"></lfx-button> + <a [href]="card.url" target="_blank" rel="noopener"> + <lfx-button label="Learn More" severity="info" size="small"></lfx-button> + </a>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.html` around lines 13 - 19, The Learn More <lfx-button> inside the cards loop has no interaction; update the template to wire the button to each card's destination (e.g., use card.link or card.route): if a route exists use routerLink binding to card.route (or call a component method like openCard(card) via (click) that navigates programmatically), and if no target exists set the button disabled or hide it; update the cards data to include a link/route property if missing and implement the component method (e.g., openCard or navigateTo) to perform Router.navigate or window.open for external URLs.apps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.html (1)
34-37:⚠️ Potential issue | 🟡 MinorWebsite anchor is non-functional.
The website field renders as a clickable anchor but has no
hrefbinding, so it doesn't navigate anywhere. Consider binding toprofile.websitewith proper URL handling.🔗 Proposed fix
<div> <div class="text-xs font-semibold text-gray-400 uppercase tracking-wide mb-1">Website</div> - <a class="text-sm text-blue-500 hover:text-blue-600 cursor-pointer">{{ profile.website }}</a> + <a [href]="'https://' + profile.website" target="_blank" rel="noopener noreferrer" class="text-sm text-blue-500 hover:text-blue-600 cursor-pointer">{{ profile.website }}</a> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.html` around lines 34 - 37, The anchor for the website (template referencing profile.website in org-profile.component.html) is missing an href and is non-functional; update the template to bind href to a sanitized/normalized URL (use the component method, e.g., getNormalizedWebsite(profile.website) in OrgProfileComponent) that prepends "https://" if the scheme is missing, and add target="_blank" rel="noopener noreferrer for safe external navigation; implement the normalization helper in the component class (e.g., normalizeWebsite or getNormalizedWebsite) and use that method in the href binding so displayed text remains profile.website while the link navigates correctly.apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.html (1)
70-75:⚠️ Potential issue | 🟠 MajorGate mutation buttons with
canEdit().The
RenewandSignbuttons are rendered based solely onagreement.status, allowing view-only users (employee,admin-view-only) to see write actions. These should be guarded withcanEdit()consistent with the rest of the org lens.🛡️ Proposed fix
- `@if` (agreement.status === 'Expired') { - <lfx-button label="Renew" [text]="true" size="small"></lfx-button> + `@if` (canEdit() && agreement.status === 'Expired') { + <lfx-button label="Renew" [text]="true" size="small"></lfx-button> } - `@if` (agreement.status === 'Pending') { - <lfx-button label="Sign" [text]="true" size="small"></lfx-button> + `@if` (canEdit() && agreement.status === 'Pending') { + <lfx-button label="Sign" [text]="true" size="small"></lfx-button> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.html` around lines 70 - 75, The Renew and Sign action buttons are shown based only on agreement.status, exposing write actions to view-only roles; update the template conditions to also check canEdit() before rendering these buttons so they only appear for editable users—modify the blocks that render <lfx-button label="Renew"> and <lfx-button label="Sign"> to require both agreement.status === 'Expired' / 'Pending' and canEdit() (the same canEdit() used elsewhere in the org lens).apps/lfx-one/src/app/layouts/main-layout/main-layout.component.ts (1)
196-206:⚠️ Potential issue | 🟠 MajorRoute-based lens initialization is missing.
Deep linking to
/org/**or/foundation/**routes renders the wrong sidebar becauseactiveLensdefaults to'me'and is only updated via user interaction with the lens-switcher. TheNavigationEndhandler should derive and set the lens from the current URL.🔧 Proposed approach
public constructor() { + // Initialize lens from current URL + this.syncLensFromUrl(this.router.url); + this.router.events .pipe( filter((event) => event instanceof NavigationEnd), takeUntilDestroyed() ) - .subscribe(() => { + .subscribe((event) => { this.appService.closeMobileSidebar(); this.appService.setProjectSelectorOpen(false); + this.syncLensFromUrl((event as NavigationEnd).urlAfterRedirects); }); } + + private syncLensFromUrl(url: string): void { + if (url.startsWith('/org')) { + this.appService.setLens('org'); + } else if (url.startsWith('/foundation')) { + this.appService.setLens('foundation'); + } else { + this.appService.setLens('me'); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/layouts/main-layout/main-layout.component.ts` around lines 196 - 206, The NavigationEnd handler currently only closes UI elements and doesn't set the route-derived lens, causing the sidebar to remain on 'me'; update the constructor's router.events subscribe block to read the current URL (use event instanceof NavigationEnd then event.urlAfterRedirects or this.router.url), derive the lens string by checking path segments or prefixes (e.g., '/org' -> 'org', '/foundation' -> 'foundation', default -> 'me'), and apply it by calling the existing lens setter (e.g., appService.setActiveLens(...) or this.activeLens = ...) before closing the mobile sidebar and toggling project selector so the sidebar reflects the route on deep links.
🧹 Nitpick comments (11)
apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html (2)
354-372: Nested iteration filters all steps per category.The inner
@foriterates over alldetailOnboardingStepsfor each category, then filters with@if (step.category === category). This is O(n×m) where n=categories and m=steps.Consider pre-grouping steps by category in the component:
protected readonly stepsByCategory = computed(() => { const grouped = new Map<string, OnboardingStep[]>(); for (const step of this.detailOnboardingSteps) { const list = grouped.get(step.category) ?? []; list.push(step); grouped.set(step.category, list); } return grouped; });Then in template:
`@for` (category of ['Documentation', 'Key Contacts', 'Board & Committees', 'Communications']; track category) { `@for` (step of stepsByCategory().get(category) ?? []; track step.label) { <!-- render step directly, no `@if` needed --> } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html` around lines 354 - 372, The template currently loops categories and then iterates over all detailOnboardingSteps for each category and filters with an `@if`, causing O(n×m) work; change to pre-group steps by category in the component (add a computed like stepsByCategory that maps category -> OnboardingStep[] by iterating detailOnboardingSteps) and update the template to iterate stepsByCategory().get(category) ?? [] for each category, removing the per-step `@if` and keeping the existing bindings (step.label, step.done, canEdit(), etc.) so rendering is linear.
41-41: Consider typed event handling instead of$any().Using
$any($event.target).valuebypasses TypeScript's type checking. A cleaner approach uses template reference variables or proper event typing.♻️ Typed alternative using template reference
- <input type="text" placeholder="Search..." class="text-sm outline-none bg-transparent text-gray-700 w-36" (input)="searchQuery.set($any($event.target).value)" /> + <input `#searchInput` type="text" placeholder="Search..." class="text-sm outline-none bg-transparent text-gray-700 w-36" (input)="searchQuery.set(searchInput.value)" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html` at line 41, Replace the untyped $any($event.target).value usage by adding a template reference on the input and using its .value (e.g. add a reference like `#q`) so the handler calls searchQuery.set(q.value); locate the input element that currently uses (input)="searchQuery.set($any($event.target).value)" and update the (input) to read from the template ref instead, ensuring the component uses the typed value and removes the $any cast.apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.ts (2)
284-286:initials()may throw on edge cases.If
nameis an empty string or a single word,n[0]could beundefinedfor empty segments from.split(' '). Consider adding a guard.🛡️ Defensive implementation
protected initials(name: string): string { - return name.split(' ').map((n) => n[0]).join(''); + return name + .split(' ') + .filter((n) => n.length > 0) + .map((n) => n[0]) + .join(''); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.ts` around lines 284 - 286, The initials(name: string) function can throw when name is empty or contains extra spaces; update org-membership.component.ts to defensively handle edge cases by trimming the input, splitting on whitespace (e.g., using name.trim().split(/\s+/)), filtering out empty segments, and then mapping each segment to its first character only when present (e.g., segment[0] or optional chaining). Ensure the function returns a safe fallback (empty string or a default) when there are no valid segments; adjust the initials method accordingly.
280-282: Unsafe type assertion insetDetailTab.The method accepts any
stringand casts it toMembershipDetailTabwithout validation. If the template or future code passes an invalid tab ID, this will silently accept it.🛡️ Suggested type-safe alternative
- protected setDetailTab(tab: string): void { - this.activeDetailTab.set(tab as MembershipDetailTab); + protected setDetailTab(tab: MembershipDetailTab): void { + this.activeDetailTab.set(tab); }Then in the template, ensure the tab objects are typed so
tab.idis alreadyMembershipDetailTab.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.ts` around lines 280 - 282, The setDetailTab method unsafely casts any string to MembershipDetailTab; change its signature to accept MembershipDetailTab (protected setDetailTab(tab: MembershipDetailTab): void) or, if you must accept string, validate the value before calling this.activeDetailTab.set — e.g., check it against the known MembershipDetailTab values/enum or a whitelist (reject/ignore or fallback to a default) and only call activeDetailTab.set with a validated MembershipDetailTab; also ensure in the template the tab objects expose tab.id typed as MembershipDetailTab so callers pass the correct type to setDetailTab.apps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.ts (1)
15-17: Consider acceptingInvolveTabdirectly for type safety.The method accepts
stringand casts toInvolveTab, bypassing compile-time validation. Since the template calls this with hardcoded valid IDs, it works, but acceptingInvolveTabdirectly would catch invalid values at compile time.♻️ Suggested improvement
- protected setInvolveTab(tab: string): void { - this.activeInvolveTab.set(tab as InvolveTab); + protected setInvolveTab(tab: InvolveTab): void { + this.activeInvolveTab.set(tab); }Then in the template, cast at the call site where the literal is known:
(click)="setInvolveTab($any(tab.id))"Or define the tabs array in the component with proper typing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.ts` around lines 15 - 17, Change the setInvolveTab method to accept an InvolveTab instead of string to enforce compile-time type safety (update the signature of setInvolveTab and use the activeInvolveTab.set call unchanged), and update any callers: in the template cast the literal at the call site (e.g. use $any(tab.id) or an explicit cast) or better yet type the tabs array as InvolveTab[] so callers pass a correctly typed value without casts.apps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.html (1)
35-35: Consider responsive breakpoints for the two-column grid.
grid-cols-2will display two columns on all screen sizes, which may cause cramped content on mobile devices.📱 Responsive suggestion
- <div class="grid grid-cols-2 gap-8"> + <div class="grid grid-cols-1 md:grid-cols-2 gap-8">Apply similarly to line 46 if the action links grid should also stack on mobile.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.html` at line 35, The two-column grid uses a fixed "grid-cols-2" which forces two columns on all breakpoints; update the class on the element containing "grid grid-cols-2 gap-8" in org-strategy.component.html to use responsive Tailwind classes (e.g., "grid grid-cols-1 sm:grid-cols-2 gap-8" or "grid grid-cols-1 md:grid-cols-2 gap-8") so it stacks on small screens and becomes two columns at the desired breakpoint, and apply the same responsive change to the similar action-links grid around line 46.apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html (1)
79-87: Add ARIA semantics to the toggle control.The toggle is keyboard-clickable, but it should expose state/intent to assistive tech (
aria-pressed+ label).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html` around lines 79 - 87, The toggle button currently lacks ARIA semantics; update the button element that uses hideFormerEmployees() and hideFormerEmployees.set(...) to expose its state and intent by adding an appropriate ARIA role and attributes: bind aria-pressed (or use role="switch" with aria-checked) to hideFormerEmployees(), add a clear aria-label (e.g., "Hide former employees") or aria-labelledby for a visible label, and mark the inner decorative span as aria-hidden="true" so assistive tech reads only the button state; keep the existing (click) handler on hideFormerEmployees.set(...) unchanged.apps/lfx-one/src/app/modules/org-lens/project-detail/org-project-detail.component.html (1)
62-69: Inline array in template reduces readability.The time range options are defined inline in the
@forloop. Consider extracting this to a component property for better maintainability.♻️ Suggested refactor
In component:
protected readonly timeRangeOptions = [ { id: '1y', label: 'Past 365 Days' }, { id: '2y', label: 'Past 2 Years' }, { id: 'all', label: 'All Time' }, ];In template:
- `@for` (range of [{id: '1y', label: 'Past 365 Days'}, {id: '2y', label: 'Past 2 Years'}, {id: 'all', label: 'All Time'}]; track range.id) { + `@for` (range of timeRangeOptions; track range.id) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/project-detail/org-project-detail.component.html` around lines 62 - 69, The template currently defines the time range options inline in the `@for` loop which hurts readability; extract that array into the component as a property (e.g., protected readonly timeRangeOptions = [{ id: '1y', label: 'Past 365 Days' }, { id: '2y', label: 'Past 2 Years' }, { id: 'all', label: 'All Time' }]) and update the template to iterate over timeRangeOptions instead of the inline array, preserving the existing bindings and calls to timeRange() and setTimeRange(range.id) and keeping track range.id for tracking.apps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.html (1)
395-395: Avoid$any()template cast.Using
$any($event.target).valuebypasses type safety. Consider using a template reference variable or typing the event handler properly.♻️ Proposed fix using template reference
- <input type="text" placeholder="Name or email address..." class="text-sm outline-none bg-transparent text-gray-700 flex-1" (input)="correctionSearch.set($any($event.target).value)" /> + <input `#searchInput` type="text" placeholder="Name or email address..." class="text-sm outline-none bg-transparent text-gray-700 flex-1" (input)="correctionSearch.set(searchInput.value)" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.html` at line 395, Replace the unsafe $any cast in the input handler by using a template reference or a properly typed event handler: add a template reference (e.g., `#search`) on the input and call correctionSearch.set(search.value) instead of correctionSearch.set($any($event.target).value), or create a component method (e.g., onCorrectionSearchInput(event: Event)) that extracts the value with typed casting (const target = event.target as HTMLInputElement; correctionSearch.set(target.value)) and bind (input)="onCorrectionSearchInput($event)"; update the template to stop using $any and keep using correctionSearch.set to store the value.apps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.html (1)
107-112: Unusual array iteration pattern.Using
[].constructor(n)to create an iterable for avatar rendering works but is non-idiomatic. Consider using a utility likeArray.from({ length: n })or a helper method for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.html` around lines 107 - 112, The template uses a non-idiomatic iterable expression [].constructor(project.contributorAvatars) to render avatars; replace it with a clear iterable such as Array.from({ length: project.contributorAvatars }) or a helper method on the component (e.g., getAvatarCount(project) returning an array) and keep the rest of the block intact so the loop still indexes into contributorColors via contributorColors[$index % contributorColors.length].apps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.ts (1)
46-215: Consider extracting static project data.The 170-line
projectsarray is defined inline in the component. For maintainability, consider moving this to a separate data file or service, especially if this data will eventually come from an API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.ts` around lines 46 - 215, The inline 170-line protected readonly projects: Project[] array in org-projects.component.ts should be moved out for maintainability; extract the static data into a new module (e.g., projects.data.ts) or a service (e.g., ProjectsService) that exports/returns Project[] and then import/use it in the component—replace the inline protected readonly projects with an assignment from the import (or a call to ProjectsService.getProjects()) and keep the Project type and property name (projects) unchanged so existing templates and logic continue to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html`:
- Around line 51-64: The select elements in org-code.component.html are not
bound to component state; add two-way bindings (e.g.,
[(ngModel)]="selectedProject" and [(ngModel)]="selectedTimeRange") or (change)
handlers that call a component method (e.g., onFilterChange() or
updateFilters()) so selections update properties and trigger filtering;
implement corresponding properties (selectedProject, selectedTimeRange) and a
filter handler in the OrgCodeComponent class to apply the filter logic. Repeat
the same bindings/fix for the other select blocks indicated (lines ~141-150 and
~230-238).
- Around line 300-303: The template currently guards e.speakingTotal with a
strict !== null check which misses undefined; update the conditional around
e.speakingTotal (the block rendering {{ e.speakingProposals }} / {{
e.speakingTotal }}) to be nullish-safe (e.g., use != null or an explicit check
for both null and undefined) so the totals branch only renders when
speakingTotal is neither null nor undefined.
In `@apps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.html`:
- Around line 95-98: The search input is only visual; wire it to the
OrgGroupsComponent by adding a component property (e.g., searchQuery) and a
handler (e.g., onSearchChange or filterAccessHolders) that filters the access
holders list used by the template, then bind the input in
org-groups.component.html with two-way binding or input event
([(ngModel)]="searchQuery" and (ngModelChange)="onSearchChange()" or
(input)="onSearchChange()"); ensure FormsModule is imported into the module so
ngModel works and update the component's displayed users array (e.g.,
filteredAccessHolders) from the handler to reflect the search.
In
`@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html`:
- Around line 385-399: The ROI inputs in org-membership.component.html are
static because they use hardcoded value attributes instead of bound state;
update the component to expose reactive state (e.g., signals named annualFee,
hoursContributed, hourlyRate or FormControls) and bind the inputs to those
(replace value="..." with reactive bindings and wire input events or
ngModel/FormControl so user edits update the component state), or if the fields
are intentionally non-interactive, mark them disabled or add a comment
indicating they are placeholders; ensure bindings reference the declared symbols
(annualFee, hoursContributed, hourlyRate or the corresponding FormControl names)
so downstream ROI calculations react to changes.
In `@apps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.html`:
- Around line 325-347: The anchor for each event (the <a> showing {{ event.name
}} inside the for-loop over events) is missing a navigation target and should
either be wired to the event detail route or converted to a non-interactive
element; update the element that renders event.name to use a proper router link
like [routerLink]="['/events', event.id]" (or the app's event detail route) so
it is focusable and keyboard-accessible, or replace the <a> with a <span> (and
remove cursor-pointer styling) if navigation is not yet available; ensure you
update the template where event.name, event.id, and the surrounding row
rendering (the for-loop over events) are defined.
In
`@apps/lfx-one/src/app/modules/org-lens/project-detail/org-project-detail.component.ts`:
- Around line 214-216: The setTimeRange method currently accepts a string and
unsafely casts it to TimeRange (method: setTimeRange, signal: timeRange, type:
TimeRange); update it to be type-safe by either changing the parameter signature
to accept TimeRange instead of string, or add runtime validation that checks the
incoming range against the allowed values (e.g., '1y', '2y', 'all') before
calling this.timeRange.set, and reject or ignore invalid values to avoid placing
an invalid value into the signal.
In `@apps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.html`:
- Around line 43-45: The "Add project" button is a write action and should be
gated by the component's permission checker; wrap the button in a template-level
guard using the existing canEdit() method (e.g., add *ngIf="canEdit()" around
the button in org-projects.component.html) so only users with edit permissions
see/activate it, and verify the component class exposes the canEdit() method (or
import/forward it) used elsewhere to keep behavior consistent across org lens
pages.
In `@apps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.html`:
- Line 59: The anchor element in org-strategy.component.html for "Read Report"
is missing an href and thus is not accessible; if it navigates to a report URL,
add the proper href to the <a> so it becomes a real link (preserve classes) and
ensure target/rel as needed; if it triggers an action, replace the anchor with a
semantic <button> or the standard <lfx-button> component used elsewhere (keeping
the existing classes/styles and wiring up the click handler such as the
component's readReport() method), and add appropriate aria-labels for
accessibility.
- Around line 47-53: The anchor elements used for actions ("Commission
Research", "Post a blog to Linux.com", "Sponsor Events", "Speak at Events",
"Create a Community Event", "Form a New Industry Initiative") are
non-navigational and lack hrefs; replace each <a class="...
cursor-pointer">...</a> with a semantically correct <button> (preserving classes
and click handlers) to restore accessibility and keyboard semantics, or if they
actually navigate, add proper href or routerLink attributes instead so screen
readers and keyboard users get correct behavior.
---
Duplicate comments:
In `@apps/lfx-one/src/app/layouts/main-layout/main-layout.component.ts`:
- Around line 196-206: The NavigationEnd handler currently only closes UI
elements and doesn't set the route-derived lens, causing the sidebar to remain
on 'me'; update the constructor's router.events subscribe block to read the
current URL (use event instanceof NavigationEnd then event.urlAfterRedirects or
this.router.url), derive the lens string by checking path segments or prefixes
(e.g., '/org' -> 'org', '/foundation' -> 'foundation', default -> 'me'), and
apply it by calling the existing lens setter (e.g.,
appService.setActiveLens(...) or this.activeLens = ...) before closing the
mobile sidebar and toggling project selector so the sidebar reflects the route
on deep links.
In `@apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.html`:
- Around line 27-32: The anchor elements inside the for-loop over keyDocs in
org-benefits.component.html (the <a> showing doc.title) lack href/routerLink, so
they are not keyboard-navigable; update the template to bind a real navigation
target—use an href bound to doc.url for external links or a [routerLink] bound
to a route property (e.g., doc.route) for internal navigation, and handle
external links with target="_blank" rel="noopener noreferrer" as needed; if some
docs intentionally have no link, replace the <a> with a focusable element (add
role="button" and tabindex="0") and wire a click handler that performs
navigation or opens a dialog, ensuring doc.title remains accessible.
- Around line 13-19: The Learn More <lfx-button> inside the cards loop has no
interaction; update the template to wire the button to each card's destination
(e.g., use card.link or card.route): if a route exists use routerLink binding to
card.route (or call a component method like openCard(card) via (click) that
navigates programmatically), and if no target exists set the button disabled or
hide it; update the cards data to include a link/route property if missing and
implement the component method (e.g., openCard or navigateTo) to perform
Router.navigate or window.open for external URLs.
In `@apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.html`:
- Around line 70-75: The Renew and Sign action buttons are shown based only on
agreement.status, exposing write actions to view-only roles; update the template
conditions to also check canEdit() before rendering these buttons so they only
appear for editable users—modify the blocks that render <lfx-button
label="Renew"> and <lfx-button label="Sign"> to require both agreement.status
=== 'Expired' / 'Pending' and canEdit() (the same canEdit() used elsewhere in
the org lens).
In `@apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html`:
- Line 43: The search inputs in org-code.component.html are not bound to any
component state or handlers, so they do nothing; wire them to the component by
adding an input binding like (input)="onSearchChange($event.target.value)" or
[(ngModel)]="searchTerm" on both search boxes and implement an
onSearchChange(searchTerm: string) method in org-code.component.ts that updates
a component property (e.g., searchTerm), calls a filterRows() method, and sets
filteredRows from rows by matching the searchTerm against the relevant row
fields; ensure filterRows and the properties (rows, filteredRows, searchTerm)
are defined and used by the template to render the displayed list (optionally
debounce input inside onSearchChange if needed).
- Around line 190-193: The filter chip buttons in org-code.component.html are
static presentational elements; make them interactive by binding them to a
component state property (e.g., selectedFilter) and handling clicks (e.g.,
(click)="setFilter('All')", (click)="setFilter('Training')",
(click)="setFilter('Certification')"); implement setFilter(filter: string) and
selectedFilter: string in the OrgCodeComponent class, update the templates to
use [ngClass] or conditional class bindings to toggle the active styling when
selectedFilter matches the chip, and add accessible attributes like
[attr.aria-pressed]="selectedFilter === '...'" so the chips update visually and
functionally (apply the same changes for the duplicated chips block).
In
`@apps/lfx-one/src/app/modules/org-lens/permissions/org-permissions.component.html`:
- Around line 10-13: The "Invite User" button and other mutation controls in the
org-permissions template are rendered unconditionally; wrap those elements with
a guard that checks the component's canEdit() permission method so non-edit
roles do not see write affordances—locate the template elements (e.g., the
Invite User button and the controls around the block referenced by the reviewer)
and only render them when canEdit() returns true (use an *ngIf or equivalent
referencing the component's canEdit() method in org-permissions component
class).
- Around line 40-43: The search input is unhooked and the template still renders
the raw users array; wire it to a search model and filter the rendered list: add
a component property like searchQuery (or use an existing one), implement a
filterUsers(search: string) method or a filteredUsers getter that returns
users.filter(u => matches search), and bind the input to it (e.g.,
[(ngModel)]="searchQuery" with (ngModelChange)="filterUsers($event)" or
(input)="onSearch($event.target.value)"); then change the template iteration to
use filteredUsers instead of users (references: users, filteredUsers or
filterUsers/onSearch, searchQuery).
In `@apps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.html`:
- Around line 34-37: The anchor for the website (template referencing
profile.website in org-profile.component.html) is missing an href and is
non-functional; update the template to bind href to a sanitized/normalized URL
(use the component method, e.g., getNormalizedWebsite(profile.website) in
OrgProfileComponent) that prepends "https://" if the scheme is missing, and add
target="_blank" rel="noopener noreferrer for safe external navigation; implement
the normalization helper in the component class (e.g., normalizeWebsite or
getNormalizedWebsite) and use that method in the href binding so displayed text
remains profile.website while the link navigates correctly.
---
Nitpick comments:
In `@apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html`:
- Around line 79-87: The toggle button currently lacks ARIA semantics; update
the button element that uses hideFormerEmployees() and
hideFormerEmployees.set(...) to expose its state and intent by adding an
appropriate ARIA role and attributes: bind aria-pressed (or use role="switch"
with aria-checked) to hideFormerEmployees(), add a clear aria-label (e.g., "Hide
former employees") or aria-labelledby for a visible label, and mark the inner
decorative span as aria-hidden="true" so assistive tech reads only the button
state; keep the existing (click) handler on hideFormerEmployees.set(...)
unchanged.
In `@apps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.html`:
- Line 395: Replace the unsafe $any cast in the input handler by using a
template reference or a properly typed event handler: add a template reference
(e.g., `#search`) on the input and call correctionSearch.set(search.value) instead
of correctionSearch.set($any($event.target).value), or create a component method
(e.g., onCorrectionSearchInput(event: Event)) that extracts the value with typed
casting (const target = event.target as HTMLInputElement;
correctionSearch.set(target.value)) and bind
(input)="onCorrectionSearchInput($event)"; update the template to stop using
$any and keep using correctionSearch.set to store the value.
In
`@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html`:
- Around line 354-372: The template currently loops categories and then iterates
over all detailOnboardingSteps for each category and filters with an `@if`,
causing O(n×m) work; change to pre-group steps by category in the component (add
a computed like stepsByCategory that maps category -> OnboardingStep[] by
iterating detailOnboardingSteps) and update the template to iterate
stepsByCategory().get(category) ?? [] for each category, removing the per-step
`@if` and keeping the existing bindings (step.label, step.done, canEdit(), etc.)
so rendering is linear.
- Line 41: Replace the untyped $any($event.target).value usage by adding a
template reference on the input and using its .value (e.g. add a reference like
`#q`) so the handler calls searchQuery.set(q.value); locate the input element that
currently uses (input)="searchQuery.set($any($event.target).value)" and update
the (input) to read from the template ref instead, ensuring the component uses
the typed value and removes the $any cast.
In
`@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.ts`:
- Around line 284-286: The initials(name: string) function can throw when name
is empty or contains extra spaces; update org-membership.component.ts to
defensively handle edge cases by trimming the input, splitting on whitespace
(e.g., using name.trim().split(/\s+/)), filtering out empty segments, and then
mapping each segment to its first character only when present (e.g., segment[0]
or optional chaining). Ensure the function returns a safe fallback (empty string
or a default) when there are no valid segments; adjust the initials method
accordingly.
- Around line 280-282: The setDetailTab method unsafely casts any string to
MembershipDetailTab; change its signature to accept MembershipDetailTab
(protected setDetailTab(tab: MembershipDetailTab): void) or, if you must accept
string, validate the value before calling this.activeDetailTab.set — e.g., check
it against the known MembershipDetailTab values/enum or a whitelist
(reject/ignore or fallback to a default) and only call activeDetailTab.set with
a validated MembershipDetailTab; also ensure in the template the tab objects
expose tab.id typed as MembershipDetailTab so callers pass the correct type to
setDetailTab.
In
`@apps/lfx-one/src/app/modules/org-lens/project-detail/org-project-detail.component.html`:
- Around line 62-69: The template currently defines the time range options
inline in the `@for` loop which hurts readability; extract that array into the
component as a property (e.g., protected readonly timeRangeOptions = [{ id:
'1y', label: 'Past 365 Days' }, { id: '2y', label: 'Past 2 Years' }, { id:
'all', label: 'All Time' }]) and update the template to iterate over
timeRangeOptions instead of the inline array, preserving the existing bindings
and calls to timeRange() and setTimeRange(range.id) and keeping track range.id
for tracking.
In `@apps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.html`:
- Around line 107-112: The template uses a non-idiomatic iterable expression
[].constructor(project.contributorAvatars) to render avatars; replace it with a
clear iterable such as Array.from({ length: project.contributorAvatars }) or a
helper method on the component (e.g., getAvatarCount(project) returning an
array) and keep the rest of the block intact so the loop still indexes into
contributorColors via contributorColors[$index % contributorColors.length].
In `@apps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.ts`:
- Around line 46-215: The inline 170-line protected readonly projects: Project[]
array in org-projects.component.ts should be moved out for maintainability;
extract the static data into a new module (e.g., projects.data.ts) or a service
(e.g., ProjectsService) that exports/returns Project[] and then import/use it in
the component—replace the inline protected readonly projects with an assignment
from the import (or a call to ProjectsService.getProjects()) and keep the
Project type and property name (projects) unchanged so existing templates and
logic continue to work.
In `@apps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.html`:
- Line 35: The two-column grid uses a fixed "grid-cols-2" which forces two
columns on all breakpoints; update the class on the element containing "grid
grid-cols-2 gap-8" in org-strategy.component.html to use responsive Tailwind
classes (e.g., "grid grid-cols-1 sm:grid-cols-2 gap-8" or "grid grid-cols-1
md:grid-cols-2 gap-8") so it stacks on small screens and becomes two columns at
the desired breakpoint, and apply the same responsive change to the similar
action-links grid around line 46.
In `@apps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.ts`:
- Around line 15-17: Change the setInvolveTab method to accept an InvolveTab
instead of string to enforce compile-time type safety (update the signature of
setInvolveTab and use the activeInvolveTab.set call unchanged), and update any
callers: in the template cast the literal at the call site (e.g. use
$any(tab.id) or an explicit cast) or better yet type the tabs array as
InvolveTab[] so callers pass a correctly typed value without casts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f2f052f-f329-4ab5-91f5-29cb89a61cbd
📒 Files selected for processing (23)
apps/lfx-one/src/app/layouts/main-layout/main-layout.component.tsapps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.htmlapps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.tsapps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.htmlapps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.tsapps/lfx-one/src/app/modules/org-lens/code/org-code.component.htmlapps/lfx-one/src/app/modules/org-lens/code/org-code.component.tsapps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.htmlapps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.tsapps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.htmlapps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.tsapps/lfx-one/src/app/modules/org-lens/org-lens.routes.tsapps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.htmlapps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.tsapps/lfx-one/src/app/modules/org-lens/permissions/org-permissions.component.htmlapps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.htmlapps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.tsapps/lfx-one/src/app/modules/org-lens/project-detail/org-project-detail.component.htmlapps/lfx-one/src/app/modules/org-lens/project-detail/org-project-detail.component.tsapps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.htmlapps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.tsapps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.htmlapps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.ts
✅ Files skipped from review due to trivial changes (1)
- apps/lfx-one/src/app/modules/org-lens/cla/org-cla.component.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/lfx-one/src/app/modules/org-lens/org-lens.routes.ts
- apps/lfx-one/src/app/modules/org-lens/benefits/org-benefits.component.ts
- apps/lfx-one/src/app/modules/org-lens/profile/org-profile.component.ts
- apps/lfx-one/src/app/modules/org-lens/code/org-code.component.ts
- apps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.ts
- apps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.ts
Comment on lines
+51
to
+64
| <select class="px-3 py-1.5 text-sm border border-gray-200 rounded-md text-gray-700 bg-white outline-none cursor-pointer min-w-44"> | ||
| <option>All Projects</option> | ||
| <option>Aether Project</option> | ||
| <option>Kubernetes</option> | ||
| <option>Linux Kernel</option> | ||
| <option>Node.js</option> | ||
| <option>PyTorch Project</option> | ||
| </select> | ||
| <select class="px-3 py-1.5 text-sm border border-gray-200 rounded-md text-gray-700 bg-white outline-none cursor-pointer"> | ||
| <option>Past 365 Days</option> | ||
| <option>Past 90 Days</option> | ||
| <option>Past 30 Days</option> | ||
| <option>All Time</option> | ||
| </select> |
Contributor
There was a problem hiding this comment.
Select filters are not connected to component state.
Project/time selects currently won’t drive filtering because there are no (change) bindings (or equivalent form bindings).
Also applies to: 141-150, 230-238
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html` around
lines 51 - 64, The select elements in org-code.component.html are not bound to
component state; add two-way bindings (e.g., [(ngModel)]="selectedProject" and
[(ngModel)]="selectedTimeRange") or (change) handlers that call a component
method (e.g., onFilterChange() or updateFilters()) so selections update
properties and trigger filtering; implement corresponding properties
(selectedProject, selectedTimeRange) and a filter handler in the
OrgCodeComponent class to apply the filter logic. Repeat the same bindings/fix
for the other select blocks indicated (lines ~141-150 and ~230-238).
Comment on lines
+300
to
+303
| @if (e.speakingTotal !== null) { | ||
| <span class="text-blue-600 font-medium">{{ e.speakingProposals }}</span> | ||
| <span class="text-gray-400"> / {{ e.speakingTotal }}</span> | ||
| } @else { |
Contributor
There was a problem hiding this comment.
speakingTotal guard should be nullish-safe.
Line 300 checks !== null; if speakingTotal is undefined, the truthy branch still renders and can show empty/invalid totals.
✅ Safer conditional
- `@if` (e.speakingTotal !== null) {
+ `@if` (e.speakingTotal != null) {📝 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
| @if (e.speakingTotal !== null) { | |
| <span class="text-blue-600 font-medium">{{ e.speakingProposals }}</span> | |
| <span class="text-gray-400"> / {{ e.speakingTotal }}</span> | |
| } @else { | |
| `@if` (e.speakingTotal != null) { | |
| <span class="text-blue-600 font-medium">{{ e.speakingProposals }}</span> | |
| <span class="text-gray-400"> / {{ e.speakingTotal }}</span> | |
| } `@else` { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/modules/org-lens/code/org-code.component.html` around
lines 300 - 303, The template currently guards e.speakingTotal with a strict !==
null check which misses undefined; update the conditional around e.speakingTotal
(the block rendering {{ e.speakingProposals }} / {{ e.speakingTotal }}) to be
nullish-safe (e.g., use != null or an explicit check for both null and
undefined) so the totals branch only renders when speakingTotal is neither null
nor undefined.
Comment on lines
+95
to
+98
| <div class="flex items-center gap-2 px-3 py-1.5 border border-gray-200 rounded-lg bg-gray-50"> | ||
| <i class="fa-light fa-magnifying-glass text-gray-400 text-sm"></i> | ||
| <input type="text" placeholder="Search by name or email..." class="text-sm outline-none bg-transparent text-gray-700 w-48" /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Search input is non-functional.
The search input in the Access Holders section has no value binding or event handler, making it purely visual. Consider connecting it to filter the displayed users.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/modules/org-lens/groups/org-groups.component.html`
around lines 95 - 98, The search input is only visual; wire it to the
OrgGroupsComponent by adding a component property (e.g., searchQuery) and a
handler (e.g., onSearchChange or filterAccessHolders) that filters the access
holders list used by the template, then bind the input in
org-groups.component.html with two-way binding or input event
([(ngModel)]="searchQuery" and (ngModelChange)="onSearchChange()" or
(input)="onSearchChange()"); ensure FormsModule is imported into the module so
ngModel works and update the component's displayed users array (e.g.,
filteredAccessHolders) from the handler to reflect the search.
Comment on lines
+385
to
+399
| <div> | ||
| <label class="text-xs font-semibold text-gray-500 uppercase tracking-wide mb-1 block">Annual Membership Fee</label> | ||
| <div class="flex items-center gap-2 border border-gray-200 rounded-lg px-3 py-2 bg-gray-50"> | ||
| <span class="text-gray-400 text-sm">$</span> | ||
| <input type="text" value="370000" class="text-sm text-gray-700 bg-transparent outline-none flex-1" /> | ||
| </div> | ||
| </div> | ||
| <div> | ||
| <label class="text-xs font-semibold text-gray-500 uppercase tracking-wide mb-1 block">Employee Hours Contributed / Year</label> | ||
| <input type="number" value="4200" class="w-full border border-gray-200 rounded-lg px-3 py-2 bg-gray-50 text-sm text-gray-700 outline-none" /> | ||
| </div> | ||
| <div> | ||
| <label class="text-xs font-semibold text-gray-500 uppercase tracking-wide mb-1 block">Avg Hourly Rate (USD)</label> | ||
| <input type="number" value="145" class="w-full border border-gray-200 rounded-lg px-3 py-2 bg-gray-50 text-sm text-gray-700 outline-none" /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
ROI calculator inputs lack reactive binding.
The form inputs have hardcoded value attributes but no two-way binding ([(ngModel)]) or reactive form controls. Changes won't update the calculated ROI values displayed below.
If these inputs should be functional (not just a static mockup), they need proper form bindings:
// In component
protected readonly annualFee = signal(370000);
protected readonly hoursContributed = signal(4200);
protected readonly hourlyRate = signal(145);<input type="number" [value]="annualFee()" (input)="annualFee.set(+searchInput.value)" ... />Or if this is intentionally a static prototype, consider adding a comment or disabling the inputs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/lfx-one/src/app/modules/org-lens/membership/org-membership.component.html`
around lines 385 - 399, The ROI inputs in org-membership.component.html are
static because they use hardcoded value attributes instead of bound state;
update the component to expose reactive state (e.g., signals named annualFee,
hoursContributed, hourlyRate or FormControls) and bind the inputs to those
(replace value="..." with reactive bindings and wire input events or
ngModel/FormControl so user edits update the component state), or if the fields
are intentionally non-interactive, mark them disabled or add a comment
indicating they are placeholders; ensure bindings reference the declared symbols
(annualFee, hoursContributed, hourlyRate or the corresponding FormControl names)
so downstream ROI calculations react to changes.
Comment on lines
+325
to
+347
| @for (event of events; track event.name) { | ||
| <tr class="border-b border-gray-50"> | ||
| <td class="py-4 pr-4 align-top"> | ||
| <a class="text-blue-500 font-medium text-sm hover:text-blue-600 cursor-pointer">{{ event.name }}</a> | ||
| <div class="text-xs text-gray-500 mt-1">{{ event.location }}</div> | ||
| </td> | ||
| <td class="py-4 pr-4 align-top text-sm text-gray-900"> | ||
| {{ event.startDate }}{{ event.endDate ? ' - ' + event.endDate : '' }} | ||
| </td> | ||
| <td class="py-4 pr-4 align-top text-sm"> | ||
| <span class="text-blue-500 font-semibold">{{ event.myRegistrants }}</span> | ||
| <span class="text-gray-900"> / {{ event.totalRegistrants }}</span> | ||
| </td> | ||
| <td class="py-4 align-top text-sm"> | ||
| @if (event.speakingTotal !== undefined) { | ||
| <span class="text-blue-500 font-semibold">{{ event.speakingProposals }}</span> | ||
| <span class="text-gray-900"> / {{ event.speakingTotal }}</span> | ||
| } @else { | ||
| <span class="text-gray-900">0</span> | ||
| } | ||
| </td> | ||
| </tr> | ||
| } |
Contributor
There was a problem hiding this comment.
Anchor element missing navigation target.
Line 328 has an <a> element with cursor styling but no href or [routerLink]. This creates an inaccessible element that looks clickable but does nothing, and keyboard users cannot focus it.
Either wire it to the actual event detail route or use a <span> if navigation isn't ready yet.
🔧 Proposed fix (assuming event detail route exists)
<td class="py-4 pr-4 align-top">
- <a class="text-blue-500 font-medium text-sm hover:text-blue-600 cursor-pointer">{{ event.name }}</a>
+ <a [routerLink]="['/org/events', event.name]" class="text-blue-500 font-medium text-sm hover:text-blue-600 cursor-pointer">{{ event.name }}</a>
<div class="text-xs text-gray-500 mt-1">{{ event.location }}</div>
</td>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/modules/org-lens/overview/org-overview.component.html`
around lines 325 - 347, The anchor for each event (the <a> showing {{ event.name
}} inside the for-loop over events) is missing a navigation target and should
either be wired to the event detail route or converted to a non-interactive
element; update the element that renders event.name to use a proper router link
like [routerLink]="['/events', event.id]" (or the app's event detail route) so
it is focusable and keyboard-accessible, or replace the <a> with a <span> (and
remove cursor-pointer styling) if navigation is not yet available; ensure you
update the template where event.name, event.id, and the surrounding row
rendering (the for-loop over events) are defined.
Comment on lines
+214
to
+216
| protected setTimeRange(range: string): void { | ||
| this.timeRange.set(range as TimeRange); | ||
| } |
Contributor
There was a problem hiding this comment.
Unsafe type assertion in setTimeRange.
The method accepts string but casts to TimeRange without validation. If called with an invalid value, the signal will hold an invalid state.
🛡️ Type-safe alternative
- protected setTimeRange(range: string): void {
- this.timeRange.set(range as TimeRange);
+ protected setTimeRange(range: TimeRange): void {
+ this.timeRange.set(range);
}Or with runtime validation:
protected setTimeRange(range: string): void {
if (range === '1y' || range === '2y' || range === 'all') {
this.timeRange.set(range);
}
}📝 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
| protected setTimeRange(range: string): void { | |
| this.timeRange.set(range as TimeRange); | |
| } | |
| protected setTimeRange(range: TimeRange): void { | |
| this.timeRange.set(range); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/lfx-one/src/app/modules/org-lens/project-detail/org-project-detail.component.ts`
around lines 214 - 216, The setTimeRange method currently accepts a string and
unsafely casts it to TimeRange (method: setTimeRange, signal: timeRange, type:
TimeRange); update it to be type-safe by either changing the parameter signature
to accept TimeRange instead of string, or add runtime validation that checks the
incoming range against the allowed values (e.g., '1y', '2y', 'all') before
calling this.timeRange.set, and reject or ignore invalid values to avoid placing
an invalid value into the signal.
Comment on lines
+43
to
+45
| <button class="px-3 py-1.5 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 cursor-pointer"> | ||
| + Add project | ||
| </button> |
Contributor
There was a problem hiding this comment.
Consider gating "Add project" with canEdit().
The "Add project" button appears to be a write action that should likely be restricted to admin users with edit permissions, consistent with other org lens pages that gate mutations behind canEdit().
🛡️ Proposed fix
+ `@if` (canEdit()) {
<button class="px-3 py-1.5 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 cursor-pointer">
+ Add project
</button>
+ }📝 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
| <button class="px-3 py-1.5 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 cursor-pointer"> | |
| + Add project | |
| </button> | |
| `@if` (canEdit()) { | |
| <button class="px-3 py-1.5 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 cursor-pointer"> | |
| Add project | |
| </button> | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/modules/org-lens/projects/org-projects.component.html`
around lines 43 - 45, The "Add project" button is a write action and should be
gated by the component's permission checker; wrap the button in a template-level
guard using the existing canEdit() method (e.g., add *ngIf="canEdit()" around
the button in org-projects.component.html) so only users with edit permissions
see/activate it, and verify the component class exposes the canEdit() method (or
import/forward it) used elsewhere to keep behavior consistent across org lens
pages.
Comment on lines
+47
to
+53
| <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Commission Research</a> | ||
| <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Post a blog to Linux.com</a> | ||
| <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Sponsor Events</a> | ||
| <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Speak at Events</a> | ||
| <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Create a Community Event</a> | ||
| <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Form a New Industry Initiative</a> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Use <button> for action links without navigation destinations.
These <a> elements have cursor-pointer but no href attribute. Screen readers and keyboard navigation expect anchors to have destinations. Since these appear to trigger actions rather than navigate, use <button> elements for better accessibility semantics.
♿ Suggested fix
- <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Commission Research</a>
- <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Post a blog to Linux.com</a>
- <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Sponsor Events</a>
- <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Speak at Events</a>
- <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Create a Community Event</a>
- <a class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer">Form a New Industry Initiative</a>
+ <button type="button" class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer text-left bg-transparent border-0 p-0">Commission Research</button>
+ <button type="button" class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer text-left bg-transparent border-0 p-0">Post a blog to Linux.com</button>
+ <button type="button" class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer text-left bg-transparent border-0 p-0">Sponsor Events</button>
+ <button type="button" class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer text-left bg-transparent border-0 p-0">Speak at Events</button>
+ <button type="button" class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer text-left bg-transparent border-0 p-0">Create a Community Event</button>
+ <button type="button" class="text-blue-500 text-sm hover:text-blue-600 cursor-pointer text-left bg-transparent border-0 p-0">Form a New Industry Initiative</button>Alternatively, if these will navigate somewhere, add proper href attributes and routerLink directives.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.html`
around lines 47 - 53, The anchor elements used for actions ("Commission
Research", "Post a blog to Linux.com", "Sponsor Events", "Speak at Events",
"Create a Community Event", "Form a New Industry Initiative") are
non-navigational and lack hrefs; replace each <a class="...
cursor-pointer">...</a> with a semantically correct <button> (preserving classes
and click handlers) to restore accessibility and keyboard semantics, or if they
actually navigate, add proper href or routerLink attributes instead so screen
readers and keyboard users get correct behavior.
| <div class="bg-gradient-to-br from-blue-900 to-blue-800 rounded-lg p-6 flex flex-col justify-end min-h-64"> | ||
| <div class="text-base font-semibold text-white mb-3">A Deep Dive Into Open Source Community Management</div> | ||
| <div class="flex justify-end"> | ||
| <a class="px-5 py-2 bg-blue-500 text-white rounded-md text-sm font-medium hover:bg-blue-600 cursor-pointer">Read Report</a> |
Contributor
There was a problem hiding this comment.
Same accessibility concern for the "Read Report" link.
If this links to an actual report URL, add the href. Otherwise, use a <button> or consider using the <lfx-button> component per the PR commit summaries mentioning standardization on LFX components.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/lfx-one/src/app/modules/org-lens/strategy/org-strategy.component.html`
at line 59, The anchor element in org-strategy.component.html for "Read Report"
is missing an href and thus is not accessible; if it navigates to a report URL,
add the proper href to the <a> so it becomes a real link (preserve classes) and
ensure target/rel as needed; if it triggers an action, replace the anchor with a
semantic <button> or the standard <lfx-button> component used elsewhere (keeping
the existing classes/styles and wiring up the click handler such as the
component's readReport() method), and add appropriate aria-labels for
accessibility.
asithade
added a commit
that referenced
this pull request
Apr 8, 2026
Rewrite my-meetings component with last/next meeting layout and lens-aware data switching (me=user, project=all, org=hidden). Backend: - Reverse-registrant lookup for upcoming meetings via ITX - Participant-based lookup for past meetings via composite IDs - Normalize email, restore board meeting type filter - Extract shared fetchByIdFilterAndLimit helper - New GET /api/user/past-meetings endpoint Frontend: - All 4 persona dashboards: lens-conditional visibility - Dashboard meeting card: recording button, same-tab nav - Pending actions: two-column grid layout - Maintainer header: lens-aware per PR #391 pattern - Dev toolbar: sync form on external persona changes - Extract initLensSwitchedData generic helper Signed-off-by: Asitha de Silva <[email protected]>
asithade
added a commit
that referenced
this pull request
Apr 8, 2026
* feat(meetings): add lens-aware last/next meeting dashboard layout Replace the scrollable upcoming meetings list with a two-column "Last Meeting" / "Next Meeting" layout on the maintainer dashboard. The component adapts per lens: "me" shows user-specific meetings via registrant-based queries, "project" shows all project meetings, and "org" hides the section entirely. Backend optimized from N+1 queries to a reverse-registrant lookup pattern. Signed-off-by: Asitha de Silva <[email protected]> * feat(meetings): lens-aware dashboard meeting layout Rewrite my-meetings component with last/next meeting layout and lens-aware data switching (me=user, project=all, org=hidden). Backend: - Reverse-registrant lookup for upcoming meetings via ITX - Participant-based lookup for past meetings via composite IDs - Normalize email, restore board meeting type filter - Extract shared fetchByIdFilterAndLimit helper - New GET /api/user/past-meetings endpoint Frontend: - All 4 persona dashboards: lens-conditional visibility - Dashboard meeting card: recording button, same-tab nav - Pending actions: two-column grid layout - Maintainer header: lens-aware per PR #391 pattern - Dev toolbar: sync form on external persona changes - Extract initLensSwitchedData generic helper Signed-off-by: Asitha de Silva <[email protected]> * fix(review): address PR #406 round 2 feedback - user.service.ts: update getUserPastMeetings JSDoc to reflect participant-based lookup (per copilot-pull-request-reviewer) - user.service.ts (frontend): update getUserMeetings JSDoc, remove stale projectUid param docs (per copilot-pull-request-reviewer) - dashboard-meeting-card: split meetingDetailUrl and queryParams for proper routerLink handling (per copilot-pull-request-reviewer) - dashboard-meeting-card: use occurrence start time for recording guard on recurring meetings (per copilot-pull-request-reviewer, coderabbitai) Signed-off-by: Asitha de Silva <[email protected]> --------- Signed-off-by: Asitha de Silva <[email protected]>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
/org/*) with 7 sections as standalone lazy-loaded Angular componentsOrgUserType(Employee / Admin View Only / Admin Edit / Conglomerate Admin) — separate from persona roles — with a dev toolbar selector visible only when the Org lens is activePages
/org/org/membership/org/projects/org/code/org/groups/org/profile/org/benefitsKey Technical Changes
OrgUserTypetype added to@lfx-one/shared/interfacesand constantsAppService.orgUserTypesignal +setOrgUserType()methodactiveLens === 'org'app.routes.tsusesloadChildrenfor theorgparent path viaORG_LENS_ROUTEScanEdit()— onlyadmin-editandconglomerate-adminsee action buttonsTest plan
yarn buildpasses with no errors🤖 Generated with Claude Code