fix: mark the contact a menu is for, and no member on a group's page

This commit is contained in:
Linus Rath
2026-11-04 11:05:37 +01:00
parent 3df3b800f8
commit f39f10d593
6 changed files with 82 additions and 7 deletions
@@ -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);
});
});
@@ -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<string>(),
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(<ContactList {...listProps} selectedContactId="c1" />);
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(
<ContactGroupDetail
group={{ id: 'g1', addressBookIds: {}, kind: 'group', name: { full: 'Team' }, members: { c1: true, c2: true } }}
members={[anna, ben]}
onEdit={vi.fn()}
onDelete={vi.fn()}
onRemoveMember={vi.fn()}
onSelectMember={vi.fn()}
/>,
);
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|$)/);
});
});
+1 -4
View File
@@ -135,12 +135,9 @@ export function ContactGroupDetail({
)}
</div>
</button>
{/* Always shown: this list is where members are managed (#pim-18). */}
<IconButton
label={t("groups.remove_member", { name: mName })}
className={cn(
"transition-opacity",
isMobile ? "opacity-100" : "opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 pointer-coarse:opacity-100"
)}
onClick={() => onRemoveMember(member.id)}
>
<UserMinus className="text-muted-foreground" />
+5 -2
View File
@@ -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<HTMLDivElement>(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)' }}
+1
View File
@@ -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}
+2 -1
View File
@@ -1409,7 +1409,8 @@ function ContactsAppContent({ linkSegments: routeSegments }: ContactsAppProps =
<ContactList
contacts={displayedContacts}
isContactReadOnly={isContactReadOnly}
selectedContactId={selectedContactId}
// A group's page opens no member: none is marked in the list (#pim-18).
selectedContactId={view === "group-detail" || view === "group-edit" ? null : selectedContactId}
searchQuery={searchQuery}
onSearchChange={setSearchQuery}
onSelectContact={guard(handleSelectContact)}