Derive Calendar/Goals/Insights/Privacy/Messages/Wiki tabs from the nav manifest - #6385
Merged
Conversation
…v manifest (#6365) These six pages each hand-maintained a local TABS array that duplicated the same destinations already declared in server/lib/navManifest.js, kept in sync only by a source-scraping drift test. Extend the tabGroup/getPageNavTabs mechanism Settings and Models already use (3c8799a) so the manifest is the one registry: each manifest entry now carries `tabGroup` (+ an optional `tabLabel` for the four short page-local labels that differ from the sidebar/⌘K label — Goals' List/Tree, Insights' and Privacy's Overview, and Wiki's Overview), and each page derives its TABS from `getPageNavTabs(group)` merged with a small local presentation map (icon, fullBleed, needsAccounts) that throws at import time if a manifest tab has no matching presentation entry. `tabId` uniqueness is now scoped to `tabGroup` when present (falling back to the existing per-section scope for Settings/Models), since Insights and Privacy both use the id "overview" while sharing the "Identity" section. `getSectionNavTabs`/`getNavSectionForPath` explicitly exclude tabGroup entries so the existing Settings/Models section child-nav is unaffected. navManifest.test.js drops these six pages' TABBED_PAGES rows (and their source-scraping); each page's own test file now asserts its TABS match `getPageNavTabs(group)` in id, label and declaration order instead. Brain, CoS, Digital Twin, MeatSpace, Media Gen, Music, Sharing and System Resources still use the old scraper — converting them needs either new manifest entries for tabs the two registries had already lost sync on (Brain's Spotify/YouTube tabs have no manifest entry at all) or handling a different source shape (Sharing's link list, POST's switch dispatch). Follow-up: #6383.
5 tasks
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
1 participant
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
tabGroup/getPageNavTabs(group)mechanism Settings and Models already use (3c8799a) to six more pages: Calendar, Goals, Insights, Privacy, Messages, Wiki. Each now derives itsTABSfrom the nav manifest plus a small local presentation map (icon,fullBleed,needsAccounts), instead of hand-maintaining a duplicate{ id, label }array kept in sync only by a source-scraping drift test.tabLabelfor the four page-local labels that differ from the sidebar/⌘K label (Goals' List/Tree, Insights' and Privacy's Overview, Wiki's Overview) — those render unchanged in the page tab bars and unchanged in the sidebar/⌘K.tabIduniqueness is now scoped totabGroupwhen present (falling back to the existing per-section scope for Settings/Models), since Insights and Privacy both use the idoverviewwhile sharing the "Identity" sidebar section.getSectionNavTabs/getNavSectionForPathexplicitly excludetabGroupentries so Settings/Models section child-nav is unaffected.navManifest.test.jsdrops these six pages'TABBED_PAGESrows and their source-scraping; each page's own test now asserts itsTABSmatchgetPageNavTabs(group)in id, label and declaration order.Remaining
Brain, CoS, Digital Twin, MeatSpace, Media Gen, Music, Sharing and System Resources still use the old scraper mechanism — filed as #6383. Two things that mechanical conversion needs to handle there: Brain's
spotify/youtubetabs currently have no manifest entry at all (a pre-existing gap, unrelated to this PR), and Sharing's tab list is a different source shape ({ to/path, label }links mixed with cross-links) than the flat{ id, label }arrays the other pages use.Refs #6365
Test plan
cd server && npx vitest run lib/navManifest.test.js— 77 passedcd client && npx vitest run src/pages/Calendar.test.jsx src/pages/Goals.test.jsx src/pages/Insights.test.jsx src/pages/Privacy.test.jsx src/pages/Messages.test.jsx src/pages/Wiki.test.jsx— 32 passedcd server && npm test— full suite green (2007 files, 39902 tests)cd client && npm test— full suite green aside from one pre-existing flake inStartStory.test.jsx(unrelated page, passes in isolation)