From e3bf18f9a6b1f850c28d8f4669a762a9a96bddfc Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 12 Oct 2026 10:44:57 +0200 Subject: [PATCH] fix(collections): a list invite can be accepted where its notification leads, on phone and desktop The invite notification links to /collections/:id, a list the invitee cannot read until they accept. The phone screen had no invite UI at all and showed "Start your first collection", and on desktop the rail with the invites is hidden below 1080px while the empty state had no way to open it. useCollections now recognises a route id that is a pending invite, keeps it out of the route sync and puts it first in the invites it hands out. The phone screen shows the pending invites as cards above the lists, and the desktop body shows them when the person has no lists of their own, or the one invite the route points at. Both reuse the existing invite strings. A route id that is neither a list nor an invite (declined, withdrawn, deleted) now drops back to /collections once the lists are in, instead of leaving the page on a list it cannot show. Refs #2645 --- client/scripts/feature-pairs.json | 1 + .../Collections/CollectionInviteCard.tsx | 39 +++++++++++++++ .../src/components/Collections/ListsRail.tsx | 14 +----- .../screens/collections/MCollInvites.tsx | 42 ++++++++++++++++ .../screens/collections/MCollections.tsx | 3 ++ client/src/pages/CollectionsPage.test.tsx | 46 +++++++++++++++++ client/src/pages/CollectionsPage.tsx | 16 ++++++ .../src/pages/CollectionsPage.wiring.test.tsx | 2 + .../pages/collections/useCollections.test.tsx | 50 +++++++++++++++++++ .../src/pages/collections/useCollections.ts | 43 +++++++++++++--- client/src/store/collectionStore.ts | 4 +- client/src/styles/collections.css | 3 ++ .../mobile/collections/MCollections.test.tsx | 36 ++++++++++++- 13 files changed, 277 insertions(+), 22 deletions(-) create mode 100644 client/src/components/Collections/CollectionInviteCard.tsx create mode 100644 client/src/mobile/screens/collections/MCollInvites.tsx diff --git a/client/scripts/feature-pairs.json b/client/scripts/feature-pairs.json index 9e74bb34c..d0e4480b9 100644 --- a/client/scripts/feature-pairs.json +++ b/client/scripts/feature-pairs.json @@ -862,6 +862,7 @@ "src/mobile/screens/atlas/MAtlasSearch.tsx": "Phone full screen country search overlay; options, suggestions and selection come through props from MAtlas.", "src/mobile/screens/atlas/MAtlasStatsCard.tsx": "Markup only; renders the AtlasStats that MAtlas passes in.", "src/mobile/screens/collections/MCollCategoryPicker.tsx": "Phone only category dropdown; value and categories come through props from the collection sheets.", + "src/mobile/screens/collections/MCollInvites.tsx": "Markup only; the invites and accept and decline come through props from useCollections via MCollections.", "src/mobile/screens/collections/MCollLabelsSheet.tsx": "Phone label sheet; create, update, delete and assign come through props from useCollections.", "src/mobile/screens/collections/MCollLinksEditor.tsx": "Markup only; link rows and the change handler come through props from the collection sheets.", "src/mobile/screens/collections/MCollListPickerSheet.tsx": "Phone list picker sheet; the move or copy action comes through onPick from useCollections.", diff --git a/client/src/components/Collections/CollectionInviteCard.tsx b/client/src/components/Collections/CollectionInviteCard.tsx new file mode 100644 index 000000000..84eeee74c --- /dev/null +++ b/client/src/components/Collections/CollectionInviteCard.tsx @@ -0,0 +1,39 @@ +import React from 'react'; +import type { IncomingCollectionInvite } from '../../store/collectionStore'; +import type { TranslationFn } from '../../types'; + +interface CollectionInviteCardProps { + invite: IncomingCollectionInvite; + onAccept: (id: number) => void; + onDecline: (id: number) => void; + t: TranslationFn; +} + +/** + * One pending list invite with Accept / Decline. Lives in the lists rail, and in + * the page body where the rail is not on screen: no lists of one's own yet, or + * the invite notification's link to a list that is still an invite (#2645). + */ +export default function CollectionInviteCard({ + invite, + onAccept, + onDecline, + t, +}: CollectionInviteCardProps): React.ReactElement { + return ( +
+
{invite.name}
+
+ {t('collections.invites.from')} {invite.from.username} +
+
+ + +
+
+ ); +} diff --git a/client/src/components/Collections/ListsRail.tsx b/client/src/components/Collections/ListsRail.tsx index 286b6017c..26f44f55a 100644 --- a/client/src/components/Collections/ListsRail.tsx +++ b/client/src/components/Collections/ListsRail.tsx @@ -4,6 +4,7 @@ import type { Collection } from '@trek/shared' import type { TranslationFn } from '../../types' import { ALL_SAVED } from '../../store/collectionStore' import type { ActiveCollectionId, IncomingCollectionInvite } from '../../store/collectionStore' +import CollectionInviteCard from './CollectionInviteCard' interface ListsRailProps { ownedLists: Collection[] @@ -90,18 +91,7 @@ export default function ListsRail(props: ListsRailProps): React.ReactElement { {incomingInvites.length} {incomingInvites.map(inv => ( -
-
{inv.name}
-
{t('collections.invites.from')} {inv.from.username}
-
- - -
-
+ ))} )} diff --git a/client/src/mobile/screens/collections/MCollInvites.tsx b/client/src/mobile/screens/collections/MCollInvites.tsx new file mode 100644 index 000000000..232791cd6 --- /dev/null +++ b/client/src/mobile/screens/collections/MCollInvites.tsx @@ -0,0 +1,42 @@ +import { Check } from 'lucide-react'; +import type { IncomingCollectionInvite } from '../../../store/collectionStore'; +import type { TranslationFn } from '../../../types'; +import { CancelPill, Eyebrow, PrimaryPill } from './MCollSheetKit'; + +interface MCollInvitesProps { + invites: IncomingCollectionInvite[]; + onAccept: (id: number) => void; + onDecline: (id: number) => void; + t: TranslationFn; +} + +/** + * Pending list invites as cards above the lists, with Accept / Decline. The + * phone has no rail, so this is the one place an invite can be answered, and + * where the invite notification lands (#2645). + */ +export default function MCollInvites({ invites, onAccept, onDecline, t }: MCollInvitesProps) { + if (invites.length === 0) return null; + return ( +
+ {t('collections.invites.title').toUpperCase()} + {invites.map((inv) => ( +
+
{inv.name}
+
+ {t('collections.invites.from')} {inv.from.username} +
+
+ onAccept(inv.collection_id)}> + {t('collections.invites.accept')} + + onDecline(inv.collection_id)}>{t('collections.invites.decline')} +
+
+ ))} +
+ ); +} diff --git a/client/src/mobile/screens/collections/MCollections.tsx b/client/src/mobile/screens/collections/MCollections.tsx index 0d1c7c3ef..bb3d81ada 100644 --- a/client/src/mobile/screens/collections/MCollections.tsx +++ b/client/src/mobile/screens/collections/MCollections.tsx @@ -21,6 +21,7 @@ import MCollEditSheet from './MCollEditSheet' import MCollLabelsSheet from './MCollLabelsSheet' import MCollTripPickerSheet from './MCollTripPickerSheet' import MCollListPickerSheet from './MCollListPickerSheet' +import MCollInvites from './MCollInvites' import { STATUS_SPEC } from './collectionsMobileModel' import { CancelPill, PrimaryPill } from './MCollSheetKit' @@ -209,6 +210,8 @@ export default function MCollections() { )} + + {c.loading && c.collections.length === 0 ? (
diff --git a/client/src/pages/CollectionsPage.test.tsx b/client/src/pages/CollectionsPage.test.tsx index 09509527b..43d566680 100644 --- a/client/src/pages/CollectionsPage.test.tsx +++ b/client/src/pages/CollectionsPage.test.tsx @@ -78,6 +78,8 @@ function makeHook(overrides: Record = {}) { hasMappable: true, members: [], incomingInvites: [], + orderedInvites: [], + routeInvite: null, counts: { all: 1, idea: 0, want: 1, visited: 0 }, view: 'list', statusFilter: 'all', @@ -167,3 +169,47 @@ describe('CollectionsPage: a marker click shows the place (#2431)', () => { expect(setSelectedPlaceId).toHaveBeenLastCalledWith(null) }) }) + +// #2645: the rail is hidden below 1080px and an invitee with no lists of their +// own saw only "Start your first collection", so the invites the person has to +// answer also sit at the top of the body. +describe('CollectionsPage: invites in the body (#2645)', () => { + const invite = { collection_id: 14, name: 'Lisbon', from: { id: 4, username: 'Lena' } } + const other = { collection_id: 3, name: 'Rome', from: { id: 5, username: 'Tom' } } + const noLists = { + collections: [], ownedLists: [], activeCollection: null, activeId: null, + places: [], visiblePlaces: [], mappable: [], hasMappable: false, canShare: false, + } + + it('with no lists of their own, every invite can be answered from the body', () => { + const handleAcceptInvite = vi.fn() + const handleDeclineInvite = vi.fn() + mockUseCollections.mockReturnValue(makeHook({ + ...noLists, incomingInvites: [invite, other], orderedInvites: [invite, other], + handleAcceptInvite, handleDeclineInvite, + })) + render() + expect(screen.getByText('Lisbon')).toBeInTheDocument() + expect(screen.getByText('Rome')).toBeInTheDocument() + expect(screen.getByText('collections.empty.firstTitle')).toBeInTheDocument() + screen.getAllByRole('button', { name: 'collections.invites.accept' })[0].click() + expect(handleAcceptInvite).toHaveBeenCalledWith(14) + screen.getAllByRole('button', { name: 'collections.invites.decline' })[1].click() + expect(handleDeclineInvite).toHaveBeenCalledWith(3) + }) + + it('with lists, only the invite the route points at is shown in the body', () => { + mockUseCollections.mockReturnValue(makeHook({ + incomingInvites: [other, invite], orderedInvites: [invite, other], routeInvite: invite, + })) + render() + expect(screen.getByText('Lisbon')).toBeInTheDocument() + expect(screen.queryByText('Rome')).not.toBeInTheDocument() + }) + + it('with lists and no invite on the route, the body leaves invites to the rail', () => { + mockUseCollections.mockReturnValue(makeHook({ incomingInvites: [invite], orderedInvites: [invite] })) + render() + expect(screen.queryByRole('button', { name: 'collections.invites.accept' })).not.toBeInTheDocument() + }) +}) diff --git a/client/src/pages/CollectionsPage.tsx b/client/src/pages/CollectionsPage.tsx index 2aab1b5fe..e1ae016f3 100644 --- a/client/src/pages/CollectionsPage.tsx +++ b/client/src/pages/CollectionsPage.tsx @@ -3,6 +3,7 @@ import { List as ListIcon, Map as MapIcon, Search, Bookmark, CheckCheck, X, Tras import Navbar from '../components/Layout/Navbar' import ConfirmDialog from '../components/shared/ConfirmDialog' import ListsRail from '../components/Collections/ListsRail' +import CollectionInviteCard from '../components/Collections/CollectionInviteCard' import ListEditorModal from '../components/Collections/ListEditorModal' import CollectionHero from '../components/Collections/CollectionHero' import CollectionList from '../components/Collections/CollectionList' @@ -50,6 +51,10 @@ function CollectionsPageDesktop(): React.ReactElement { // Labels are per-collection, so only on a real (non "All saved") list. const isRealList = !c.isAllSaved && typeof c.activeId === 'number' const canManageLabels = isRealList && c.canEdit + // Invites also sit at the top of the body: with no lists of one's own the + // body is all there is (and the rail is hidden below 1080px), and the invite + // notification links to a list that is still an invite (#2645). + const bodyInvites = noLists ? c.orderedInvites : (c.routeInvite ? [c.routeInvite] : []) // Selecting a place from the list toggles it, so clicking the row again clears // it, as does the map background. A marker click always shows the place: a second @@ -206,6 +211,17 @@ function CollectionsPageDesktop(): React.ReactElement {
+ {bodyInvites.length > 0 && ( +
+
+ {t('collections.invites.title')} + {bodyInvites.length} +
+ {bodyInvites.map(inv => ( + + ))} +
+ )} {noLists ? (
diff --git a/client/src/pages/CollectionsPage.wiring.test.tsx b/client/src/pages/CollectionsPage.wiring.test.tsx index 80a7a6579..3bd463e16 100644 --- a/client/src/pages/CollectionsPage.wiring.test.tsx +++ b/client/src/pages/CollectionsPage.wiring.test.tsx @@ -211,6 +211,8 @@ function makeHook(overrides: Hook = {}): Hook { hasMappable: true, members: [], incomingInvites: [], + orderedInvites: [], + routeInvite: null, counts: { all: 1, idea: 0, want: 1, visited: 0 }, view: 'list', statusFilter: 'all', diff --git a/client/src/pages/collections/useCollections.test.tsx b/client/src/pages/collections/useCollections.test.tsx index 65087a00e..599a64742 100644 --- a/client/src/pages/collections/useCollections.test.tsx +++ b/client/src/pages/collections/useCollections.test.tsx @@ -1207,3 +1207,53 @@ describe('useCollections: GPX (#2301)', () => { expect(answer).toEqual(read) }) }) + +// #2645: the invite notification links to /collections/:id, a list the invitee +// cannot read until they accept. The page has to offer the invite there, and a +// link to a list that is gone (declined, cancelled, deleted) must not leave the +// page pointing at nothing. +describe('useCollections — route that is an invite (#2645)', () => { + const invite = { collection_id: 14, name: 'Lisbon', from: { id: 4, username: 'Lena' } } + + it('FE-PAGE-COLL-086: a route id that is a pending invite exposes it and does not open the list', async () => { + routeParams = { id: '14' } + store.incomingInvites = [invite] + const { result } = await setup() + expect(result.current.routeInvite).toEqual(invite) + expect(store.setActive).not.toHaveBeenCalledWith(14) + expect(navigate).not.toHaveBeenCalled() + }) + + it('FE-PAGE-COLL-087: the invite on the route is listed first', async () => { + routeParams = { id: '14' } + const other = { collection_id: 3, name: 'Rome', from: { id: 5, username: 'Tom' } } + store.incomingInvites = [other, invite] + const { result } = await setup() + expect(result.current.orderedInvites).toEqual([invite, other]) + }) + + it('FE-PAGE-COLL-088: a route id that is neither a list nor an invite bounces to /collections once the lists are in', async () => { + routeParams = { id: '14' } + store.collections = [collection({ id: 2 })] + await setup() + await waitFor(() => expect(navigate).toHaveBeenCalledWith('/collections', { replace: true })) + }) + + it('FE-PAGE-COLL-089: no bounce while the lists are still loading', async () => { + routeParams = { id: '14' } + let finish: () => void = () => {} + store.loadAll.mockImplementationOnce(() => new Promise(res => { finish = res })) + await setup() + expect(navigate).not.toHaveBeenCalled() + await act(async () => { finish() }) + await waitFor(() => expect(navigate).toHaveBeenCalledWith('/collections', { replace: true })) + }) + + it('FE-PAGE-COLL-090: a route id that is one of the lists stays put', async () => { + routeParams = { id: '14' } + store.collections = [collection({ id: 14 })] + await setup() + await act(async () => {}) + expect(navigate).not.toHaveBeenCalled() + }) +}) diff --git a/client/src/pages/collections/useCollections.ts b/client/src/pages/collections/useCollections.ts index 1857e0cd5..137b1a4b6 100644 --- a/client/src/pages/collections/useCollections.ts +++ b/client/src/pages/collections/useCollections.ts @@ -79,8 +79,11 @@ export function useCollections() { // place from the detail panel, or the select-mode set for a bulk copy. const [copyIds, setCopyIds] = useState(null) - // Initial load. - useEffect(() => { loadAll() }, []) + // Initial load. listsLoaded says the rail is fresh from this visit, which + // the route check below waits for before it calls a list id unknown. A load + // that fails leaves it off, so an offline page never drops the route. + const [listsLoaded, setListsLoaded] = useState(false) + useEffect(() => { loadAll().then(() => setListsLoaded(true), () => setListsLoaded(false)) }, []) // Central admin-defined categories, for assigning a category to a saved place. const [categories, setCategories] = useState([]) @@ -104,15 +107,41 @@ export function useCollections() { return () => cancelAnimationFrame(raf) }, [view, isWide]) + // The invite notification links to /collections/:id, a list the invitee + // cannot read before they accept (#2645). When the route is such an invite + // the page offers it instead of opening the list, and lists it first. + const routeInvite = useMemo(() => { + if (!routeId) return null + const id = Number(routeId) + return incomingInvites.find(inv => inv.collection_id === id) ?? null + }, [routeId, incomingInvites]) + const orderedInvites = useMemo( + () => (routeInvite ? [routeInvite, ...incomingInvites.filter(inv => inv !== routeInvite)] : incomingInvites), + [routeInvite, incomingInvites], + ) + // Keep the active list in sync with the URL (/collections/:id, or the - // "All saved" union at /collections). + // "All saved" union at /collections). A pending invite on the route shows + // the union behind its card. useEffect(() => { - const next: ActiveCollectionId = routeId ? Number(routeId) : ALL_SAVED + const next: ActiveCollectionId = routeId && !routeInvite ? Number(routeId) : ALL_SAVED if (Number.isNaN(next as number)) return if (next !== activeId) setActive(next) - // Only re-run when the route or the loaded list set changes. + // Only re-run when the route, the loaded list set or the invite on it changes. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [routeId, collections.length]) + }, [routeId, collections.length, routeInvite]) + + // A route id that is neither one of the lists nor a pending invite is a link + // to something gone: a declined or withdrawn invite, a deleted list, one the + // person was removed from. Drop it once the lists are in, rather than leave + // the URL on a list the page cannot show. + useEffect(() => { + if (!listsLoaded || loading || !routeId) return + const id = Number(routeId) + if (Number.isNaN(id)) return + if (collections.some(l => l.id === id) || incomingInvites.some(inv => inv.collection_id === id)) return + navigate('/collections', { replace: true }) + }, [listsLoaded, loading, routeId, collections, incomingInvites, navigate]) // Live sync via WebSocket. collections:deleted / :removed must bounce a member // off a list that was removed (or that they were kicked from) under them. @@ -490,7 +519,7 @@ export function useCollections() { collections, ownedLists, sharedLists, activeCollection, isAllSaved, isOwner, myRole, canEdit, canDelete, canShare, shareMemberCount, - activeId, places, visiblePlaces, mappable, hasMappable, members, incomingInvites, counts, + activeId, places, visiblePlaces, mappable, hasMappable, members, incomingInvites, routeInvite, orderedInvites, counts, view, statusFilter, categoryFilter, categoryOptions, ratingFilter, sortMode, search, selectedPlaceId, selectMode, selectedIds, labels, labelFilter, labelOptions, loading, placesLoading, diff --git a/client/src/store/collectionStore.ts b/client/src/store/collectionStore.ts index 265c558fa..3455be9df 100644 --- a/client/src/store/collectionStore.ts +++ b/client/src/store/collectionStore.ts @@ -180,8 +180,8 @@ export const useCollectionStore = create((set, get) => ({ } catch { // The list may have been left / removed / deleted out from under us (a WS // event or the URL sync can re-request an id we just lost access to). Clear - // it instead of leaving an uncaught 403/404 rejection; the route sync then - // bounces to /collections. + // it instead of leaving an uncaught 403/404 rejection; useCollections drops + // a route id that is no longer one of the lists. if (get().activeId === id) set({ activeId: null, places: [], members: [], labels: [] }) } finally { set({ placesLoading: false }) diff --git a/client/src/styles/collections.css b/client/src/styles/collections.css index 762bfd951..22bb9dc89 100644 --- a/client/src/styles/collections.css +++ b/client/src/styles/collections.css @@ -92,6 +92,9 @@ .trek-dash .col-invite-accept:hover { opacity: .9; } .trek-dash .col-invite-decline { border: 1px solid var(--line-2); color: var(--ink-2); } .trek-dash .col-invite-decline:hover { background: var(--surface); } +/* the same cards at the top of the page body (no lists yet, or the invite's own link) */ +.trek-dash .col-invites-main { max-width: 420px; margin: 0 auto 22px; } +.trek-dash .col-invites-main .col-invite { padding: 13px; } /* ----------------- hero ----------------- */ .trek-dash .col-hero { diff --git a/client/tests/unit/mobile/collections/MCollections.test.tsx b/client/tests/unit/mobile/collections/MCollections.test.tsx index 1f90cd9ac..b44bec202 100644 --- a/client/tests/unit/mobile/collections/MCollections.test.tsx +++ b/client/tests/unit/mobile/collections/MCollections.test.tsx @@ -1,4 +1,4 @@ -// FE-MOB-COLSCR-001 to FE-MOB-COLSCR-031 +// FE-MOB-COLSCR-001 to FE-MOB-COLSCR-033 import React from 'react' import { describe, it, expect, vi, beforeEach } from 'vitest' import { render, screen, fireEvent, waitFor } from '../../../helpers/render' @@ -159,6 +159,8 @@ function makeHook(over: Record = {}): Record { mappable: [place(10, 'Elbphilharmonie', { lat: 53.5, lng: 10 })], members: [], incomingInvites: [], + orderedInvites: [], + routeInvite: null, counts: { all: 1, idea: 1, want: 0, visited: 0 }, view: 'list', statusFilter: 'all', @@ -186,6 +188,7 @@ function makeHook(over: Record = {}): Record { copyIds: null, openCopyForSelectedPlace: vi.fn(), openCopyForSelection: vi.fn(), closeCopy: vi.fn(), handleCopyToTrip: vi.fn(), handleSelectList: vi.fn(), handleDeleteList: vi.fn(), handleStatusChange: vi.fn(), handleRatePlace: vi.fn(), handleDeleteSelected: vi.fn(), + handleAcceptInvite: vi.fn(), handleDeclineInvite: vi.fn(), allVisibleSelected: false, handleSelectAll: vi.fn(), listPickerMode: null, setListPickerMode: vi.fn(), handleMoveToList: vi.fn(), handleDuplicateToList: vi.fn(), ...over, @@ -658,3 +661,34 @@ describe('MCollections', () => { expect(screen.getByText('add-target:null::1')).toBeInTheDocument() }) }) + +// #2645: the phone had no way to answer a list invite, and the notification's +// link landed on "Start your first collection". +describe('MCollections: invites (#2645)', () => { + const invite = { collection_id: 14, name: 'Lisbon', from: { id: 4, username: 'Lena' } } + + it('FE-MOB-COLSCR-032: with no lists of their own, the invite can be accepted or declined', () => { + mocks.coll = makeHook({ + collections: [], ownedLists: [], activeCollection: null, activeId: null, canShare: false, + incomingInvites: [invite], orderedInvites: [invite], routeInvite: invite, + }) + render() + + expect(screen.getByText('Lisbon')).toBeInTheDocument() + expect(screen.getByText(/Lena/)).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: /collections.invites.accept/ })) + expect(fn('handleAcceptInvite')).toHaveBeenCalledWith(14) + fireEvent.click(screen.getByRole('button', { name: 'collections.invites.decline' })) + expect(fn('handleDeclineInvite')).toHaveBeenCalledWith(14) + }) + + it('FE-MOB-COLSCR-033: invites sit above the lists too, in the order the hook gives', () => { + const other = { collection_id: 3, name: 'Rome', from: { id: 5, username: 'Tom' } } + mocks.coll = makeHook({ incomingInvites: [other, invite], orderedInvites: [invite, other] }) + render() + + const names = screen.getAllByText(/^(Lisbon|Rome)$/).map(el => el.textContent) + expect(names).toEqual(['Lisbon', 'Rome']) + expect(screen.getByPlaceholderText('collections.search')).toBeInTheDocument() + }) +})