fix(nav): stabilize sidebar @for track keys for me-lens menu

[nunoeufrasio]

What

Fixes the Me-lens sidebar rendering in a garbled state on load — menu items appearing under the wrong section headers (e.g. Mentorships / Badges under Security instead of My Growth), then re-shuffling into the correct layout a few seconds later, with some icons (Training & Certifications, Badges) staying wrong even after it settled.

Why

The sidebar menu is built from computed signals that depend on data resolving after first paint — feature flags (default false, real values arrive when LaunchDarkly reaches READY) and persona/lens roles (cookie defaults, then /api/user/personas). So the menu paints once, then recomputes into a different shape.

Two things turned that clean recompute into a garbled one:

1. Unstable @for track keys. Every loop in sidebar.component.html tracked by item.label / childItem.label. Labels are not a stable identity across the reshape, so Angular reused and mis-mapped DOM nodes → items rendered under the wrong section header.
2. Font Awesome kit JS. Icons load via the SVG+JS kit, which replaces each <i class="fa-…"> with a baked-in <svg> on first render. When a node was reused (because of the label-based track) and its [ngClass]="item.icon" changed, FA had already converted the node and didn't re-render it → the wrong icon stuck permanently.

How

Track menu items by a stable per-destination key instead of by label, using an inline track expression (the repo convention — no functions in templates):

@for (item of itemsWithTestIds(); track item.routerLink ?? item.url ?? item.label) {

Applied to all four @for loops (top-level items, section children, group children, footer items), with the childItem equivalent in the nested loops. Each leaf item has a unique routerLink/url; sections and command-only items fall back to their (unique) label. A condensed HTML comment above the first loop documents the root cause. Because icons are static per destination, retained nodes never need an icon swap and genuinely new items get a fresh <i> that FA renders correctly — fixing both the misplacement and the stale icons.

Verification

• yarn check-types, yarn lint:check, and yarn build all pass.
• Live visual verification was blocked in this environment: the app's Auth0 redirect_uri is pinned to localhost:4200 and that port was held by another process, so an authenticated Me-lens render couldn't be captured. The change is a mechanical control-flow track-key fix with no behavioral or styling change; data-testid output is unchanged.

Notes

• No JIRA ticket associated with this branch.
• The in-flight feat/navigation-improvements refactor moves this menu into SidebarNavService and carries the same four label-tracked loops; the same inline track item.routerLink ?? item.url ?? item.label key should be applied there when it rebases/merges.

[coderabbitai]

Walkthrough

The sidebar template now tracks repeated items by routerLink, url, or label fallback instead of label-only keys across the main list, child lists, footer items, and grouped child items.

Changes

Sidebar item tracking

Layer / File(s)	Summary
Stable track expressions apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html	The main, child, footer, and grouped child @for loops now use destination-derived tracking keys, and the main loop includes a comment explaining the choice.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly names the sidebar track-key stabilization and matches the main change.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The description accurately explains the sidebar track-key fix and its effect on misrendered sections and icons.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/me-lens-menu

---

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR fixes a Me-lens sidebar rendering bug where menu items briefly appeared under the wrong section headers (and some icons stuck permanently wrong) on load. The sidebar menu is built from computed signals that depend on feature flags and persona/lens data resolving after first paint, so the menu recomputes into a different shape. Because the four @for loops tracked by item.label, Angular reused and mis-mapped DOM nodes during the reshape; combined with Font Awesome's kit JS baking an <svg> into each <i> on first render, reused nodes retained stale icons. The fix introduces a stable per-destination track key.

Changes:

• Added trackNavItem(item) returning item.routerLink ?? item.url ?? item.label as a stable identity for @for tracking.
• Applied it to all four sidebar menu loops (top-level items, section children, group children, footer items), replacing the previous label-based tracking.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts	Adds the trackNavItem helper providing a stable per-destination identity, with a comment explaining the reshape/Font-Awesome root cause.
apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html	Switches the four sidebar @for loops from track item.label/track childItem.label to track trackNavItem(...).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[MRashad26]

Code Review — PR #1059 fix(nav): stabilize sidebar @for track keys for me-lens menu

Overview

Correct root-cause diagnosis: label-based @for track keys were unstable across the two-phase render (LaunchDarkly + persona API resolve after first paint), causing Angular to reuse mis-mapped DOM nodes. Because Font Awesome's SVG+JS kit bakes <svg> into each <i> on first paint, reused nodes kept stale icons. Using routerLink ?? url ?? label as the stable key is the right fix — every leaf item has a unique destination, and sections fall back to their unique label.

Secrets / critical-constants check ✅

No secrets introduced.

Code-standards audit

#	Severity	Finding
1	🔴 Critical	track trackNavItem(item) calls a component method in 4 @for loops

Code quality notes

• The stable-key derivation logic is correct (routerLink ?? url ?? label). ✅
• All four @for loops updated — no missed occurrences. ✅
• ⚠️ Coordination note: the in-flight PR #1057 (feat/navigation-improvements) introduces the same four label-tracked @for loops independently and also adds trackNavItem. When these two branches merge, the inline-expression approach (see critical finding) removes the conflict.

Verdict: FAIL ❌ (1 critical)

One-line fix per loop — swap to inline expression and drop the method.

[nunoeufrasio]

Review Feedback Addressed

Commit: f284908

Changes Made

• sidebar.component.html: replaced track trackNavItem(...) with the inline expression track (item.routerLink ?? item.url ?? item.label) (and the childItem equivalents) in all four @for loops, matching the repo's no-functions-in-templates convention (per @MRashad26)
• sidebar.component.ts: removed the now-unused trackNavItem method and its comment block; preserved the root-cause rationale as a condensed HTML comment above the first loop

Validation

yarn check-types, yarn lint:check, and yarn build all pass. The track logic is unchanged — this is a pure convention alignment, so the rendering fix from the original commit is preserved.

Coordination note

The observation about PR #1057 (feat/navigation-improvements) carrying the same four label-tracked loops still stands — when it rebases/merges, the same inline-expression track key should be applied there.

Threads Resolved

1 of 1 unresolved thread addressed in this iteration.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[luismoriguerra]

Audit: PR #1059 — fix(nav): stabilize sidebar @for track keys for me-lens menu

nunoeufrasio · f284908d → main · OPEN · 2 files, +14/−4

Scope: Surgical fix for Me-lens sidebar DOM reuse bug

1. Why

Label-based @for track keys + post-paint menu reshape + Font Awesome SVG bake-in → garbled Me-lens sidebar on load.

2. How

Four @for loops use inline track item.routerLink ?? item.url ?? item.label; trackNavItem() removed (addresses @MRashad26).

3. Is it correct

Yes — correct and convention-compliant at latest head. CI Code Quality Checks FAIL — same Prettier issue (remove parens around track expressions).

4. Gaps

1. Blocking — CI format:check on sidebar.component.html
2. Non-blocking — 7-line HTML comment exceeds one-line convention but documents valuable FA root cause

5. Q&A

• Ship independently of feat(navigation): move lens switcher into sidebar tabs #1057? Yes — land if feat(navigation): move lens switcher into sidebar tabs #1057 blocked; otherwise feat(navigation): move lens switcher into sidebar tabs #1057 supersedes.
• Why routerLink ?? url ?? label? Unique destination per leaf; sections fall back to unique label.

6. Business rules & ADRs

• Sidebar @for must track stable per-destination identity, not display labels.
• ADR status: none — UI-only.

Verdict: 🔴 Required changes (blocked-on-CI-format; then ✅ approved as standalone hotfix)

Coordination: If #1057 merges first → close #1059 as superseded.

[luismoriguerra]

Audit inline: CI format blocker on track expressions.

[nunoeufrasio]

Review Feedback Addressed

Commit: 1cabd12

Changes Made

• sidebar.component.html: removed the parentheses around the ?? track expressions in all four @for loops, resolving the Prettier / format:check CI blocker (per @luismoriguerra)

Validation

yarn prettier --check and yarn lint:check both pass on the file. Formatting-only change — no semantic difference to the track keys.

Threads Resolved

1 of 1 unresolved thread addressed in this iteration.

[luismoriguerra]

Re-audit: PR #1059 — fix(nav): stabilize sidebar @for track keys for me-lens menu

Head: 1cabd121edbc019977eeb7e4e52319822df52ad7 · Verdict: ✅ Approved

What changed since last audit

• fix(review): fix prettier formatting on track expressions — removes parens so format:check passes.

Prior findings → status

Finding	Status
CI format:check	✅ Fixed
trackNavItem() method in template	✅ Fixed (inlined)

CI & gates

Code Quality Checks and all other checks — pass. Local format:check and lint:check green.

Merge readiness: safe-to-merge as standalone hotfix. If #1057 lands first, this PR becomes redundant.

---

[luismoriguerra]

Re-audit clean: format:check and inlined @for track keys are fixed. CI and local lint/format checks pass. Approved as safe-to-merge hotfix.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[nunoeufrasio]

Review Feedback Addressed

Documentation Update (no code change)

• PR description: updated the "How" section to show the inline track item.routerLink ?? item.url ?? item.label expression (the description still described a trackNavItem helper that was removed earlier in review), and corrected the PR feat(navigation): move lens switcher into sidebar tabs #1057 coordination note to reference the inline track key (flagged by copilot-pull-request-reviewer)

No Change Needed

• Did not re-extract a trackNavItem helper: a component method in a track expression violates the repo's no-functions-in-templates convention (the reason it was removed earlier). The inline expression repeated across the four loops is the established repo pattern.

Threads Resolved

1 of 1 unresolved thread addressed in this iteration. No code change, so no new commit.