fix(nav): default project-selector to Projects tab on Project lens#717
fix(nav): default project-selector to Projects tab on Project lens#717manishdixitlfx wants to merge 3 commits into
Conversation
For hybrid personas (users with both board and project roles), the Project lens button serves as a merged entry that shows both Foundations and Projects in the picker. The active tab defaulted to "All", which sorts foundations first per the load-priority comment — so opening the picker on Project lens led with a Foundation row (e.g. AAIF tagged Executive Director) even though the user just clicked Project. Fix: compute a defaultTab signal that returns 'projects' when the lens is 'project' and 'all' otherwise, then set activeTab to defaultTab() on both panel show and panel hide. Users on Project lens now see Projects first; they can still switch to Foundations or All inside the picker. Tabs only render in hybrid mode, so non-hybrid users (project-only or board-only personas) see no UX change. Verified in Chrome on localhost: Project lens entry opens the picker with the Projects tab active and EnerGNN (the active project) as the first result. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughProjectSelectorComponent now computes a default tab based on the current lens input, defaulting to ChangesDefault tab logic based on lens
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adjusts the project selector popover behavior for hybrid personas so that when the user is in the Project lens, the selector defaults to the Projects tab (instead of All), ensuring project results are shown first.
Changes:
- Add a
defaultTabcomputed signal that mapslens === 'project'to'projects', otherwise'all'. - Reset
activeTabtodefaultTab()when the popover opens and when it closes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
For hybrid personas (board + project roles), the Project lens button is the merged entry — the Foundation button is hidden. The default-picker used PROJECT_SCOPED_PERSONA_PRIORITY which skips board-member entirely, so a user with board roles on TLF / AAIF / TPG-IT was being defaulted to whichever contributor project the server returned first (e.g. LFX One via 4368 contributions), ignoring their highest-authority role. Fix: - pickItemByPersonaPriority now uses the full PERSONA_PRIORITY (executive-director, board-member, maintainer, contributor) when the user is a hybrid persona, so board roles aren't skipped. - Among same-persona matches, prefer the root-most project (one whose parentProjectUid is outside the user's accessible item set). That way The Linux Foundation beats AAIF when both carry board-member, since AAIF's parent is TLF (in the user's set) and TLF's parent is the org meta uid (outside the set). - getPriorityUid uses the same picker so the priority signal stays in sync with the default-selection signal. Foundation lens is unchanged (still uses BOARD_SCOPED_PERSONA_PRIORITY). Non-hybrid project-only users are unchanged (still PROJECT_SCOPED_PERSONA_PRIORITY). Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
dealako
left a comment
There was a problem hiding this comment.
Hey Manish! Nice, focused PR — two well-scoped fixes that tighten up the hybrid-persona experience without disturbing non-hybrid or foundation-only users. The commit 1 tab-default change is clean and the rationale is obvious. Commit 2 is the meatier one and overall correct, though I have one clarification question on its stated behaviour before you land it.
0 blocking · 3 minor · 2 nit
Decision: Approved with minor comments — the clarification in commit 2 is worth confirming before merge, but nothing rises to blocking.
AI bot reconciliation
- CodeRabbit: PR is in draft, skipped — no findings to reconcile.
- Copilot: summarised commit 1's tab-default change only; did not surface commit 2 or any findings. My comments extend coverage to commit 2 with no conflicts.
- Cursor Bot: no comments on this PR.
Revision tracking: first human review round.
[minor] PR description / test plan
The title and description cover commit 1 (tab default) only. Commit 2 makes a substantive change to the default-selection algorithm — PROJECT_SCOPED_PERSONA_PRIORITY → full PERSONA_PRIORITY for hybrid + root-most tiebreaker — with no corresponding test-plan entry. Recommend adding a bullet like: "Hybrid user with board roles on multiple items and contributor activity elsewhere — verify default selection lands on a board-role project, not the top contributor result."
| /** | ||
| * Project lens for a hybrid persona is the merged entry (Foundation button is hidden); use the | ||
| * full priority so the user's highest-authority role wins instead of falling through to contributor. | ||
| */ | ||
| private getPickerPriority(lens: NavLens): readonly PersonaType[] { | ||
| if (lens === 'foundation') return BOARD_SCOPED_PERSONA_PRIORITY; | ||
| return this.lensService.isHybridPersona() ? PERSONA_PRIORITY : PROJECT_SCOPED_PERSONA_PRIORITY; | ||
| } | ||
|
|
||
| /** Prefer items where the user holds an in-priority persona; among same-persona matches prefer root-most (parent outside the user's accessible set). Falls back to the first item. */ | ||
| private pickItemByPersonaPriority(items: LensItem[], priority: readonly PersonaType[]): LensItem { | ||
| const personaProjects = this.personaService.personaProjects(); | ||
| const itemUids = new Set(items.map((i) => i.uid)); | ||
| const enrichedByUid = new Map(this.personaService.detectedProjects().map((p) => [p.projectUid, p])); | ||
| for (const persona of priority) { | ||
| const projects = personaProjects[persona] ?? []; | ||
| const matches: { item: LensItem; isRoot: boolean }[] = []; | ||
| for (const project of projects) { | ||
| const match = items.find((item) => item.uid === project.projectUid); | ||
| if (match) return match; | ||
| const item = items.find((i) => i.uid === project.projectUid); | ||
| if (!item) continue; | ||
| const parentUid = enrichedByUid.get(item.uid)?.parentProjectUid; | ||
| const isRoot = !parentUid || !itemUids.has(parentUid); | ||
| matches.push({ item, isRoot }); | ||
| } | ||
| if (matches.length === 0) continue; | ||
| // Prefer a root-level match (no in-set parent) so e.g. The Linux Foundation beats AAIF when both carry board-member. | ||
| return (matches.find((m) => m.isRoot) ?? matches[0]).item; | ||
| } | ||
| return items[0]; | ||
| } |
There was a problem hiding this comment.
[minor] Clarification: does the TLF-beats-AAIF root-most scenario actually reach this code for hybrid project-lens users?
The commit message (and the comment at line 207) describe preferring The Linux Foundation over AAIF when both carry board-member, because AAIF's parent is TLF (in the user's set) and TLF's parent is the org meta UID (outside the set).
But applyVisibilityFilters (lines 391-396) strips isFoundation items from project-lens results whenever availableLenses contains 'foundation' — which it does for every hybrid user, because LensService.getAllowedLensIds() adds 'foundation' whenever hasBoardRole() is true (lens.service.ts:73-85). This was an explicit choice in PR #713 (fix(nav): restore availableLenses to full allowed set … keep filtering foundations out of the project lens for hybrid users).
So for the hybrid project-lens case pickItemByPersonaPriority sees a foundation-free items array: TLF is never present, and the described tiebreak between TLF and AAIF can't materialise on this lens. The core fix — board-member matches rank ahead of contributor matches — is still meaningful and correct. But the root-most preference seems to apply on the foundation lens (where items aren't filtered) and to non-hybrid project-only users, not the scenario described.
Could you confirm whether I'm reading the filter interaction correctly, or point to where TLF does appear in the project-lens item set for hybrid users? Happy to be corrected if there's a data path I'm missing.
| matches.push({ item, isRoot }); | ||
| } | ||
| if (matches.length === 0) continue; | ||
| // Prefer a root-level match (no in-set parent) so e.g. The Linux Foundation beats AAIF when both carry board-member. |
There was a problem hiding this comment.
[minor] Comment wording: "item set" implies the user's full accessible set, not the current page
itemUids is built from items.map(i => i.uid) — the items in the current page only. In a paginated context an entry could appear root-most simply because its parent hasn't been loaded yet. Suggest: "parent outside the current page's loaded items" (or similar) to make the scope clear.
| protected readonly activeTab = signal<SelectorTab>('all'); | ||
| protected readonly selectorTabs: readonly SelectorTab[] = ['all', 'foundations', 'projects']; | ||
| // Project lens defaults the picker to its matching tab so projects rank first; foundation lens uses 'all' (the only other hybrid case, where users explicitly went to a board context). | ||
| private readonly defaultTab: Signal<SelectorTab> = computed(() => (this.lens() === 'project' ? 'projects' : 'all')); |
There was a problem hiding this comment.
[nit] Visibility grouping
private readonly defaultTab sits between the protected readonly fields above and below it. Per the component-organization rule the structure order groups fields by visibility. Cosmetic — easy to move if you're already in the file.
|
|
||
| protected readonly activeTab = signal<SelectorTab>('all'); | ||
| protected readonly selectorTabs: readonly SelectorTab[] = ['all', 'foundations', 'projects']; | ||
| // Project lens defaults the picker to its matching tab so projects rank first; foundation lens uses 'all' (the only other hybrid case, where users explicitly went to a board context). |
There was a problem hiding this comment.
[nit] Long inline comment (~180 chars)
The parenthetical // … the only other hybrid case, where users explicitly went to a board context pushes this past comfortable reading width. Could trim or wrap — non-blocking.
| const item = items.find((i) => i.uid === project.projectUid); | ||
| if (!item) continue; | ||
| const parentUid = enrichedByUid.get(item.uid)?.parentProjectUid; | ||
| const isRoot = !parentUid || !itemUids.has(parentUid); | ||
| matches.push({ item, isRoot }); |
| for (const project of projects) { | ||
| const match = items.find((item) => item.uid === project.projectUid); | ||
| if (match) return match; | ||
| const item = items.find((i) => i.uid === project.projectUid); |
Summary
For hybrid personas (users with both board and project roles), the Project lens button serves as a merged entry that shows both Foundations and Projects in the picker. The active tab defaulted to "All", which sorts foundations first per the load-priority comment — so opening the picker on Project lens led with a Foundation row (e.g. AAIF tagged "Executive Director") even though the user just clicked Project.
Fix
Compute a
defaultTabsignal that returns'projects'whenlens === 'project','all'otherwise. SetactiveTabtodefaultTab()on both panel show and panel hide:Tabs only render in hybrid mode (
@if (hybridMode())), so non-hybrid users (project-only or board-only personas) see no UX change.Test plan
Verified in Chrome on
localhost:4200with a hybrid persona (ED + Board Member + Contributor):/project/overview?project=...Out of scope
The deeper question of whether
setLens('project')should also auto-defaultcurrentPersonato a project-scoped persona (maintainer→contributor) is intentionally left out — that's a separate behavior change inLensService/PersonaServiceand warrants its own discussion. The constantsPROJECT_SCOPED_PERSONA_PRIORITYalready exist for that follow-up.🤖 Generated with Claude Code