diff --git a/client/src/components/Planner/PlaceFormModal.tsx b/client/src/components/Planner/PlaceFormModal.tsx index c2eb7086e..6e3878487 100644 --- a/client/src/components/Planner/PlaceFormModal.tsx +++ b/client/src/components/Planner/PlaceFormModal.tsx @@ -1196,9 +1196,8 @@ export default function PlaceFormModal(props: PlaceFormModalProps) { options={[ { value: '', label: t('places.noCategory') }, ...(categories || []).map(c => ({ - // form.category_id is a string; CustomSelect matches options by - // strict equality, so the option value must be a string too — - // otherwise the chosen category never renders in the trigger. + // A string like form.category_id, so the picked option hands + // back the same kind of value the form already keeps. value: String(c.id), label: c.name, })), diff --git a/client/src/components/Trips/TripMembersModal.test.tsx b/client/src/components/Trips/TripMembersModal.test.tsx index d0dfb2e97..d82fa8396 100644 --- a/client/src/components/Trips/TripMembersModal.test.tsx +++ b/client/src/components/Trips/TripMembersModal.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-MEMBERS-001 to FE-COMP-MEMBERS-056 +// FE-COMP-MEMBERS-001 to FE-COMP-MEMBERS-057 import type { Mock } from 'vitest'; import { act, render, screen, fireEvent, waitFor } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; @@ -725,6 +725,26 @@ describe('TripMembersModal', () => { expect(addToast).toHaveBeenCalledWith('alice added', 'success', undefined); }); + // #2478: the pick showed nowhere, only the Invite button knew about it. + it('FE-COMP-MEMBERS-057: the picked user shows in the field before the invite is sent', async () => { + const user = userEvent.setup(); + server.use(http.post('/api/trips/1/members', () => HttpResponse.json({ success: true }))); + render(); + + await screen.findByText('Invite User'); + const trigger = screen.getByText('Select user…').closest('button') as HTMLButtonElement; + await user.click(trigger); + await user.click(await screen.findByRole('button', { name: 'alice' })); + + expect(trigger).toHaveTextContent('alice'); + expect(screen.queryByText('Select user…')).toBeNull(); + expect(screen.getByRole('button', { name: 'Invite' })).toBeEnabled(); + + // Once sent, the field is free for the next pick again. + await user.click(screen.getByRole('button', { name: 'Invite' })); + await waitFor(() => expect(trigger).toHaveTextContent('Select user…')); + }); + it('FE-COMP-MEMBERS-042: transferring ownership reloads the app', async () => { const onClose = vi.fn(); const reload = vi.fn(); diff --git a/client/src/components/shared/CustomSelect.test.tsx b/client/src/components/shared/CustomSelect.test.tsx index 2f13fc5b6..da081bbc7 100644 --- a/client/src/components/shared/CustomSelect.test.tsx +++ b/client/src/components/shared/CustomSelect.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, fireEvent } from '../../../tests/helpers/render'; +import { render, screen, within } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; import CustomSelect from './CustomSelect'; @@ -145,6 +145,46 @@ describe('CustomSelect', () => { expect(menu.style.maxWidth).toBe('180px'); }); + // #2478: the trip share dialog stored String(value) against options keyed by numeric + // user ids, so the pick never showed in the trigger although it was made. + const USERS = [ + { value: 7, label: 'alice' }, + { value: 12, label: 'bob' }, + ]; + + it('FE-COMP-SELECT-014: a value held as text finds the option keyed by the same number', async () => { + const user = userEvent.setup(); + render(); + + const trigger = screen.getByRole('button'); + expect(trigger).toHaveTextContent('bob'); + expect(screen.queryByText('Pick a user')).toBeNull(); + + // The open list marks the same option with its check, and only that one. + await user.click(trigger); + const menu = within(screen.getByText('alice').closest('div[style*="position: fixed"]') as HTMLElement); + expect(menu.getByRole('button', { name: 'bob' }).querySelector('svg')).not.toBeNull(); + expect(menu.getByRole('button', { name: 'alice' }).querySelector('svg')).toBeNull(); + }); + + it('FE-COMP-SELECT-015: a numeric value finds the option keyed by the same text', () => { + render(); + expect(screen.getByRole('button')).toHaveTextContent('March'); + }); + + it('FE-COMP-SELECT-016: a caller holding nothing still sees the placeholder', () => { + render(); + expect(screen.getByRole('button')).toHaveTextContent('Pick a user'); + }); + + it('FE-COMP-SELECT-017: picking hands back the option value as it is', async () => { + const user = userEvent.setup(); + render(); + await user.click(screen.getByRole('button')); + await user.click(screen.getByRole('button', { name: 'alice' })); + expect(onChange).toHaveBeenCalledWith(7); + }); + it('FE-COMP-SELECT-008: disabled state prevents the dropdown from opening', async () => { const user = userEvent.setup(); render(); diff --git a/client/src/components/shared/CustomSelect.tsx b/client/src/components/shared/CustomSelect.tsx index 6c02c27d1..82932e8b3 100644 --- a/client/src/components/shared/CustomSelect.tsx +++ b/client/src/components/shared/CustomSelect.tsx @@ -4,8 +4,8 @@ import { ChevronDown, Check } from 'lucide-react' import { useAnchoredPosition, scrollAnchorIntoView } from '../../hooks/useAnchoredPosition' interface SelectOption { - // Callers use both string keys and numeric ids (e.g. day/place ids) as values; - // the component only does strict-equality lookups and key rendering, so either works. + // Callers use both string keys and numeric ids (e.g. day/place ids) as values, and + // the value they hold need not be of the same kind as the options: see sameValue. value: string | number label: string icon?: React.ReactNode @@ -36,6 +36,20 @@ interface CustomSelectProps { menuFit?: 'anchor' | 'content' } +/** + * Whether an option is the one the caller holds. + * + * onChange hands back the option's own value, and callers routinely store it in another + * form: the trip share dialog kept String(value) against options keyed by numeric user + * ids, so a strict comparison never found the pick and the trigger stayed on its + * placeholder while the invite button already knew who was chosen (#2478). An id reads + * the same as a number or as text, so both sides are compared as text. A caller holding + * nothing (null or undefined at runtime) matches no option. + */ +function sameValue(held: string | number | null | undefined, option: string | number): boolean { + return held != null && String(held) === String(option) +} + /** How wide a content-fitted menu may get before it stops reading as a menu. */ const MENU_MAX_WIDTH = 420 @@ -92,7 +106,7 @@ export default function CustomSelect({ return () => document.removeEventListener('mousedown', handleClick) }, [open]) - const selected = options.find(o => o.value === value) + const selected = options.find(o => sameValue(value, o.value)) const filtered = searchable && search ? (() => { const q = search.toLowerCase() @@ -232,7 +246,7 @@ export default function CustomSelect({ ) } - const isSelected = option.value === value + const isSelected = sameValue(value, option.value) return (