diff --git a/client/src/components/Layout/BottomNav.tsx b/client/src/components/Layout/BottomNav.tsx index a252dc697..bb5290c2b 100644 --- a/client/src/components/Layout/BottomNav.tsx +++ b/client/src/components/Layout/BottomNav.tsx @@ -1,4 +1,4 @@ -import { useNavigate, useLocation, useMatch } from 'react-router' +import { useNavigate, useLocation } from 'react-router' import { useAddonStore } from '../../store/addonStore' import { usePluginStore } from '../../store/pluginStore' import { useSettingsStore } from '../../store/settingsStore' @@ -8,6 +8,7 @@ import type { LucideIcon } from 'lucide-react' import { resolvePluginIcon } from '../shared/PluginIcon' import { useAuthStore } from '../../store/authStore' import { visibleManagedNavItems } from '../../managed' +import { isNavItemActive, useCreateAction } from './useCreateAction' const ADDON_NAV: Record = { vacay: { icon: CalendarDays, labelKey: 'admin.addons.catalog.vacay.name' }, @@ -18,37 +19,6 @@ const ADDON_NAV: Record = { interface NavItem { to: string; label: string; icon: LucideIcon } -// The centre "+" means something different per context: inside a trip it adds a -// place, on the journey list it starts a journey, inside a journey it adds an -// entry — everywhere else it creates a new trip. Pages pick the intent up from -// the ?create= query param. -function useCreateAction(): { label: string; run: () => void } { - const navigate = useNavigate() - const { t } = useTranslation() - const inTrip = useMatch('/trips/:id') - const inJourney = useMatch('/journey/:id') - const onJourneyList = useMatch('/journey') - - if (inTrip) { - // The "+" is context-aware per active tab: Bookings → reservation, - // Transports → transport, Costs → expense. Tabs without a create modal - // (lists / files / collab) fall through to adding a place. #1349 - const id = inTrip.params.id - const tripTab = typeof sessionStorage !== 'undefined' ? sessionStorage.getItem(`trip-tab-${id}`) : null - if (tripTab === 'finanzplan') return { label: t('costs.addExpense'), run: () => navigate(`/trips/${id}?create=expense`) } - if (tripTab === 'buchungen') return { label: t('reservations.addManual'), run: () => navigate(`/trips/${id}?create=reservation`) } - if (tripTab === 'transports') return { label: t('transport.addManual'), run: () => navigate(`/trips/${id}?create=transport`) } - return { label: t('places.addPlace'), run: () => navigate(`/trips/${id}?create=place`) } - } - if (inJourney) { - return { label: t('journey.detail.addEntry'), run: () => navigate(`/journey/${inJourney.params.id}?create=entry`) } - } - if (onJourneyList) { - return { label: t('journey.new'), run: () => navigate('/journey?create=1') } - } - return { label: t('dashboard.newTrip'), run: () => navigate('/dashboard?create=1') } -} - export default function BottomNav() { const { t } = useTranslation() const navigate = useNavigate() @@ -60,7 +30,7 @@ export default function BottomNav() { // nav pill (Navbar) — otherwise they were only reachable by typing /plugins/:id. const pagePlugins = usePluginStore(s => s.plugins).filter(p => p.type === 'page') const location = useLocation() - const create = useCreateAction() + const create = useCreateAction(false) const isAdmin = useAuthStore(s => s.user?.role === 'admin') const items: NavItem[] = [ @@ -78,8 +48,7 @@ export default function BottomNav() { const left = items.slice(0, splitAt) const right = items.slice(splitAt) - const isActive = (to: string) => - to === '/dashboard' ? location.pathname === '/dashboard' : location.pathname.startsWith(to) + const isActive = (to: string) => isNavItemActive(location.pathname, to) const renderItem = ({ to, label, icon: Icon }: NavItem) => { const active = isActive(to) diff --git a/client/src/components/Layout/useCreateAction.test.tsx b/client/src/components/Layout/useCreateAction.test.tsx new file mode 100644 index 000000000..fa5f9066b --- /dev/null +++ b/client/src/components/Layout/useCreateAction.test.tsx @@ -0,0 +1,112 @@ +// FE-COMP-CREATEACTION-001 to -009: the context-aware "+" both bottom navs share. +import { act, renderHook } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { MemoryRouter } from 'react-router'; + +import { useJourneyStore } from '../../store/journeyStore'; +import { isNavItemActive, useCreateAction } from './useCreateAction'; + +const mockNavigate = vi.fn(); +vi.mock('react-router', async () => { + const actual = await vi.importActual('react-router'); + return { ...actual, useNavigate: () => mockNavigate }; +}); +vi.mock('../../i18n', () => ({ useTranslation: () => ({ t: (k: string) => k }) })); + +function at(path: string, phone: boolean) { + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + return renderHook(() => useCreateAction(phone), { wrapper }).result.current; +} + +beforeEach(() => { + mockNavigate.mockClear(); + sessionStorage.clear(); + useJourneyStore.setState({ mobileGalleryOpen: false }); +}); + +describe('useCreateAction', () => { + it('FE-COMP-CREATEACTION-001: creates a trip by default on both navs', () => { + for (const phone of [false, true]) { + const action = at('/dashboard', phone); + expect(action.label).toBe('dashboard.newTrip'); + act(() => action.run()); + expect(mockNavigate).toHaveBeenLastCalledWith('/dashboard?create=1'); + } + }); + + it('FE-COMP-CREATEACTION-002: inside a trip it follows the remembered tab', () => { + const cases: [string | null, string, string][] = [ + ['finanzplan', 'costs.addExpense', 'expense'], + ['buchungen', 'reservations.addManual', 'reservation'], + ['transports', 'transport.addManual', 'transport'], + ['lists', 'places.addPlace', 'place'], + [null, 'places.addPlace', 'place'], + ]; + for (const [tab, label, create] of cases) { + sessionStorage.clear(); + if (tab) sessionStorage.setItem('trip-tab-7', tab); + const action = at('/trips/7', false); + expect(action.label).toBe(label); + act(() => action.run()); + expect(mockNavigate).toHaveBeenLastCalledWith(`/trips/7?create=${create}`); + } + }); + + it('FE-COMP-CREATEACTION-003: inside a journey it adds an entry', () => { + const action = at('/journey/3', true); + expect(action).toMatchObject({ label: 'journey.detail.addEntry' }); + expect(action.upload).toBeUndefined(); + act(() => action.run()); + expect(mockNavigate).toHaveBeenLastCalledWith('/journey/3?create=entry'); + }); + + it('FE-COMP-CREATEACTION-004: the phone uploads a photo while the journey gallery is open', () => { + useJourneyStore.setState({ mobileGalleryOpen: true }); + const action = at('/journey/3', true); + expect(action).toMatchObject({ label: 'common.upload', upload: true }); + act(() => action.run()); + expect(mockNavigate).toHaveBeenLastCalledWith('/journey/3?create=photo'); + }); + + it('FE-COMP-CREATEACTION-005: the desktop nav ignores the open gallery', () => { + useJourneyStore.setState({ mobileGalleryOpen: true }); + expect(at('/journey/3', false).label).toBe('journey.detail.addEntry'); + }); + + it('FE-COMP-CREATEACTION-006: the journey list starts a journey', () => { + const action = at('/journey', false); + expect(action.label).toBe('journey.new'); + act(() => action.run()); + expect(mockNavigate).toHaveBeenLastCalledWith('/journey?create=1'); + }); + + it('FE-COMP-CREATEACTION-007: the atlas opens the country search on the phone only', () => { + const phone = at('/atlas', true); + expect(phone.label).toBe('atlas.searchCountry'); + act(() => phone.run()); + expect(mockNavigate).toHaveBeenLastCalledWith('/atlas?search=1'); + expect(at('/atlas', false).label).toBe('dashboard.newTrip'); + }); + + it('FE-COMP-CREATEACTION-008: collections add a place to the open list on the phone only', () => { + const all = at('/collections', true); + expect(all.label).toBe('collections.addPlace'); + act(() => all.run()); + expect(mockNavigate).toHaveBeenLastCalledWith('/collections?create=place'); + const one = at('/collections/12', true); + act(() => one.run()); + expect(mockNavigate).toHaveBeenLastCalledWith('/collections/12?create=place'); + expect(at('/collections/12', false).label).toBe('dashboard.newTrip'); + }); +}); + +describe('isNavItemActive', () => { + it('FE-COMP-CREATEACTION-009: the dashboard matches exactly, other items by prefix', () => { + expect(isNavItemActive('/dashboard', '/dashboard')).toBe(true); + expect(isNavItemActive('/dashboard/x', '/dashboard')).toBe(false); + expect(isNavItemActive('/vacay/2026', '/vacay')).toBe(true); + expect(isNavItemActive('/atlas', '/vacay')).toBe(false); + }); +}); diff --git a/client/src/components/Layout/useCreateAction.ts b/client/src/components/Layout/useCreateAction.ts new file mode 100644 index 000000000..6501e3204 --- /dev/null +++ b/client/src/components/Layout/useCreateAction.ts @@ -0,0 +1,75 @@ +import { useMatch, useNavigate } from 'react-router'; + +import { useTranslation } from '../../i18n'; +import { useJourneyStore } from '../../store/journeyStore'; + +interface CreateAction { + label: string; + run: () => void; + upload?: boolean; +} + +/** + * The centre "+" of the bottom navs means something different per context: inside a + * trip it adds a place, on the journey list it starts a journey, inside a journey it + * adds an entry, everywhere else it creates a new trip. Pages pick the intent up from + * the query params. + * + * `phone` adds what only the phone dock offers: inside a journey whose gallery is open + * it uploads a photo, on the atlas it opens the country search, on collections it adds + * a place to the active list. + */ +export function useCreateAction(phone: boolean): CreateAction { + const navigate = useNavigate(); + const { t } = useTranslation(); + const galleryOpen = useJourneyStore((state) => state.mobileGalleryOpen); + const inTrip = useMatch('/trips/:id'); + const inJourney = useMatch('/journey/:id'); + const onJourneyList = useMatch('/journey'); + const onAtlas = useMatch('/atlas'); + const onCollections = useMatch('/collections'); + const inCollection = useMatch('/collections/:id'); + + if (inTrip) { + // The "+" is context-aware per active tab: Bookings → reservation, + // Transports → transport, Costs → expense. Tabs without a create modal + // (lists / files / collab) fall through to adding a place. #1349 + const id = inTrip.params.id; + const tripTab = typeof sessionStorage !== 'undefined' ? sessionStorage.getItem(`trip-tab-${id}`) : null; + if (tripTab === 'finanzplan') + return { label: t('costs.addExpense'), run: () => navigate(`/trips/${id}?create=expense`) }; + if (tripTab === 'buchungen') + return { label: t('reservations.addManual'), run: () => navigate(`/trips/${id}?create=reservation`) }; + if (tripTab === 'transports') + return { label: t('transport.addManual'), run: () => navigate(`/trips/${id}?create=transport`) }; + return { label: t('places.addPlace'), run: () => navigate(`/trips/${id}?create=place`) }; + } + if (inJourney) { + // Context-aware per tab, like the trip's "+": the Gallery holds photos, so + // there the one big action is uploading one. + const journeyId = inJourney.params.id; + if (phone && galleryOpen) { + return { label: t('common.upload'), run: () => navigate(`/journey/${journeyId}?create=photo`), upload: true }; + } + return { label: t('journey.detail.addEntry'), run: () => navigate(`/journey/${journeyId}?create=entry`) }; + } + if (onJourneyList) { + return { label: t('journey.new'), run: () => navigate('/journey?create=1') }; + } + if (phone && onAtlas) { + return { label: t('atlas.searchCountry'), run: () => navigate('/atlas?search=1') }; + } + if (phone && (onCollections || inCollection)) { + // Picking a list moves the route to /collections/:id, so the exact match + // alone dropped the "+" through to creating a trip (#1930). The handoff keeps + // the id, otherwise adding would land on "All saved". + const path = inCollection ? `/collections/${inCollection.params.id}` : '/collections'; + return { label: t('collections.addPlace'), run: () => navigate(`${path}?create=place`) }; + } + return { label: t('dashboard.newTrip'), run: () => navigate('/dashboard?create=1') }; +} + +/** Whether a nav item is the current route: the dashboard only on its exact path, the rest by prefix. */ +export function isNavItemActive(pathname: string, to: string): boolean { + return to === '/dashboard' ? pathname === '/dashboard' : pathname.startsWith(to); +} diff --git a/client/src/mobile/components/MBottomNav.tsx b/client/src/mobile/components/MBottomNav.tsx index ad1bc3830..7a7ecb8de 100644 --- a/client/src/mobile/components/MBottomNav.tsx +++ b/client/src/mobile/components/MBottomNav.tsx @@ -1,75 +1,16 @@ import { useEffect, useState } from 'react' -import { useNavigate, useLocation, useMatch } from 'react-router' +import { useNavigate, useLocation } from 'react-router' import { useSettingsStore } from '../../store/settingsStore' import { useTranslation } from '../../i18n' -import { useJourneyStore } from '../../store/journeyStore' import { ChevronRight, MoreHorizontal, Plus, Search, Upload } from 'lucide-react' import type { LucideIcon } from 'lucide-react' import { normalizeAppearance } from '@trek/shared' import { useNavItems, splitMobileNav } from '../../components/Layout/navItems' +import { isNavItemActive, useCreateAction } from '../../components/Layout/useCreateAction' import MFab from './MFab' interface NavItem { to: string; label: string; icon: LucideIcon } -// The centre "+" means something different per context: inside a trip it adds a -// place, on the journey list it starts a journey (deliberate deviation from the -// demo, which reserves the FAB for entries — the list has no journey to add -// into yet), inside a journey it adds an entry, on the atlas it opens the -// country search, on collections it adds a place to the active list — -// everywhere else it creates a new trip. Pages pick the intent up from the -// query params. The result is unused on /vacay: that screen draws its own centre -// FAB, so the dock yields the slot instead (see screenFabSlot below, #1811). -function useCreateAction(): { label: string; run: () => void; upload?: boolean } { - const navigate = useNavigate() - const { t } = useTranslation() - const galleryOpen = useJourneyStore(state => state.mobileGalleryOpen) - const inTrip = useMatch('/trips/:id') - const inJourney = useMatch('/journey/:id') - const onJourneyList = useMatch('/journey') - const onAtlas = useMatch('/atlas') - const onCollections = useMatch('/collections') - const inCollection = useMatch('/collections/:id') - - if (inTrip) { - // The "+" is context-aware per active tab: Bookings → reservation, - // Transports → transport, Costs → expense. Tabs without a create modal - // (lists / files / collab) fall through to adding a place. #1349 - const id = inTrip.params.id - const tripTab = typeof sessionStorage !== 'undefined' ? sessionStorage.getItem(`trip-tab-${id}`) : null - if (tripTab === 'finanzplan') return { label: t('costs.addExpense'), run: () => navigate(`/trips/${id}?create=expense`) } - if (tripTab === 'buchungen') return { label: t('reservations.addManual'), run: () => navigate(`/trips/${id}?create=reservation`) } - if (tripTab === 'transports') return { label: t('transport.addManual'), run: () => navigate(`/trips/${id}?create=transport`) } - return { label: t('places.addPlace'), run: () => navigate(`/trips/${id}?create=place`) } - } - if (inJourney) { - // Context-aware per tab, like the trip's "+": the Gallery holds photos, so - // there the one big action is uploading one. Read from sessionStorage - // because the tab is the screen's own state and the dock is a sibling — - // exactly how the trip tabs hand theirs over. - const journeyId = inJourney.params.id - if (galleryOpen) { - return { label: t('common.upload'), run: () => navigate(`/journey/${journeyId}?create=photo`), upload: true } - } - return { label: t('journey.detail.addEntry'), run: () => navigate(`/journey/${journeyId}?create=entry`) } - } - if (onJourneyList) { - return { label: t('journey.new'), run: () => navigate('/journey?create=1') } - } - if (onAtlas) { - return { label: t('atlas.searchCountry'), run: () => navigate('/atlas?search=1') } - } - if (onCollections || inCollection) { - // Picking a list moves the route to /collections/:id, so the exact match - // alone dropped the "+" through to creating a trip — the one state the - // screen is normally used in (#1930). The handoff keeps the id, otherwise - // adding would land on "All saved" and the sheet would ask for the list the - // user is already looking at. - const path = inCollection ? `/collections/${inCollection.params.id}` : '/collections' - return { label: t('collections.addPlace'), run: () => navigate(`${path}?create=place`) } - } - return { label: t('dashboard.newTrip'), run: () => navigate('/dashboard?create=1') } -} - /** * Floating glass dock of the mobile shell. Same tab/gating/"+" logic as the * legacy BottomNav (addons, ?create= contract), redesigned as the demo's icon @@ -81,7 +22,9 @@ export default function MBottomNav() { const { t } = useTranslation() const navigate = useNavigate() const location = useLocation() - const create = useCreateAction() + // The "+" result is unused on /vacay: that screen draws its own centre FAB, so + // the dock yields the slot instead (see screenFabSlot below, #1811). + const create = useCreateAction(true) const [moreOpen, setMoreOpen] = useState(false) // Close the popover when the route changes underneath it (browser back etc.). @@ -97,8 +40,7 @@ export default function MBottomNav() { const dockItems: NavItem[] = split.bar const moreItems: NavItem[] = split.more - const isActive = (to: string) => - to === '/dashboard' ? location.pathname === '/dashboard' : location.pathname.startsWith(to) + const isActive = (to: string) => isNavItemActive(location.pathname, to) const moreActive = moreItems.some(item => isActive(item.to)) // The FAB gives way to a decorative logo slot on screens without an add