diff --git a/client/src/components/Tours/packing/TourPackingLinkDialog.tsx b/client/src/components/Tours/packing/TourPackingLinkDialog.tsx index 8f42aaaa5..e44ee95db 100644 --- a/client/src/components/Tours/packing/TourPackingLinkDialog.tsx +++ b/client/src/components/Tours/packing/TourPackingLinkDialog.tsx @@ -14,7 +14,7 @@ import { } from '../../shared/DialogShell'; import { Tooltip } from '../../shared/Tooltip'; import { ChoiceList, ChoiceRow } from '../collection/choiceParts'; -import { CategoryDot, CountBadge } from './packingLinkParts'; +import { CategoryDot, CountBadge, DimmedLink } from './packingLinkParts'; import type { TourPackingLinkController } from './useTourPackingLink'; /** One thing to point at: its mark, its name and how many items it holds. */ @@ -44,16 +44,57 @@ function Choice({ ); } +/** Everything the tour can point at: the whole list first, then each category. */ +function Choices({ packing }: { packing: TourPackingLinkController }) { + const { t } = useTranslation(); + return ( + + + packing.choose(null)} + testId="tour-packing-choice-whole" + mark={ + + + + } + name={t('tours.packing.whole')} + count={packing.wholeCount} + /> + {packing.categories.map((category) => ( + packing.choose(category.name)} + testId={`tour-packing-choice-${category.name}`} + mark={ + + + + } + name={category.name} + count={category.count} + /> + ))} + + + ); +} + /** * P2D-XD-01: the dialog that points a tour at its trip's packing list, as a * whole or at one of its categories, each with its colour and how many items * it holds. It writes only through the button in its foot; letting go of a - * link is offered there too, and neither touches the list itself. + * link is offered there too, and neither touches the list itself. With Packing + * off there is nothing to choose and a link cannot be made again, so it shows + * only the dimmed link and asks before letting go of it. */ export function TourPackingLinkDialog({ packing, tourName }: { packing: TourPackingLinkController; tourName: string }) { const { t } = useTranslation(); const labelId = useId(); if (!packing.open) return null; + const linkOnly = !packing.packingEnabled; return ( {t('common.cancel')} - void packing.confirm()} - icon={ - packing.saving ? ( - - ) : ( - - ) - } - > - {t('tours.packing.confirm')} - + {!linkOnly && ( + void packing.confirm()} + icon={ + packing.saving ? ( + + ) : ( + + ) + } + > + {t('tours.packing.confirm')} + + )} } > - - - packing.choose(null)} - testId="tour-packing-choice-whole" - mark={ - - - - } - name={t('tours.packing.whole')} - count={packing.wholeCount} - /> - {packing.categories.map((category) => ( - packing.choose(category.name)} - testId={`tour-packing-choice-${category.name}`} - mark={ - - - - } - name={category.name} - count={category.count} - /> - ))} - - + {linkOnly && packing.link ? : } ); } diff --git a/client/src/components/Tours/packing/TourPackingSection.tsx b/client/src/components/Tours/packing/TourPackingSection.tsx index 51c5b2142..841c7be16 100644 --- a/client/src/components/Tours/packing/TourPackingSection.tsx +++ b/client/src/components/Tours/packing/TourPackingSection.tsx @@ -1,14 +1,13 @@ -import type { TourListItem, TourPackingLink } from '@trek/shared'; -import { AlertTriangle, ArrowUpRight, Backpack, Check, Link2, PackageOpen, PowerOff, Unlink } from 'lucide-react'; +import type { TourListItem } from '@trek/shared'; +import { AlertTriangle, ArrowUpRight, Backpack, Check, Link2, PackageOpen, Unlink } from 'lucide-react'; import type { ReactNode } from 'react'; import { useTranslation } from '../../../i18n'; import type { PackingItem } from '../../../types'; import { BOX } from '../../Planner/bookings/bookingParts'; import { SoftPill, WhiteBadge } from '../../Planner/planParts'; import { DialogSection, fs } from '../../shared/DialogShell'; -import { Tooltip } from '../../shared/Tooltip'; import { CardAction, TourNotice } from '../tourParts'; -import { CategoryDot } from './packingLinkParts'; +import { CategoryDot, DimmedLink, linkName } from './packingLinkParts'; import { TourPackingLinkDialog } from './TourPackingLinkDialog'; import { useTourPackingLink, type TourPackingLinkController } from './useTourPackingLink'; @@ -71,11 +70,6 @@ function ItemRow({ item }: { item: PackingItem }) { ); } -/** The name of what the tour points at: a category by its own name, otherwise the whole list. */ -function linkName(link: TourPackingLink, t: (key: string) => string): string { - return link.category ?? t('tours.packing.whole'); -} - /** The linked list as it reads now: its items with how many are packed, or why there are none to show. */ function LinkedCard({ packing, onOpenPacking }: { packing: TourPackingLinkController; onOpenPacking?: () => void }) { const { t } = useTranslation(); @@ -192,17 +186,7 @@ function SectionBody({ ); } if (!packing.link) return ; - if (!packing.packingEnabled) { - // Dimmed, not hidden: the tour still points at the list, it just cannot be shown. - return ( -
- }> - {linkName(packing.link, t)} - {t('tours.packing.addonOff')} - -
- ); - } + if (!packing.packingEnabled) return ; return ; } @@ -239,14 +223,11 @@ export function TourPackingSection({ ); } else if (editable && packing.link) { + // With Packing off a link cannot be made again, so letting go asks first, in the tour's packing dialog. action = ( - - - void packing.unlink()} icon={}> - {t('tours.packing.unlink')} - - - + }> + {t('tours.packing.unlink')} + ); } return ( diff --git a/client/src/components/Tours/packing/packingLinkParts.tsx b/client/src/components/Tours/packing/packingLinkParts.tsx index 9ad51e9cb..c040df278 100644 --- a/client/src/components/Tours/packing/packingLinkParts.tsx +++ b/client/src/components/Tours/packing/packingLinkParts.tsx @@ -1,6 +1,26 @@ -import { Package } from 'lucide-react'; +import type { TourPackingLink } from '@trek/shared'; +import { Package, PowerOff } from 'lucide-react'; import { useTranslation } from '../../../i18n'; import { tintOf, WhiteBadge } from '../../Planner/planParts'; +import { TourNotice } from '../tourParts'; + +/** The name of what the tour points at: a category by its own name, otherwise the whole list. */ +export function linkName(link: TourPackingLink, t: (key: string) => string): string { + return link.category ?? t('tours.packing.whole'); +} + +/** A link while Packing is off: dimmed, not hidden, since the tour still points at the list it cannot show. */ +export function DimmedLink({ link }: { link: TourPackingLink }) { + const { t } = useTranslation(); + return ( +
+ }> + {linkName(link, t)} + {t('tours.packing.addonOff')} + +
+ ); +} /** A packing category's own colour, as the packing list gives it, with a soft halo of the same colour. */ export function CategoryDot({ color }: { color: string }) { diff --git a/client/src/components/Tours/packing/tourPacking.test.tsx b/client/src/components/Tours/packing/tourPacking.test.tsx index afc5dcb17..d5cf95455 100644 --- a/client/src/components/Tours/packing/tourPacking.test.tsx +++ b/client/src/components/Tours/packing/tourPacking.test.tsx @@ -12,7 +12,7 @@ import type { PackingItem } from '../../../types'; import { TourPackingSection } from './TourPackingSection'; import { useTourPackingLink } from './useTourPackingLink'; -// FE-TOUR-PACKING-001 to FE-TOUR-PACKING-010 +// FE-TOUR-PACKING-001 to FE-TOUR-PACKING-011 const toast = vi.hoisted(() => ({ success: vi.fn(), error: vi.fn() })); vi.mock('../../shared/Toast', () => ({ useToast: () => toast })); @@ -201,10 +201,29 @@ describe('TourPackingSection', () => { section(); expect(await screen.findByText('Packing lists are switched off, so the list cannot be shown.')).toBeInTheDocument(); fireEvent.click(screen.getByText('Remove link')); + // One click only asks: the dialog shows the dimmed link, nothing to choose and no way to link again. + expect(unlink).not.toHaveBeenCalled(); + const dialog = await screen.findByRole('dialog'); + expect(within(dialog).getByTestId('tour-packing-dimmed')).toBeInTheDocument(); + expect(within(dialog).queryByTestId('tour-packing-choice-whole')).not.toBeInTheDocument(); + expect(within(dialog).queryByTestId('tour-packing-confirm')).not.toBeInTheDocument(); + fireEvent.click(within(dialog).getByTestId('tour-packing-unlink')); await waitFor(() => expect(unlink).toHaveBeenCalledWith(5, 11)); await waitFor(() => expect(screen.queryByTestId('tour-packing-section')).not.toBeInTheDocument()); }); + it('FE-TOUR-PACKING-011: with Packing off, cancelling the question keeps the link', async () => { + setPacking(false); + vi.mocked(tourPackingLinkRepo.read).mockResolvedValue(GEAR); + const unlink = vi.spyOn(tourPackingLinkRepo, 'unlink').mockResolvedValue(null); + section(); + fireEvent.click(await screen.findByText('Remove link')); + fireEvent.click(within(await screen.findByRole('dialog')).getByText('Cancel')); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + expect(unlink).not.toHaveBeenCalled(); + expect(screen.getByTestId('tour-packing-dimmed')).toBeInTheDocument(); + }); + it('FE-TOUR-PACKING-010: a failed read is said, with a retry', async () => { vi.mocked(tourPackingLinkRepo.read).mockRejectedValueOnce(new Error('500')); section(); diff --git a/client/src/hooks/useListsSubTab.test.ts b/client/src/hooks/useListsSubTab.test.ts index b46917549..54dbc9505 100644 --- a/client/src/hooks/useListsSubTab.test.ts +++ b/client/src/hooks/useListsSubTab.test.ts @@ -1,7 +1,7 @@ import { act, renderHook } from '@testing-library/react'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; -import { useListsSubTab } from './useListsSubTab'; +import { rememberListsSubTab, useListsSubTab } from './useListsSubTab'; // FE-HOOK-LISTSTAB-001 to FE-HOOK-LISTSTAB-006 @@ -47,3 +47,40 @@ describe('useListsSubTab', () => { expect(renderHook(() => useListsSubTab(3, { onlyKnownTabs: true })).result.current[0]).toBe('todo'); }); }); + +// FE-HOOK-LISTSTAB-007 to FE-HOOK-LISTSTAB-009: picking the sub-tab from outside the Lists tab. +describe('rememberListsSubTab', () => { + it('FE-HOOK-LISTSTAB-007: the Lists tab opens on the sub-tab picked before it was on screen', () => { + sessionStorage.setItem('trip-lists-subtab-7', 'todo'); + rememberListsSubTab(7, 'packing'); + expect(renderHook(() => useListsSubTab(7)).result.current[0]).toBe('packing'); + }); + + it('FE-HOOK-LISTSTAB-008: a Lists tab already on screen follows, one for another trip does not', () => { + sessionStorage.setItem('trip-lists-subtab-7', 'todo'); + sessionStorage.setItem('trip-lists-subtab-8', 'todo'); + const same = renderHook(() => useListsSubTab(7)); + const other = renderHook(() => useListsSubTab(8)); + act(() => rememberListsSubTab(7, 'packing')); + expect(same.result.current[0]).toBe('packing'); + expect(other.result.current[0]).toBe('todo'); + same.unmount(); + act(() => rememberListsSubTab(7, 'todo')); + expect(sessionStorage.getItem('trip-lists-subtab-7')).toBe('todo'); + }); + + it('FE-HOOK-LISTSTAB-009: blocked storage still switches the tab on screen and does not throw', () => { + const { result } = renderHook(() => useListsSubTab(7)); + const setItem = vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { + throw new Error('blocked'); + }); + const getItem = vi.spyOn(Storage.prototype, 'getItem').mockImplementation(() => { + throw new Error('blocked'); + }); + act(() => rememberListsSubTab(7, 'todo')); + expect(result.current[0]).toBe('todo'); + expect(renderHook(() => useListsSubTab(9)).result.current[0]).toBe('packing'); + setItem.mockRestore(); + getItem.mockRestore(); + }); +}); diff --git a/client/src/hooks/useListsSubTab.ts b/client/src/hooks/useListsSubTab.ts index f75e7efa4..481b72b91 100644 --- a/client/src/hooks/useListsSubTab.ts +++ b/client/src/hooks/useListsSubTab.ts @@ -1,10 +1,39 @@ -import { useState } from 'react'; +import { useEffect, useState } from 'react'; /** The two lists the Lists tab switches between. */ export type ListsSubTab = 'packing' | 'todo'; const storageKey = (tripId: number) => `trip-lists-subtab-${tripId}`; +/** Lists tabs that are on screen, told when another surface picks their sub-tab. */ +const followers = new Set<(tripId: number, tab: ListsSubTab) => void>(); + +function readStored(tripId: number): string | null { + try { + return sessionStorage.getItem(storageKey(tripId)); + } catch { + return null; + } +} + +function store(tripId: number, tab: ListsSubTab) { + try { + sessionStorage.setItem(storageKey(tripId), tab); + } catch { + // Storage can be blocked; the choice then lasts as long as the tab is on screen. + } +} + +/** + * Picks the Lists tab's sub-tab from outside it, e.g. "Open packing list" on a + * tour. The Lists tab opens with it next time, and one already on screen + * switches to it right away. + */ +export function rememberListsSubTab(tripId: number, tab: ListsSubTab) { + store(tripId, tab); + followers.forEach((follow) => follow(tripId, tab)); +} + /** * The Lists tab's sub-tab, remembered per trip for the browser session. Both the * desktop container and the phone shell read and write the same key. The phone @@ -13,13 +42,22 @@ const storageKey = (tripId: number) => `trip-lists-subtab-${tripId}`; */ export function useListsSubTab(tripId: number, { onlyKnownTabs = false }: { onlyKnownTabs?: boolean } = {}) { const [subTab, setSubTabState] = useState(() => { - const saved = sessionStorage.getItem(storageKey(tripId)); + const saved = readStored(tripId); if (onlyKnownTabs) return saved === 'todo' ? 'todo' : 'packing'; return (saved as ListsSubTab) || 'packing'; }); + useEffect(() => { + const follow = (forTrip: number, tab: ListsSubTab) => { + if (forTrip === tripId) setSubTabState(tab); + }; + followers.add(follow); + return () => { + followers.delete(follow); + }; + }, [tripId]); const setSubTab = (tab: ListsSubTab) => { setSubTabState(tab); - sessionStorage.setItem(storageKey(tripId), tab); + store(tripId, tab); }; return [subTab, setSubTab] as const; } diff --git a/client/src/pages/tripPlanner/useTourDetailActions.test.ts b/client/src/pages/tripPlanner/useTourDetailActions.test.ts index c2cd05044..c409ba9be 100644 --- a/client/src/pages/tripPlanner/useTourDetailActions.test.ts +++ b/client/src/pages/tripPlanner/useTourDetailActions.test.ts @@ -1,17 +1,33 @@ // P2D-XD-01: from a tour's packing link to the trip's Lists tab, the tour's detail closed on the way. -import { renderHook } from '@testing-library/react'; +import { act, renderHook } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; +import { useListsSubTab } from '../../hooks/useListsSubTab'; import { useTourDetailActions } from './useTourDetailActions'; import type { useTripPlanner } from './useTripPlanner'; describe('useTourDetailActions', () => { + const planner = () => ({ tripId: 5, setSelectedPlaceId: vi.fn(), handleTabChange: vi.fn() }); + const actions = (p: ReturnType) => + renderHook(() => useTourDetailActions(p as unknown as ReturnType)).result.current; + it('FE-TOUR-DETAIL-ACTIONS-001: opening the packing list closes the tour and switches to the Lists tab', () => { - const planner = { setSelectedPlaceId: vi.fn(), handleTabChange: vi.fn() }; - const { result } = renderHook(() => - useTourDetailActions(planner as unknown as ReturnType) - ); - result.current.onOpenPacking(); - expect(planner.setSelectedPlaceId).toHaveBeenCalledWith(null); - expect(planner.handleTabChange).toHaveBeenCalledWith('listen'); + const p = planner(); + actions(p).onOpenPacking(); + expect(p.setSelectedPlaceId).toHaveBeenCalledWith(null); + expect(p.handleTabChange).toHaveBeenCalledWith('listen'); + }); + + it('FE-TOUR-DETAIL-ACTIONS-002: the Lists tab opens on packing even when the to-dos were last open', () => { + sessionStorage.setItem('trip-lists-subtab-5', 'todo'); + actions(planner()).onOpenPacking(); + expect(renderHook(() => useListsSubTab(5)).result.current[0]).toBe('packing'); + }); + + it('FE-TOUR-DETAIL-ACTIONS-003: a Lists tab already on screen on the to-dos switches to packing', () => { + sessionStorage.setItem('trip-lists-subtab-5', 'todo'); + const lists = renderHook(() => useListsSubTab(5)); + expect(lists.result.current[0]).toBe('todo'); + act(() => actions(planner()).onOpenPacking()); + expect(lists.result.current[0]).toBe('packing'); }); }); diff --git a/client/src/pages/tripPlanner/useTourDetailActions.ts b/client/src/pages/tripPlanner/useTourDetailActions.ts index 62d0a21d6..6b87d76e9 100644 --- a/client/src/pages/tripPlanner/useTourDetailActions.ts +++ b/client/src/pages/tripPlanner/useTourDetailActions.ts @@ -1,4 +1,5 @@ import type { TourListItem } from '@trek/shared'; +import { rememberListsSubTab } from '../../hooks/useListsSubTab'; import { tourRepo } from '../../repo/tourRepo'; import { useTripStore } from '../../store/tripStore'; import type { Place } from '../../types'; @@ -36,9 +37,10 @@ export function useTourDetailActions(planner: ReturnType) onToursChanged: (placeIds: number[]) => invalidateTourPlaceIds({ placeIds }), /** LIB-02: a tour taken over from another trip; the tours rail opens it in the planner. */ onTakenOver: bringIn, - /** P2D-XD-01: the packing list a tour points at, in the trip's Lists tab. */ + /** P2D-XD-01: the packing list a tour points at, in the trip's Lists tab, on its packing side. */ onOpenPacking: () => { planner.setSelectedPlaceId(null); + rememberListsSubTab(tripId, 'packing'); planner.handleTabChange('listen'); }, onSaveAsNew: async (tour: TourListItem) => {