From f39f10d5932a5ae3dcafb4ad5e6bdc26f2e17b8a Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 11:05:37 +0100 Subject: [PATCH] fix: mark the contact a menu is for, and no member on a group's page --- .../contacts/__tests__/contacts-app.test.tsx | 12 ++++ .../__tests__/contacts-ux-pim.test.tsx | 61 +++++++++++++++++++ components/contacts/contact-group-detail.tsx | 5 +- components/contacts/contact-list-item.tsx | 7 ++- components/contacts/contact-list.tsx | 1 + components/contacts/contacts-app.tsx | 3 +- 6 files changed, 82 insertions(+), 7 deletions(-) diff --git a/components/contacts/__tests__/contacts-app.test.tsx b/components/contacts/__tests__/contacts-app.test.tsx index 7d8ba656b..31f533ba1 100644 --- a/components/contacts/__tests__/contacts-app.test.tsx +++ b/components/contacts/__tests__/contacts-app.test.tsx @@ -307,4 +307,16 @@ describe('ContactsApp', () => { fireEvent.click(screen.getByText('context_menu.duplicate')); await waitFor(() => expect(useToastStore.getState().toasts.at(-1)?.title).toBe('toast.error_create')); }); + + it("marks no member in the list on a group's page, even with a contact open before", () => { + const team: ContactCard = { id: 'g-team', addressBookIds: {}, kind: 'group', name: { full: 'Team' }, members: { 'c-alice': true, 'c-bob': true } }; + renderContacts([alice, bob, team]); + fireEvent.click(screen.getByText('Alice Smith')); + expect(document.querySelector('[data-contact-id="c-alice"]')).toHaveAttribute('aria-selected', 'true'); + fireEvent.click(screen.getAllByRole('button', { name: 'open_categories' })[0]); + fireEvent.click(within(drawer()).getByText('Team')); + expect(screen.getByText('groups.members_label')).toBeInTheDocument(); + expect(document.querySelector('[data-contact-id="c-alice"]')).toHaveAttribute('aria-selected', 'false'); + expect(document.querySelectorAll('[data-contact-id][aria-selected="true"]')).toHaveLength(0); + }); }); diff --git a/components/contacts/__tests__/contacts-ux-pim.test.tsx b/components/contacts/__tests__/contacts-ux-pim.test.tsx index c0dbef52c..496ca5a72 100644 --- a/components/contacts/__tests__/contacts-ux-pim.test.tsx +++ b/components/contacts/__tests__/contacts-ux-pim.test.tsx @@ -20,6 +20,8 @@ vi.mock('@/components/contacts/contact-activity', () => ({ ContactActivity: () = import { ContactForm } from '../contact-form'; import { ContactDetail } from '../contact-detail'; import { ContactsSidebar } from '../contacts-sidebar'; +import { ContactList } from '../contact-list'; +import { ContactGroupDetail } from '../contact-group-detail'; const anna: ContactCard = { id: 'c1', @@ -28,6 +30,12 @@ const anna: ContactCard = { emails: { e0: { address: 'anna@example.net', contexts: { private: true } } }, phones: { p0: { number: '+49 30 1', features: { voice: true }, contexts: { private: true } } }, }; +const ben: ContactCard = { + id: 'c2', + addressBookIds: {}, + name: { components: [{ kind: 'given', value: 'Ben' }, { kind: 'surname', value: 'Muster' }], isOrdered: true }, + emails: { e0: { address: 'ben@example.net' } }, +}; describe('pim-19: standard words read as people say them', () => { it('offers "Phone" and "Home" in the editor, storing voice and private as before', () => { @@ -122,3 +130,56 @@ describe('pim-26: the Contacts menus', () => { expect(within(menu).getByText('ux_pim_contact.new_address_book_menu')).toBeInTheDocument(); }); }); + +const listProps = { + contacts: [anna, ben], + searchQuery: '', + onSearchChange: vi.fn(), + onSelectContact: vi.fn(), + onCreateNew: vi.fn(), + selectedContactIds: new Set(), + onToggleSelection: vi.fn(), + onSelectRangeContacts: vi.fn(), + onSelectAll: vi.fn(), + onClearSelection: vi.fn(), + onBulkDelete: vi.fn(), + onBulkAddToGroup: vi.fn(), + onBulkExport: vi.fn(), + onEditContact: vi.fn(), + onDeleteContact: vi.fn(), + onAddContactToGroup: vi.fn(), +}; + +describe('pim-17: right-clicking a contact marks whose menu is open', () => { + it('gives the right-clicked row the hover colour while its menu is open', () => { + render(); + const row = (id: string) => document.querySelector(`[data-contact-id="${id}"]`) as HTMLElement; + expect(row('c2')).not.toHaveAttribute('data-menu-open'); + fireEvent.contextMenu(row('c2')); + expect(screen.getByRole('menu')).toBeInTheDocument(); + expect(row('c2')).toHaveAttribute('data-menu-open'); + expect(row('c2').className).toMatch(/(^|\s)bg-muted(\s|$)/); + // The open contact stays as it is. + expect(row('c1')).not.toHaveAttribute('data-menu-open'); + fireEvent.keyDown(screen.getByRole('menu'), { key: 'Escape' }); + expect(row('c2')).not.toHaveAttribute('data-menu-open'); + }); +}); + +describe("pim-18: a group's page manages its members", () => { + it('shows "Remove from group" on every member row, not only on hover', () => { + render( + , + ); + const remove = screen.getAllByRole('button', { name: 'contacts.groups.remove_member' }); + expect(remove).toHaveLength(2); + for (const button of remove) expect(button.className).not.toMatch(/(^|\s)opacity-0(\s|$)/); + }); +}); diff --git a/components/contacts/contact-group-detail.tsx b/components/contacts/contact-group-detail.tsx index 48a251453..e7cba4665 100644 --- a/components/contacts/contact-group-detail.tsx +++ b/components/contacts/contact-group-detail.tsx @@ -135,12 +135,9 @@ export function ContactGroupDetail({ )} + {/* Always shown: this list is where members are managed (#pim-18). */} onRemoveMember(member.id)} > diff --git a/components/contacts/contact-list-item.tsx b/components/contacts/contact-list-item.tsx index a376e5fae..8181ae43d 100644 --- a/components/contacts/contact-list-item.tsx +++ b/components/contacts/contact-list-item.tsx @@ -25,9 +25,11 @@ interface ContactListItemProps { onFocus?: () => void; /** Long-press on touch: the row's menu, as right-click does (R7). */ onLongPress?: (row: HTMLElement) => void; + /** The row's menu is open: it keeps the hover colour, so it's clear whose menu it is. */ + isMenuTarget?: boolean; } -export function ContactListItem({ contact, isSelected, isChecked, hasSelection, density, selectedContactIds, onClick, onCheckboxClick, onContextMenu, isFocused = false, onFocus, onLongPress }: ContactListItemProps) { +export function ContactListItem({ contact, isSelected, isChecked, hasSelection, density, selectedContactIds, onClick, onCheckboxClick, onContextMenu, isFocused = false, onFocus, onLongPress, isMenuTarget = false }: ContactListItemProps) { const rowRef = useRef(null); const longPress = useLongPress(() => { if (rowRef.current) onLongPress?.(rowRef.current); @@ -72,6 +74,7 @@ export function ContactListItem({ contact, isSelected, isChecked, hasSelection, aria-selected={isSelected || isChecked} tabIndex={isFocused ? 0 : -1} data-contact-id={contact.id} + data-menu-open={isMenuTarget || undefined} onFocus={onFocus} draggable onDragStart={handleDragStart} @@ -81,7 +84,7 @@ export function ContactListItem({ contact, isSelected, isChecked, hasSelection, "w-full flex items-center cursor-pointer select-none transition-all duration-200 border-b border-border outline-none focus-inset", isSelected ? "bg-selection shadow-sm" - : "bg-background hover:bg-muted hover:shadow-sm", + : isMenuTarget ? "bg-muted shadow-sm" : "bg-background hover:bg-muted hover:shadow-sm", isChecked && !isSelected && "ring-2 ring-primary/20 bg-selection/60", )} style={{ gap: 'var(--density-item-gap)', paddingInline: '16px', paddingBlock: 'var(--density-item-py)' }} diff --git a/components/contacts/contact-list.tsx b/components/contacts/contact-list.tsx index 1fec581dd..fe2379c77 100644 --- a/components/contacts/contact-list.tsx +++ b/components/contacts/contact-list.tsx @@ -715,6 +715,7 @@ export function ContactList({ isChecked={selectedContactIds.has(contact.id)} hasSelection={hasSelection} isFocused={contact.id === cursorId} + isMenuTarget={contextMenu.isOpen && contextMenu.data?.id === contact.id} onFocus={() => setFocusedId(contact.id)} density={density} selectedContactIds={selectedContactIds} diff --git a/components/contacts/contacts-app.tsx b/components/contacts/contacts-app.tsx index c7f6723e1..8e2d237d0 100644 --- a/components/contacts/contacts-app.tsx +++ b/components/contacts/contacts-app.tsx @@ -1409,7 +1409,8 @@ function ContactsAppContent({ linkSegments: routeSegments }: ContactsAppProps =