fix(frontend): highlight the portal item the content is showing - #2493
hello1101n merged 2 commits into
Conversation
The context pane matched `?item=` against its item ids, so a URL naming no item highlighted nothing while the content area rendered the zone's default view, and an item left behind by another zone highlighted nothing while that zone's view fell back to its own default. Resolve the effective item once in nav-model (`resolveZoneItem`: the URL item if the zone lists it, else the zone's default) and read it from both the pane and the zone-content dispatcher, so the menu and the content can no longer name different things. Signed-off-by: hello1101n <hello1101n@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughChangesPortal navigation resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to This change centralizes portal item selection so navigation highlighting and displayed content stay aligned, including default and invalid-item cases. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Portal
participant ContextPane
participant NavModel
participant Navigation
participant ZoneContent
participant ZoneView
Portal->>ContextPane: render active zone and portal item
ContextPane->>NavModel: resolveZoneItem(activeZone, portalItem)
NavModel-->>ContextPane: active item or null
ContextPane->>Navigation: render navigation with active item
Portal->>ZoneContent: render active zone and selected item
ZoneContent->>NavModel: resolveZoneItem(activeZone, selectedItem)
NavModel-->>ZoneContent: normalized item or null
ZoneContent->>ZoneView: render normalized item
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem
The context pane matched
?item=against its own item ids, so the highlight and the content could disagree in two ways:itemin the URL —/portal?zone=overviewrenders At a glance, but the menu marked nothing. The reader sees content with no idea which entry produced it.itembelonging to another zone — e.g. a People route still carryingitem=trend. The view falls back to the roster, the pane matches nothing, and again no row is marked.Overview, AI & Cost and People were all affected; each of those views already falls back to a default item internally, so the disagreement was structural rather than a one-off.
Fix
Resolve the effective item once, in the nav model:
Both the context pane and the
ZoneContentdispatcher read it, so the menu and the content can no longer name different things.ZONE_DEFAULT_ITEMis the single place a zone default is written;DEFAULT_OVERVIEW_ITEMnow derives from it instead of restating the id.Zones deliberately absent from the map:
itemit renders a generic scaffold that is no menu row, so nothing should be highlighted.PersonSectionsNav.Behaviour change
?zone=overview&item=<unknown-id>now renders the default view instead of “x isn't an Overview view yet.” — unavoidable if the menu and the content must agree on one value.Testing
nav-model.test.ts— defaults name a zone the rail has and an item the pane always renders (never aplanned/unbuiltrow);resolveZoneItemkeeps a listed item, falls back for a foreign one, falls back when none is named, stays null for Manage.context-pane.test.tsx— default highlighted for each of the three zones, highlight follows an explicititem, a foreignitemis ignored, Manage still highlights nothing.typecheckandlintclean.Summary by CodeRabbit
Bug Fixes
Tests
Closes #2494
Screenshots
Local compose install, demo seed roster. Rail icon and pane row both mark the open view.
?zone=overview?zone=aicost/ic/<person>/team?item=trend?zone=overviewafter the fix:/ic/<person>/team— roster marked:?zone=overview&item=<unknown-id>— the behaviour change above, rendering the default view:?zone=managewith no item — nothing marked, deliberately: