Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/mosaic-active-device-dialogs.md
Comment thread
maxyinger marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
Comment thread
maxyinger marked this conversation as resolved.
42 changes: 42 additions & 0 deletions .claude/skills/mosaic/references/views.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,48 @@ Which affordance lands in which slot, and how a consumer's `order` array
rearranges a list, are decisions with no React in them. They live in
`*.layout.ts` / `*.utils.ts` and get their own tests — the view calls the result.

## Removing a row takes focus with it

A dialog returns focus to its trigger on close. When the action removes the row
that trigger sits on — signing a device out, removing an email — the trigger
unmounts and focus falls to `<body>`. Nothing catches it: the menu item that
opened the dialog unmounted with the menu, and a dialog mounted at the section
rather than inside the menu has no floating-tree ancestor to walk back to. A
keyboard user loses their place mid-list and a screen reader announces nothing.

Hand focus to a surviving element. `finalFocus` on `Dialog.Popup` and on the
`Confirmation` block takes a function, resolved when the dialog closes — which is
after the row has gone, so it can pick from what is left:

```tsx
const triggers = useRef(new Map<string, HTMLButtonElement>());
const removed = useRef<number | undefined>(undefined);

const removeRow = async (row: Row) => {
const index = rows.findIndex(candidate => candidate.id === row.id);
await onRemove(row.id);
// Only once it is really gone: a cancelled or failed attempt keeps its own trigger.
removed.current = index;
};

const focusAfterRemove = () => {
const index = removed.current;
removed.current = undefined;
if (index === undefined) {
return null; // null keeps the default — the trigger, which is still there
}
const next = rows[Math.min(index, rows.length - 1)] ?? anchorRow;
return (next && triggers.current.get(next.id)) ?? null;
};
```

Prefer the row that took the removed one's place, the last row when it was the
last, and a control that outlives the list once it is empty.

Test the removal, not just the cancel: `toHaveFocus()` on the row that should
have caught it. A suite that only asserts focus after cancelling passes while
every successful removal drops focus on the floor.

## Testing

Render the view directly with **plain props and `vi.fn()` callbacks**. No Clerk
Expand Down
43 changes: 38 additions & 5 deletions packages/mosaic/src/blocks/confirmation/confirmation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@ import type { ReactNode } from 'react';
import { Banner } from '../../components/banner';
import { Button, SubmitButton } from '../../components/button';
import { Card } from '../../components/card';
import type { DialogHandle, DialogTriggerProps } from '../../components/dialog';
import type { DialogFocusTarget, DialogHandle, DialogTriggerProps } from '../../components/dialog';
import { Dialog } from '../../components/dialog';
import { useConfirmationController } from './confirmation.controller';

/** The weight the confirming button carries: an undoable action takes `primary`. */
export type ConfirmationColor = 'negative' | 'primary';

interface ConfirmationCardProps {
color: ConfirmationColor;
finalFocus: DialogFocusTarget | undefined;
title: string;
description: ReactNode;
actionLabel: string;
Expand All @@ -18,6 +23,8 @@ interface ConfirmationCardProps {
}

function ConfirmationCard({
color,
finalFocus,
title,
description,
actionLabel,
Expand All @@ -27,7 +34,10 @@ function ConfirmationCard({
errorMessage,
}: ConfirmationCardProps) {
return (
<Dialog.Popup compactPlacement='sheet'>
<Dialog.Popup
compactPlacement='sheet'
finalFocus={finalFocus}
>
<Card.Root
elevation='overlay'
renderBranding={false}
Expand Down Expand Up @@ -60,7 +70,7 @@ function ConfirmationCard({
<SubmitButton
type='button'
fullWidth
color='negative'
color={color}
isPending={isConfirming}
onClick={onConfirm}
>
Expand All @@ -75,6 +85,13 @@ function ConfirmationCard({
export interface ConfirmationControlledProps {
/** Whether the dialog is open */
open: boolean;
/** The weight the confirming button carries. An action that can be undone takes `primary` (default: `negative`) */
color?: ConfirmationColor;
/**
* Where focus returns when the dialog closes. Default: the trigger — which a confirmed removal
* may have taken off the page, so a list hands back the row that replaced it instead.
*/
finalFocus?: DialogFocusTarget;
/** Callback when open state changes */
onOpenChange: (open: boolean) => void;
/** Element that opens the dialog */
Expand All @@ -97,6 +114,8 @@ export interface ConfirmationControlledProps {

function ControlledConfirmation({
open,
color = 'negative',
finalFocus,
onOpenChange,
trigger,
title,
Expand All @@ -115,6 +134,8 @@ function ControlledConfirmation({
>
{trigger ? <Dialog.Trigger render={trigger} /> : null}
<ConfirmationCard
color={color}
finalFocus={finalFocus}
title={title}
description={description}
actionLabel={actionLabel}
Expand Down Expand Up @@ -152,6 +173,13 @@ function resolve<Payload, Value>(value: FromPayload<Payload, Value>, payload: Pa
export interface ConfirmationHandleProps<Payload> {
/** Opens the dialog with a payload from anywhere: `handle.open(payload)` */
handle: ConfirmationHandle<Payload>;
/** The weight the confirming button carries. An action that can be undone takes `primary` (default: `negative`) */
color?: ConfirmationColor;
/**
* Where focus returns when the dialog closes. Default: the trigger — which a confirmed removal
* may have taken off the page, so a list hands back the row that replaced it instead.
*/
finalFocus?: DialogFocusTarget;
/** Dialog heading, or a function of the payload */
title: FromPayload<Payload, string>;
/** What the action does and why it warrants a second look, or a function of the payload. Takes markup, for a name to emphasise */
Expand All @@ -166,6 +194,8 @@ export interface ConfirmationHandleProps<Payload> {

function HandleConfirmation<Payload>({
handle,
color = 'negative',
finalFocus,
title,
description,
actionLabel,
Expand All @@ -184,6 +214,8 @@ function HandleConfirmation<Payload>({
{({ payload }) =>
payload === undefined ? null : (
<ConfirmationCard
color={color}
finalFocus={finalFocus}
title={resolve(title, payload)}
description={resolve(description, payload)}
actionLabel={resolve(actionLabel, payload)}
Expand All @@ -205,8 +237,9 @@ function HandleConfirmation<Payload>({
export type ConfirmationProps<Payload = unknown> = ConfirmationControlledProps | ConfirmationHandleProps<Payload>;

/**
* Confirmation dialog for a destructive action that is worth a second look but not worth
* making the user type for. Use `Destructive` for the actions that are.
* Confirmation dialog for an action worth a second look but not worth making the user type for.
* Use `Destructive` for the destructive actions that are. `color` sets the weight the confirming
* button carries: `negative` for what cannot be undone, `primary` for what can.
*
* An `alertdialog`: it announces as an interruption, an outside press cannot answer it, and the
* card withholds its corner dismiss. Escape still closes it, the way the cancel action does. Under
Expand Down
1 change: 1 addition & 0 deletions packages/mosaic/src/blocks/confirmation/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export { Confirmation } from './confirmation';
export type {
ConfirmationColor,
ConfirmationControlledProps,
ConfirmationHandle,
ConfirmationHandleProps,
Expand Down
2 changes: 1 addition & 1 deletion packages/mosaic/src/components/card/card.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const header = stylex.create({
display: 'flex',
flexDirection: 'column',
flexGrow: '1',
rowGap: space['1'],
rowGap: space['0.5'],
},
title: {
color: colorVars['--cl-color-foreground'],
Expand Down
56 changes: 56 additions & 0 deletions packages/mosaic/src/components/data-list/data-list.styles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import * as stylex from '@stylexjs/stylex';

import { colorVars, fontWeightVars, radiusVars, space, typeScaleVars } from '../../tokens.stylex';

export const list = stylex.create({
base: {
borderColor: colorVars['--cl-color-border'],
borderRadius: radiusVars['--cl-radius-lg'],
borderStyle: 'solid',
borderWidth: '1px',
paddingInline: space['3'],
backgroundColor: colorVars['--cl-color-background-subtle'],
display: 'flex',
flexDirection: 'column',
width: '100%',
},
});

export const item = stylex.create({
base: {
paddingBlock: space['3'],
alignItems: 'baseline',
columnGap: space['6'],
display: 'flex',
justifyContent: 'space-between',
},
divided: {
borderBlockEndColor: colorVars['--cl-color-border'],
borderBlockEndStyle: 'solid',
borderBlockEndWidth: {
default: '1px',
':last-child': 0,
},
},
});

export const label = stylex.create({
base: {
color: colorVars['--cl-color-foreground'],
flexShrink: 0,
fontSize: typeScaleVars['--cl-text-sm-size'],
fontWeight: fontWeightVars['--cl-font-medium'],
lineHeight: typeScaleVars['--cl-text-sm-leading'],
},
});

export const value = stylex.create({
base: {
color: colorVars['--cl-color-foreground-secondary'],
fontSize: typeScaleVars['--cl-text-sm-size'],
fontWeight: fontWeightVars['--cl-font-normal'],
lineHeight: typeScaleVars['--cl-text-sm-leading'],
textAlign: 'end',
minWidth: 0,
},
});
48 changes: 48 additions & 0 deletions packages/mosaic/src/components/data-list/data-list.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { render, screen } from '@testing-library/react';
import { describe, expect, it } from 'vitest';

import { MosaicProvider } from '../../MosaicProvider';
import { DataList } from './data-list';

function renderList(props?: { divided?: boolean }) {
return render(
<MosaicProvider>
<DataList.Root {...props}>
<DataList.Item>
<DataList.Label>IP address</DataList.Label>
<DataList.Value>2600:100e:b10b:787b</DataList.Value>
</DataList.Item>
</DataList.Root>
</MosaicProvider>,
);
}

describe('DataList', () => {
it('pairs each value with its label', () => {
const { container } = renderList();

const label = screen.getByText('IP address');
const value = screen.getByText('2600:100e:b10b:787b');
expect(label.tagName).toBe('DT');
expect(value.tagName).toBe('DD');
expect(container.querySelector('dl')).toContainElement(label);
});

it('carries the slot classes a theme targets', () => {
const { container } = renderList();

expect(container.querySelector('.cl-data-list')).toBeInTheDocument();
expect(container.querySelector('.cl-data-list-item')).toBeInTheDocument();
expect(screen.getByText('IP address')).toHaveClass('cl-data-list-label');
expect(screen.getByText('2600:100e:b10b:787b')).toHaveClass('cl-data-list-value');
});

it('reflects whether the items are ruled', () => {
const { container, unmount } = renderList();
expect(container.querySelector('.cl-data-list')).toHaveAttribute('data-divided');
unmount();

const plain = renderList({ divided: false });
expect(plain.container.querySelector('.cl-data-list')).not.toHaveAttribute('data-divided');
});
});
Loading
Loading