diff --git a/client/src/components/Layout/navItems.test.ts b/client/src/components/Layout/navItems.test.ts index 0b68577fc..a3dd9899e 100644 --- a/client/src/components/Layout/navItems.test.ts +++ b/client/src/components/Layout/navItems.test.ts @@ -25,14 +25,33 @@ describe('buildNavItems', () => { }) describe('splitMobileNav', () => { - it('falls back to Dashboard + first two in the bar when there is no config', () => { + it('docks the top two enabled defaults when there is no config', () => { for (const cfg of [undefined, { bar: [], more: [] }]) { const s = splitMobileNav(items, cfg) - expect(ids(s.bar)).toEqual(['dashboard', 'vacay', 'atlas']) - expect(ids(s.more)).toEqual(['journey', 'collections']) + expect(ids(s.bar)).toEqual(['dashboard', 'vacay', 'journey']) + // Atlas is still reachable, one tap further under More. + expect(ids(s.more)).toEqual(['atlas', 'collections']) } }) + it('keeps the Vacay/Atlas dock on an instance where Journey is switched off', () => { + const s = splitMobileNav([item('dashboard'), item('vacay'), item('atlas')], undefined) + expect(ids(s.bar)).toEqual(['dashboard', 'vacay', 'atlas']) + expect(ids(s.more)).toEqual([]) + }) + + it('fills the free slot from the next default rather than leaving it empty', () => { + const s = splitMobileNav([item('dashboard'), item('atlas'), item('journey')], undefined) + expect(ids(s.bar)).toEqual(['dashboard', 'journey', 'atlas']) + expect(ids(s.more)).toEqual([]) + }) + + it('leaves an account that picked its own dock alone', () => { + const s = splitMobileNav(items, { bar: ['vacay', 'atlas'], more: ['journey', 'collections'] }) + expect(ids(s.bar)).toEqual(['dashboard', 'vacay', 'atlas']) + expect(ids(s.more)).toEqual(['journey', 'collections']) + }) + it('honours a custom split and order, Dashboard still first', () => { const s = splitMobileNav(items, { bar: ['journey'], more: ['collections', 'vacay', 'atlas'] }) expect(ids(s.bar)).toEqual(['dashboard', 'journey']) diff --git a/client/src/components/Layout/navItems.ts b/client/src/components/Layout/navItems.ts index 61db772de..9cc34db93 100644 --- a/client/src/components/Layout/navItems.ts +++ b/client/src/components/Layout/navItems.ts @@ -35,8 +35,18 @@ const ADDON_ICONS: Record = { /** The bar holds Dashboard + at most this many custom items; the rest go to More. * Two keeps the dock at 3 destinations (Dashboard + 2) so the More slot still fits. */ export const MOBILE_NAV_MAX_BAR = 2 -/** The built-in dock next to Dashboard for an un-customised account; everything else starts under More. */ -export const DEFAULT_DOCK_IDS = ['vacay', 'atlas'] +/** + * The built-in dock next to Dashboard for an un-customised account, in priority + * order: the first MOBILE_NAV_MAX_BAR of these that are actually enabled take the + * slots, everything else starts under More. + * + * Journey outranks Atlas because a phone in the field wants the journal it writes + * to every day, not the map of countries it already visited. Journey ships + * disabled though, so an instance that never turned it on keeps the Vacay/Atlas + * dock it has today. Listing all four known global addons rather than just the + * top two also keeps the dock filled when one of them is switched off. + */ +export const DEFAULT_DOCK_IDS = ['vacay', 'journey', 'atlas', 'collections'] export function buildNavItems( globalAddons: { id: string; name: string; icon: string }[], @@ -90,10 +100,10 @@ export interface MobileNavSplit { /** * Resolve the persisted `{ bar, more }` id split against the live nav items. * Dashboard is always pinned first in the bar. An empty/absent config falls back - * to the built-in dock (Dashboard + Vacay/Atlas where enabled, everything else - * under More) so an un-customised user sees exactly today's layout. Stored ids - * that no longer resolve are dropped; newly available items are appended under - * More. + * to the built-in dock (Dashboard + the top DEFAULT_DOCK_IDS that are enabled, + * everything else under More). Stored ids that no longer resolve are dropped; + * newly available items are appended under More, so an account that once picked + * its own dock keeps it untouched. */ export function splitMobileNav( items: NavItemDef[], @@ -102,16 +112,20 @@ export function splitMobileNav( const dashboard = items.find((i) => i.id === 'dashboard') const head = dashboard ? [dashboard] : [] const rest = items.filter((i) => i.id !== 'dashboard') + const byId = new Map(rest.map((i) => [i.id, i])) + const pick = (ids: string[]) => ids.map((id) => byId.get(id)).filter((x): x is NavItemDef => !!x) if (!cfg || (cfg.bar.length === 0 && cfg.more.length === 0)) { + const dock = pick(DEFAULT_DOCK_IDS).slice(0, MOBILE_NAV_MAX_BAR) + const docked = new Set(dock.map((i) => i.id)) return { - bar: [...head, ...rest.filter((i) => DEFAULT_DOCK_IDS.includes(i.id))], - more: rest.filter((i) => !DEFAULT_DOCK_IDS.includes(i.id)), + bar: [...head, ...dock], + // Store order, not DEFAULT_DOCK_IDS order, so More keeps reading like the + // addon list itself. + more: rest.filter((i) => !docked.has(i.id)), } } - const byId = new Map(rest.map((i) => [i.id, i])) - const pick = (ids: string[]) => ids.map((id) => byId.get(id)).filter((x): x is NavItemDef => !!x) const known = new Set([...cfg.bar, ...cfg.more]) const barPicked = pick(cfg.bar) diff --git a/client/src/components/Settings/useMobileNavEditor.test.tsx b/client/src/components/Settings/useMobileNavEditor.test.tsx index dc5244992..6dfe0f065 100644 --- a/client/src/components/Settings/useMobileNavEditor.test.tsx +++ b/client/src/components/Settings/useMobileNavEditor.test.tsx @@ -38,8 +38,8 @@ describe('useMobileNavEditor', () => { it('FE-COMP-NAVEDITOR-001: an empty value falls back to the built-in dock split', () => { const { result } = setup({ bar: [], more: [] }); - expect(ids(result.current.barItems)).toEqual(['vacay', 'atlas']); - expect(ids(result.current.moreItems)).toEqual(['journey', 'collections']); + expect(ids(result.current.barItems)).toEqual(['vacay', 'journey']); + expect(ids(result.current.moreItems)).toEqual(['atlas', 'collections']); expect(result.current.hasMore).toBe(true); expect(result.current.barFull).toBe(true); });