diff --git a/.changeset/profile-tab-responsiveness.md b/.changeset/profile-tab-responsiveness.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/profile-tab-responsiveness.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/bundlewatch.config.json b/packages/mosaic/bundlewatch.config.json index c4b17c00eb7..8fb82f4a261 100644 --- a/packages/mosaic/bundlewatch.config.json +++ b/packages/mosaic/bundlewatch.config.json @@ -1,6 +1,6 @@ { "files": [ { "path": "./dist/index.js", "maxSize": "125KB" }, - { "path": "./dist/styles.css", "maxSize": "14KB" } + { "path": "./dist/styles.css", "maxSize": "15KB" } ] } diff --git a/packages/mosaic/src/components/panel/panel.styles.ts b/packages/mosaic/src/components/panel/panel.styles.ts index c9e74458738..adc85003f85 100644 --- a/packages/mosaic/src/components/panel/panel.styles.ts +++ b/packages/mosaic/src/components/panel/panel.styles.ts @@ -13,10 +13,7 @@ export const styles = stylex.create({ display: 'block', }, - /** - * The headline as a button: the heading's own type, inline so the caret can align to its - * x-height, with a little room around it for the focus ring. - */ + // Inline so the caret can align to the title's x-height. navTrigger: { font: 'inherit', borderRadius: radiusVars['--cl-radius-md'], @@ -28,16 +25,10 @@ export const styles = stylex.create({ display: 'inline', textAlign: 'start', }, - /** - * Beside the title, `vertical-align: middle`: the caret's midpoint on the baseline plus half the - * x-height, which centers it on the lowercase letters rather than the line box. Sized in `em` - * through the icon's `inherit` size, so it scales with the heading; coloured through the icon's - * own variable rather than `color`, which the icon sets itself. - */ + // `vertical-align: middle` centers the caret on the title's x-height rather than the line box. caret: { '--_cl-icon-color': colorVars['--cl-color-foreground-secondary'], - fontSize: '0.6em', - marginInlineStart: '0.25em', + marginInlineStart: space['1'], verticalAlign: 'middle', }, sections: { diff --git a/packages/mosaic/src/components/panel/panel.tsx b/packages/mosaic/src/components/panel/panel.tsx index 2cb11c725ce..174bebb1cd8 100644 --- a/packages/mosaic/src/components/panel/panel.tsx +++ b/packages/mosaic/src/components/panel/panel.tsx @@ -2,6 +2,7 @@ import * as stylex from '@stylexjs/stylex'; import React from 'react'; import { useRender } from '../../primitives/utils'; +import { isKeyboardEvent } from '../../primitives/utils/interaction-modality'; import type { MosaicComponentProps } from '../../props'; import { mergeStyleProps, themeProps } from '../../props'; import { focusOutline } from '../../utils/focus-outline.styles'; @@ -24,15 +25,7 @@ const Root = React.forwardRef(function PanelRoot }); }); -/** - * A page's headline. Inside a profile that has gone compact the headline IS the way to the other - * pages: the heading holds a button — the title, and a caret beside it — that opens the navigation - * sheet. Anywhere else — the wide layout, or a page rendered on its own — it is the heading alone. - * - * The caret sits `vertical-align: middle`, which CSS defines as the box's midpoint on the parent's - * baseline plus half its x-height: optically centered on the lowercase letters rather than on the - * line box. That needs an inline formatting context, so the button is `display: inline`. - */ +// In a compact profile the heading holds a button that opens the navigation. const Title = React.forwardRef(function PanelTitle( { children, render, xstyle, ...rest }, ref, @@ -42,7 +35,6 @@ const Title = React.forwardRef(function PanelTi const registerPageTitle = profile?.registerPageTitle; const page = panel?.value; const level = useHeadingLevel(); - // The sheet's return-focus target. A no-op outside a profile's page, where there is no sheet. const registerTrigger = React.useCallback( (element: HTMLButtonElement | null) => { if (page !== undefined) { @@ -69,7 +61,9 @@ const Title = React.forwardRef(function PanelTi type='button' aria-haspopup='dialog' aria-expanded={profile.navOpen} - onClick={profile.openNav} + onClick={event => + profile.navOpen ? profile.closeNav() : profile.openNav(isKeyboardEvent(event.nativeEvent)) + } {...mergeStyleProps( themeProps('profile-nav-trigger'), stylex.props(reset.base, styles.navTrigger, focusOutline.visible), @@ -78,7 +72,7 @@ const Title = React.forwardRef(function PanelTi {children} @@ -105,9 +99,7 @@ const Sections = React.forwardRef(function P }); /** - * A page of content, composed through `Panel.Root`, `Panel.Title`, and `Panel.Sections`. The - * sections sit one heading level below the title. Every part accepts the Mosaic `render` prop and - * forwards its ref. + * A page of content. `Panel.Sections` sit one heading level below `Panel.Title`. * * ```tsx * diff --git a/packages/mosaic/src/components/popover/popover.tsx b/packages/mosaic/src/components/popover/popover.tsx index 6edaa3c27c7..bcbdcb79603 100644 --- a/packages/mosaic/src/components/popover/popover.tsx +++ b/packages/mosaic/src/components/popover/popover.tsx @@ -2,7 +2,7 @@ import * as stylex from '@stylexjs/stylex'; import React from 'react'; import { useAccessibleNameWarning } from '../../hooks/useAccessibleNameWarning'; -import type { PopoverProps as HeadlessPopoverProps } from '../../primitives/popover'; +import type { PopoverFocusTarget, PopoverProps as HeadlessPopoverProps } from '../../primitives/popover'; import { Popover as Primitive } from '../../primitives/popover'; import type { MosaicComponentProps } from '../../props'; import { mergeStyleProps, themeProps } from '../../props'; @@ -13,19 +13,12 @@ export type PopoverSize = 'sm' | 'md' | 'lg' | 'anchor'; export type PopoverRootProps = HeadlessPopoverProps; -/** - * The headless parts type their props (and the `render` callback's argument) against - * the raw tag props, which carry the non-standard HTML `color` attribute typed - * `string`. Re-typing them through `MosaicComponentProps` drops it, so a `render` - * callback can spread straight into a Mosaic component whose own `color` is a narrow - * variant union. - */ +// Drops the raw `color: string` attr so a `render` callback can spread into a Mosaic component. export type PopoverTriggerProps = MosaicComponentProps<'button'>; export type PopoverCloseProps = MosaicComponentProps<'button'>; export type PopoverTitleProps = MosaicComponentProps<'h2'>; export type PopoverDescriptionProps = MosaicComponentProps<'p'>; -/** The anchor. Renders a ` + + + + + + + Close + + + + + + ); + } + + it('treats a press on the anchor as its own, not an outside press', async () => { + const user = userEvent.setup(); + render(); + + const anchor = screen.getByRole('button', { name: 'Anchor' }); + await user.click(anchor); + expect(screen.getByRole('dialog', { name: 'Anchored' })).toBeInTheDocument(); + + await user.click(anchor); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + }); + + it('returns focus to the anchor on close', async () => { + const user = userEvent.setup(); + render(); + + const anchor = screen.getByRole('button', { name: 'Anchor' }); + await user.click(anchor); + await user.click(screen.getByRole('button', { name: 'Close' })); + + await waitFor(() => expect(anchor).toHaveFocus()); + }); + + it('returns focus where finalFocus points', async () => { + const user = userEvent.setup(); + render( screen.getByRole('button', { name: 'Elsewhere' })} />); + + await user.click(screen.getByRole('button', { name: 'Anchor' })); + await user.click(screen.getByRole('button', { name: 'Close' })); + + await waitFor(() => expect(screen.getByRole('button', { name: 'Elsewhere' })).toHaveFocus()); + }); + }); + describe('ARIA attributes', () => { it('positioner has aria-labelledby linked to title', () => { renderPopover({ defaultOpen: true }); @@ -139,8 +208,6 @@ describe('Popover', () => { }); it('keeps positioner aria-labelledby/aria-describedby wired to the correct elements', () => { - // The primitive owns the ids on Title and Description (id is omitted from - // their public props) — the aria pairing must always resolve correctly. renderPopover({ defaultOpen: true }); const title = document.querySelector('[data-testid="popover-title"]'); @@ -154,8 +221,6 @@ describe('Popover', () => { }); it('omits aria-labelledby and aria-describedby when no Title or Description is rendered', () => { - // Title and Description are optional. When absent, the positioner must not - // emit dangling idrefs pointing at elements that were never rendered. render( Open popover @@ -247,9 +312,7 @@ describe('Popover', () => { expect(positioner).toHaveAttribute('data-side', 'bottom'); }); - // jsdom reports a zero-sized viewport, which leaves `shift` no room and pins the alignment - // axis to its padding whatever the offset asked for. Giving it a viewport is what lets the - // offset show up in the transform at all. + // jsdom's zero-sized viewport leaves `shift` no room, which would hide the offset. async function transformWithViewport(props: Partial>) { vi.spyOn(document.documentElement, 'clientWidth', 'get').mockReturnValue(1024); vi.spyOn(document.documentElement, 'clientHeight', 'get').mockReturnValue(768); @@ -279,7 +342,6 @@ describe('Popover', () => { renderPopover(); await user.click(screen.getByRole('button', { name: 'Open popover' })); - // FloatingFocusManager schedules focus via requestAnimationFrame await new Promise(r => requestAnimationFrame(r)); const positioner = document.querySelector('[data-testid="popover-positioner"]'); @@ -299,9 +361,7 @@ describe('Popover', () => { it('focuses the first tabbable element when opened with the keyboard', async () => { renderPopover(); - // A button handles Enter/Space itself, so keyboard activation reaches the popover - // as a click with no pointer behind it. userEvent stamps its synthetic keyboard - // click with a pointerType, which browsers do not. + // userEvent stamps keyboard clicks with a pointerType; browsers send `detail: 0` and none. fireEvent.click(screen.getByRole('button', { name: 'Open popover' }), { detail: 0 }); await waitFor(() => expect(screen.getByRole('button', { name: 'Close' })).toHaveFocus()); }); diff --git a/packages/swingset/src/stories/panel.component.mdx b/packages/swingset/src/stories/panel.component.mdx index fe7fd875c85..089c31457e4 100644 --- a/packages/swingset/src/stories/panel.component.mdx +++ b/packages/swingset/src/stories/panel.component.mdx @@ -5,7 +5,7 @@ import * as PanelStories from './panel.component.stories'; Panel lays out one page of content: a title, and the sections below it. `Panel.Root` stacks its children, `Panel.Title` renders the page's heading, and `Panel.Sections` stacks a column of [Section](/components/section)s one heading level below the title. Inside a -[Profile](/components/profile) page that has gone compact, `Panel.Title` opens the navigation sheet. +[Profile](/components/profile) page that has gone compact, `Panel.Title` opens the navigation. ## Example diff --git a/packages/swingset/src/stories/profile.component.mdx b/packages/swingset/src/stories/profile.component.mdx index 6c7ab590943..fef5cc99df4 100644 --- a/packages/swingset/src/stories/profile.component.mdx +++ b/packages/swingset/src/stories/profile.component.mdx @@ -58,7 +58,7 @@ height and the pages scrolling inside; `flush` is the page's own content — no scrolls, the columns a gap apart, held to a reading width and centered. A `NavItem` and a `Page` pair by `value`. `icon` and `badge` take nodes, so a page can supply its own leading mark and trailing status. A `Badge` passed to `badge` is `neutral` unless it sets its own `color`. `Title` is a visually hidden heading: it names the navigation, the compact -sheet, and — inside a dialog — the dialog itself, the way `Card.Title` does. +popover or sheet, and — inside a dialog — the dialog itself, the way `Card.Title` does. ## Parts @@ -92,12 +92,11 @@ renders. `Panel.Sections` puts section titles one level below the page title. Se Below `48rem` of the profile's **own** width — or, over the page, the dialog's viewport's, so the profile collapses exactly when the dialog fills the screen — the frame goes: the profile is the page -there, flush with whatever holds it, and the navigation leaves the column for a sheet: `Panel.Title` -grows a caret that opens a `Drawer` holding the tablist, and a choice closes it. Where the tablist -renders is a DOM decision — one tablist, in the column or in the sheet, never both — so the root -reads the query's answer off a sentinel it renders, `1px` wide and `2px` once the query matches, -rather than measuring itself: the breakpoint lives in CSS alone. The same surface collapses in a -narrow layout slot, an inline dialog, or a phone alike. +there, flush with whatever holds it, and the navigation leaves the column: `Panel.Title` grows a +caret that opens the tablist, and a choice closes it. On a viewport of `40rem` or more the tablist +opens in a `Popover` under the title; on a phone, in a `Drawer` as tall as its content. The root +reads the layout off a sentinel it renders: `1px` wide, `2px` once the query matches, `3px` on a +phone's viewport as well. ## Customising the marks