diff --git a/packages/design-system/src/components/Attribute/Attribute.e2e.ts b/packages/design-system/src/components/Attribute/Attribute.e2e.ts index ed163a509..6712fcf8d 100644 --- a/packages/design-system/src/components/Attribute/Attribute.e2e.ts +++ b/packages/design-system/src/components/Attribute/Attribute.e2e.ts @@ -10,6 +10,8 @@ const attributeStory = createStoryHelper('data-display-attribute', [ 'Horizontal With Actions Menu Only', 'With Inline Edit', 'Horizontal With Inline Edit', + 'Mixed', + 'Horizontal Mixed', ] as const); test.describe('Component: Attribute', () => { @@ -84,6 +86,20 @@ test.describe('Component: Attribute', () => { await expect(page.getByTestId('attr--input')).toBeFocused(); await expect(page).toHaveScreenshot({ animations: 'disabled' }); }); + + test('Should render mixed hosts with aligned values correctly', async ({ page }) => { + // Plain value, AttributeActions target, and InlineEdit host stacked in + // one column — the value's left text edge must line up across all rows. + await attributeStory.goto(page, 'Mixed'); + await expect(page).toHaveScreenshot({ animations: 'disabled' }); + }); + + test('Should render horizontal mixed hosts with aligned values correctly', async ({ page }) => { + // Same alignment contract as the vertical Mixed story — the -ml-7 pull + // applies to InlineEdit and AttributeActionsTarget in both orientations. + await attributeStory.goto(page, 'Horizontal Mixed'); + await expect(page).toHaveScreenshot({ animations: 'disabled' }); + }); }); test.describe('Interactions', () => { diff --git a/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-horizontal-composition-correctly-chromium.png b/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-horizontal-composition-correctly-chromium.png index a633ab500..5d6e0e727 100644 Binary files a/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-horizontal-composition-correctly-chromium.png and b/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-horizontal-composition-correctly-chromium.png differ diff --git a/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-horizontal-mixed-hosts-with-aligned-values-correctly-chromium.png b/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-horizontal-mixed-hosts-with-aligned-values-correctly-chromium.png new file mode 100644 index 000000000..6c1966505 Binary files /dev/null and b/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-horizontal-mixed-hosts-with-aligned-values-correctly-chromium.png differ diff --git a/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-hove-adf10-with-full-rounded-corners-on-the-actions-target-chromium.png b/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-hove-adf10-with-full-rounded-corners-on-the-actions-target-chromium.png index dcf8d7f44..cc240a19b 100644 Binary files a/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-hove-adf10-with-full-rounded-corners-on-the-actions-target-chromium.png and b/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-hove-adf10-with-full-rounded-corners-on-the-actions-target-chromium.png differ diff --git a/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-mixed-hosts-with-aligned-values-correctly-chromium.png b/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-mixed-hosts-with-aligned-values-correctly-chromium.png new file mode 100644 index 000000000..3d5c8eda2 Binary files /dev/null and b/packages/design-system/src/components/Attribute/Attribute.e2e.ts-snapshots/Component-Attribute-Visual-Should-render-mixed-hosts-with-aligned-values-correctly-chromium.png differ diff --git a/packages/design-system/src/components/Attribute/Attribute.stories.tsx b/packages/design-system/src/components/Attribute/Attribute.stories.tsx index e40df27d6..1d76ecaff 100644 --- a/packages/design-system/src/components/Attribute/Attribute.stories.tsx +++ b/packages/design-system/src/components/Attribute/Attribute.stories.tsx @@ -894,3 +894,60 @@ export const WithInlineEdit: StoryFn = () => ; export const HorizontalWithInlineEdit: StoryFn = () => ( ); + +// ─── Mixed hosts ───────────────────────────────────────────────────────────── +// Plain value, AttributeActions target, and InlineEdit host stacked in one +// column — the value's left text edge must line up across all three rows in +// both orientations (AttributeValue's -ml-7 seam pulls the hover boxes left, +// not the text). + +function AttributeMixedExample({ orientation }: { orientation?: AttributeProps['orientation'] }) { + const [name, setName] = useState('Checkout API'); + return ( +
+ + Request ID + + abc-123-def-456 + + + + + Source IP + + + + 142.198.167.52 + + {renderActionsItems()} + + + + + + Name + + setName(v as string)}> + {name} + + + + + + + +
+ ); +} + +export const Mixed: StoryFn = () => ; +Mixed.storyName = 'Mixed (plain + Actions + InlineEdit)'; + +export const HorizontalMixed: StoryFn = () => ; +HorizontalMixed.storyName = 'Horizontal Mixed (plain + Actions + InlineEdit)'; diff --git a/packages/design-system/src/components/Attribute/Attribute.tsx b/packages/design-system/src/components/Attribute/Attribute.tsx index f73f823a5..0d9ceb552 100644 --- a/packages/design-system/src/components/Attribute/Attribute.tsx +++ b/packages/design-system/src/components/Attribute/Attribute.tsx @@ -52,7 +52,7 @@ export const Attribute: FC = ({ data-orientation={orientation} data-empty={!loading && isEmpty ? '' : undefined} className={cn( - isHorizontal ? 'flex flex-row items-start gap-4' : 'flex flex-col', + isHorizontal ? 'flex flex-row items-start gap-12' : 'flex flex-col', className, )} > diff --git a/packages/design-system/src/components/Attribute/AttributeInlineEdit.integration.test.tsx b/packages/design-system/src/components/Attribute/AttributeInlineEdit.integration.test.tsx index 5d54b65e2..e46b96427 100644 --- a/packages/design-system/src/components/Attribute/AttributeInlineEdit.integration.test.tsx +++ b/packages/design-system/src/components/Attribute/AttributeInlineEdit.integration.test.tsx @@ -78,16 +78,16 @@ describe('InlineEdit integration', () => { expect(value.className).toContain('has-[[data-slot=inline-edit]]:overflow-visible'); }); - it('omits the -ml-7 hit-target offset in horizontal orientation', () => { - // In horizontal orientation AttributeValue sits right next to - // AttributeLabel with only a 4px gap — a 7px pull would eat into the - // label's space, so the offset only applies when stacked vertically. + it('keeps the -ml-7 hit-target pulls in horizontal orientation', () => { + // Both pulls apply in both orientations so the guest's text lines up + // where a plain value would sit (see AttributeValue.tsx). render( {}} orientation='horizontal' />); const value = screen.getByTestId('attr--value'); - expect(value.className).not.toContain('has-[[data-slot=inline-edit]]:-ml-7'); - expect(value.className).not.toContain('has-[[data-slot=attribute-actions-target]]:-ml-7'); - // Nothing to cancel when there's no ambient -ml-7 pull. - expect(value.className).not.toContain('[&_[data-slot=inline-edit-error]]:ml-7'); + expect(value.className).toContain('has-[[data-slot=inline-edit]]:-ml-7'); + expect(value.className).toContain('has-[[data-slot=attribute-actions-target]]:-ml-7'); + // The pull is active for the hosted InlineEdit, so its error cancel + // must follow in horizontal too. + expect(value.className).toContain('[&_[data-slot=inline-edit-error]]:ml-7'); // The vertical-axis cancellation still applies regardless of orientation. expect(value.className).toContain('[&_[data-slot=inline-edit-preview]]:-my-4'); expect(value.className).toContain('[&_[data-slot=inline-edit-control]]:-my-4'); diff --git a/packages/design-system/src/components/Attribute/AttributeValue.tsx b/packages/design-system/src/components/Attribute/AttributeValue.tsx index e62754d15..f16230214 100644 --- a/packages/design-system/src/components/Attribute/AttributeValue.tsx +++ b/packages/design-system/src/components/Attribute/AttributeValue.tsx @@ -45,20 +45,21 @@ export const AttributeValue: FC = ({ ref, children, classNa '[&_[data-slot=inline-edit-control]]:-my-4', // Same seam, horizontal axis: pulls the hover/pressed background and // hit target 7px further left than surrounding content, on the - // AttributeValue box itself rather than duplicated per guest. - // Applies whenever AttributeValue hosts either an InlineEdit or an - // AttributeActionsTarget — both render their own wide hover row and - // want it pulled flush with the label above. Vertical orientation - // only — in horizontal orientation AttributeValue sits right next to - // AttributeLabel with only a 4px gap, so a 7px pull would eat into - // the label's space. - !isHorizontal && 'has-[[data-slot=inline-edit]]:-ml-7', - !isHorizontal && 'has-[[data-slot=attribute-actions-target]]:-ml-7', + // AttributeValue box itself rather than duplicated per guest. Applies + // whenever AttributeValue hosts either an InlineEdit or an + // AttributeActionsTarget, in both orientations, so the guest's text + // lines up where a plain value would sit (in horizontal the 7px pull + // fits inside the 12px label gap set by Attribute). + 'has-[[data-slot=inline-edit]]:-ml-7', + 'has-[[data-slot=attribute-actions-target]]:-ml-7', // The -ml-7 above pulls the whole box left, including InlineEditError // — which isn't part of the wide hover row and has no reason to // follow it. Cancel the pull for that one guest so the error message // stays flush with the label instead of overhanging its left edge. - !isHorizontal && '[&_[data-slot=inline-edit-error]]:ml-7', + // (An error can only render inside an InlineEdit guest, so whenever + // this rule matches, the inline-edit pull above is active — in both + // orientations.) + '[&_[data-slot=inline-edit-error]]:ml-7', // Un-clip non-portaled editor dropdowns (horizontal `truncate` sets // overflow-hidden). :has() only matches while an InlineEdit is hosted. 'has-[[data-slot=inline-edit]]:overflow-visible', diff --git a/packages/design-system/src/components/Dialog/Dialog.e2e.ts b/packages/design-system/src/components/Dialog/Dialog.e2e.ts index 955ba93ff..296e9bcb7 100644 --- a/packages/design-system/src/components/Dialog/Dialog.e2e.ts +++ b/packages/design-system/src/components/Dialog/Dialog.e2e.ts @@ -15,6 +15,8 @@ const dialogStory = createStoryHelper('overlay-dialog', [ 'With Tabs', 'With Input At Edge', 'With Input At Edge Scrollable', + 'With Nested Select', + 'With Nested Drawer', ] as const); const getDialogContent = (page: Page) => page.getByTestId('dialog--content'); @@ -91,9 +93,103 @@ test.describe('Component: Dialog', () => { await focusViaKeyboard(page, input); await expect(page).toHaveScreenshot({ animations: 'disabled' }); }); + + test('Should render the select dropdown above a nested dialog correctly', async ({ page }) => { + await dialogStory.goto(page, 'With Nested Select'); + await page.getByRole('button', { name: 'Open dialog with nested select' }).click(); + await page.getByRole('button', { name: 'Open nested dialog' }).click(); + await page.getByTestId('nested-select--button').click(); + await expect(page.locator('[data-scope="select"][data-part="content"]')).toBeVisible(); + await expect(page).toHaveScreenshot({ animations: 'disabled' }); + }); }); test.describe('Interactions', () => { + test('Should not push the dialog back when a drawer opens inside it', async ({ page }) => { + // Regression: Drawer and Dialog share one Ark dialog machine, so zag's + // data-has-nested cannot tell them apart — a Drawer opened from a + // Dialog wrongly triggered the dialog's pushed-back animation. The + // pushed-back CSS keys on the DS-owned same-kind attribute now. + await dialogStory.goto(page, 'With Nested Drawer'); + await page.getByRole('button', { name: 'Open dialog with drawer inside' }).click(); + const dialogContent = page.locator('[data-scope="dialog"][data-part="content"]').first(); + await expect(dialogContent).toBeVisible(); + + await page.getByRole('button', { name: 'Open drawer' }).click(); + await expect(page.locator('[data-scope="dialog"][data-part="content"]')).toHaveCount(2); + + await expect(dialogContent).not.toHaveAttribute('data-has-nested-same'); + const pushedBack = await dialogContent.evaluate(el => { + const cs = getComputedStyle(el); + return { scale: cs.scale, translate: cs.translate }; + }); + expect(pushedBack).toEqual({ scale: 'none', translate: 'none' }); + }); + + test('Should stack the select dropdown above a nested dialog when opened inside it', async ({ + page, + }) => { + // Regression: the dropdown content's static z-50 mirrored into its + // positioner sat below the nested dialog positioner (50 + layer * 20), + // hiding the open menu. The content z-index is layer-aware now. + await dialogStory.goto(page, 'With Nested Select'); + await page.getByRole('button', { name: 'Open dialog with nested select' }).click(); + await page.getByRole('button', { name: 'Open nested dialog' }).click(); + await page.getByTestId('nested-select--button').click(); + + const content = page.locator('[data-scope="select"][data-part="content"]'); + await expect(content).toBeVisible(); + + // The topmost element at the dropdown's location must belong to the + // dropdown itself — not to the nested dialog covering it. + // Let the open animation (zoom-in/slide-in) finish before hit-testing — + // mid-animation the content box is still transforming and the probe + // point can land outside it. + await content.evaluate(el => Promise.all(el.getAnimations().map(a => a.finished))); + const isOnTop = await content.evaluate(el => { + const rect = el.getBoundingClientRect(); + const top = document.elementFromPoint( + rect.left + rect.width / 2, + rect.top + rect.height / 2, + ); + return !!top && el.contains(top); + }); + expect(isOnTop).toBe(true); + + // The option is actually interactive: selecting closes the dropdown. + await page.getByRole('option', { name: 'Vue' }).click(); + await expect(content).toBeHidden(); + }); + + test('Should stack the dropdown menu above a nested dialog when opened inside it', async ({ + page, + }) => { + await dialogStory.goto(page, 'With Nested Select'); + await page.getByRole('button', { name: 'Open dialog with nested select' }).click(); + await page.getByRole('button', { name: 'Open nested dialog' }).click(); + await page.getByRole('button', { name: 'Open menu' }).click(); + + const content = page.locator('[data-scope="menu"][data-part="content"]'); + await expect(content).toBeVisible(); + + // Let the open animation (zoom-in/slide-in) finish before hit-testing — + // mid-animation the content box is still transforming and the probe + // point can land outside it. + await content.evaluate(el => Promise.all(el.getAnimations().map(a => a.finished))); + const isOnTop = await content.evaluate(el => { + const rect = el.getBoundingClientRect(); + const top = document.elementFromPoint( + rect.left + rect.width / 2, + rect.top + rect.height / 2, + ); + return !!top && el.contains(top); + }); + expect(isOnTop).toBe(true); + + await page.getByRole('menuitem', { name: 'First action' }).click(); + await expect(content).toBeHidden(); + }); + test('Should open dialog when trigger button is clicked', async ({ page }) => { await dialogStory.goto(page, 'Basic'); await expect(getDialogContent(page)).toBeHidden(); diff --git a/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-footer-correctly-chromium.png b/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-footer-correctly-chromium.png index 76fb6cc32..dc0ff2fb0 100644 Binary files a/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-footer-correctly-chromium.png and b/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-footer-correctly-chromium.png differ diff --git a/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-footer-left-actions-correctly-chromium.png b/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-footer-left-actions-correctly-chromium.png index afb8d5f7c..02290a1ef 100644 Binary files a/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-footer-left-actions-correctly-chromium.png and b/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-footer-left-actions-correctly-chromium.png differ diff --git a/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-tabs-correctly-chromium.png b/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-tabs-correctly-chromium.png index 9c9ab095e..fda88d7df 100644 Binary files a/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-tabs-correctly-chromium.png and b/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-dialog-with-tabs-correctly-chromium.png differ diff --git a/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-the-select-dropdown-above-a-nested-dialog-correctly-chromium.png b/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-the-select-dropdown-above-a-nested-dialog-correctly-chromium.png new file mode 100644 index 000000000..be9b9c88b Binary files /dev/null and b/packages/design-system/src/components/Dialog/Dialog.e2e.ts-snapshots/Component-Dialog-Visual-Should-render-the-select-dropdown-above-a-nested-dialog-correctly-chromium.png differ diff --git a/packages/design-system/src/components/Dialog/Dialog.stories.tsx b/packages/design-system/src/components/Dialog/Dialog.stories.tsx index efc525b12..bbf6d24ae 100644 --- a/packages/design-system/src/components/Dialog/Dialog.stories.tsx +++ b/packages/design-system/src/components/Dialog/Dialog.stories.tsx @@ -1,8 +1,32 @@ import { useState } from 'react'; +import { createListCollection } from '@ark-ui/react/collection'; import type { Meta, StoryFn } from 'storybook-react-rsbuild'; import { PanelRight } from '../../icons'; import { Button } from '../Button'; +import { + Drawer, + DrawerBody, + DrawerContent, + DrawerHeader, + DrawerTitle, + DrawerTrigger, +} from '../Drawer'; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '../DropdownMenu'; import { Input } from '../Input'; +import { + Select, + SelectButton, + SelectContent, + SelectOption, + SelectOptionIndicator, + SelectOptionText, + SelectPositioner, +} from '../Select'; import { HStack, VStack } from '../Stack'; import { Switch, SwitchControl, SwitchLabel } from '../Switch'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '../Tabs'; @@ -371,6 +395,123 @@ export const WithInputAtEdgeScrollable: StoryFn = () => { ); }; +/** A Drawer opened from a Dialog must NOT push the dialog back — only a nested Dialog does */ +export const WithNestedDrawer: StoryFn = () => { + return ( + + + + + + + + Dialog stays in place + + + + + Opening the drawer below must not scale or shift this dialog. + + + + + + + + + Drawer over a dialog + + + + + + + + + + + + ); +}; + +/** Select and DropdownMenu opened inside a nested dialog must render above the nested dialog, not underneath it */ +export const WithNestedSelect: StoryFn = () => { + const collection = createListCollection({ + items: [ + { label: 'React', value: 'react' }, + { label: 'Vue', value: 'vue' }, + { label: 'Angular', value: 'angular' }, + ], + }); + + return ( + + + + + + + + [Level 1] Main Dialog + + + + + + + + + [Level 2] With Select + + + + + + + + + + + + First action + Second action + + + + + + + + + + + + + + ); +}; + /** Controlled dialog with external state */ export const Controlled: StoryFn = () => { const [open, setOpen] = useState(false); diff --git a/packages/design-system/src/components/Dialog/Dialog.tsx b/packages/design-system/src/components/Dialog/Dialog.tsx index 1b0397b80..5c67a9e6d 100644 --- a/packages/design-system/src/components/Dialog/Dialog.tsx +++ b/packages/design-system/src/components/Dialog/Dialog.tsx @@ -9,6 +9,6 @@ export const Dialog: FC = ({ minWidth = DIALOG_WIDTH_CONSTRAINTS.min, maxWidth = DIALOG_WIDTH_CONSTRAINTS.max, ...props -}) => ; +}) => ; Dialog.displayName = 'Dialog'; diff --git a/packages/design-system/src/components/Dialog/DialogContent.tsx b/packages/design-system/src/components/Dialog/DialogContent.tsx index c3ed5400a..cc00bdb8a 100644 --- a/packages/design-system/src/components/Dialog/DialogContent.tsx +++ b/packages/design-system/src/components/Dialog/DialogContent.tsx @@ -1,4 +1,4 @@ -import type { FC, Ref } from 'react'; +import type { CSSProperties, FC, Ref } from 'react'; import { Dialog as ArkUiDialog } from '@ark-ui/react/dialog'; import { cn } from '../../utils/cn'; import { useTestId } from '../../utils/testId'; @@ -8,6 +8,7 @@ import { DrawerPortal, drawerContentVariants, useDrawerContext, + useNestedSameKindCount, } from '../Drawer'; import { DialogPositioner } from './DialogPositioner'; @@ -16,6 +17,7 @@ type DialogContentProps = DrawerContentProps & { ref?: Ref }; export const DialogContent: FC = ({ children, ref }) => { const testId = useTestId('content'); const { width, overlay } = useDrawerContext(); + const nestedSameKindCount = useNestedSameKindCount(); return ( @@ -25,6 +27,10 @@ export const DialogContent: FC = ({ children, ref }) => { 0 ? '' : undefined} className={cn( drawerContentVariants({ isResizing: false }), 'flex flex-col min-h-0', @@ -32,13 +38,13 @@ export const DialogContent: FC = ({ children, ref }) => { 'origin-top', // Animations - 'data-[has-nested=dialog]:scale-[calc(var(--dialog-pushed-back-scale)-(var(--nested-layer-count)*var(--dialog-pushed-back-ratio)))]', - 'data-[has-nested=dialog]:-translate-y-[calc((var(--dialog-pushed-back-offset)/(var(--dialog-pushed-back-scale)-(var(--nested-layer-count)*var(--dialog-pushed-back-ratio))))*var(--nested-layer-count))]', + 'data-[has-nested-same]:scale-[calc(var(--dialog-pushed-back-scale)-(var(--nested-same-kind-count)*var(--dialog-pushed-back-ratio)))]', + 'data-[has-nested-same]:-translate-y-[calc((var(--dialog-pushed-back-offset)/(var(--dialog-pushed-back-scale)-(var(--nested-same-kind-count)*var(--dialog-pushed-back-ratio))))*var(--nested-same-kind-count))]', // Header '**:data-[slot=drawer-header]:pt-20 **:data-[slot=drawer-header]:pb-16 **:data-[slot=drawer-header]:px-24', )} - style={{ width }} + style={{ width, '--nested-same-kind-count': nestedSameKindCount } as CSSProperties} > {children} diff --git a/packages/design-system/src/components/Drawer/Drawer.e2e.ts b/packages/design-system/src/components/Drawer/Drawer.e2e.ts index e3707f285..94e657308 100644 --- a/packages/design-system/src/components/Drawer/Drawer.e2e.ts +++ b/packages/design-system/src/components/Drawer/Drawer.e2e.ts @@ -15,6 +15,8 @@ const drawerStory = createStoryHelper('overlay-drawer', [ 'With Nested', 'With Tabs', 'Resizable With Overflow List', + 'With Nested Select', + 'With Nested Dialog', ] as const); const getDrawerContent = (page: Page) => page.getByTestId('drawer--content'); @@ -74,9 +76,103 @@ test.describe('Component: Drawer', () => { await expect(page.locator('[data-scope="dialog"][data-part="content"]')).toBeVisible(); await expect(page).toHaveScreenshot({ animations: 'disabled' }); }); + + test('Should render the select dropdown above a nested drawer correctly', async ({ page }) => { + await drawerStory.goto(page, 'With Nested Select'); + await page.getByRole('button', { name: 'Open drawer with nested select' }).click(); + await page.getByRole('button', { name: 'Open nested drawer' }).click(); + await page.getByTestId('nested-select--button').click(); + await expect(page.locator('[data-scope="select"][data-part="content"]')).toBeVisible(); + await expect(page).toHaveScreenshot({ animations: 'disabled' }); + }); }); test.describe('Interactions', () => { + test('Should not push the drawer back when a dialog opens inside it', async ({ page }) => { + // Regression: Drawer and Dialog share one Ark dialog machine, so zag's + // data-has-nested cannot tell them apart — a Dialog opened from a + // Drawer wrongly triggered the drawer's pushed-back animation. The + // pushed-back CSS keys on the DS-owned same-kind attribute now. + await drawerStory.goto(page, 'With Nested Dialog'); + await page.getByRole('button', { name: 'Open drawer with dialog inside' }).click(); + const drawerContent = page.locator('[data-scope="dialog"][data-part="content"]').first(); + await expect(drawerContent).toBeVisible(); + + await page.getByRole('button', { name: 'Open dialog' }).click(); + await expect(page.locator('[data-scope="dialog"][data-part="content"]')).toHaveCount(2); + + await expect(drawerContent).not.toHaveAttribute('data-has-nested-same'); + const pushedBack = await drawerContent.evaluate(el => { + const cs = getComputedStyle(el); + return { scale: cs.scale, translate: cs.translate }; + }); + expect(pushedBack).toEqual({ scale: 'none', translate: 'none' }); + }); + + test('Should stack the select dropdown above a nested drawer when opened inside it', async ({ + page, + }) => { + // Regression: the dropdown content's static z-50 mirrored into its + // positioner sat below the nested drawer positioner (50 + layer * 20), + // hiding the open menu. The content z-index is layer-aware now. + await drawerStory.goto(page, 'With Nested Select'); + await page.getByRole('button', { name: 'Open drawer with nested select' }).click(); + await page.getByRole('button', { name: 'Open nested drawer' }).click(); + await page.getByTestId('nested-select--button').click(); + + const content = page.locator('[data-scope="select"][data-part="content"]'); + await expect(content).toBeVisible(); + + // The topmost element at the dropdown's location must belong to the + // dropdown itself — not to the nested drawer covering it. + // Let the open animation (zoom-in/slide-in) finish before hit-testing — + // mid-animation the content box is still transforming and the probe + // point can land outside it. + await content.evaluate(el => Promise.all(el.getAnimations().map(a => a.finished))); + const isOnTop = await content.evaluate(el => { + const rect = el.getBoundingClientRect(); + const top = document.elementFromPoint( + rect.left + rect.width / 2, + rect.top + rect.height / 2, + ); + return !!top && el.contains(top); + }); + expect(isOnTop).toBe(true); + + // The option is actually interactive: selecting closes the dropdown. + await page.getByRole('option', { name: 'Vue' }).click(); + await expect(content).toBeHidden(); + }); + + test('Should stack the dropdown menu above a nested drawer when opened inside it', async ({ + page, + }) => { + await drawerStory.goto(page, 'With Nested Select'); + await page.getByRole('button', { name: 'Open drawer with nested select' }).click(); + await page.getByRole('button', { name: 'Open nested drawer' }).click(); + await page.getByRole('button', { name: 'Open menu' }).click(); + + const content = page.locator('[data-scope="menu"][data-part="content"]'); + await expect(content).toBeVisible(); + + // Let the open animation (zoom-in/slide-in) finish before hit-testing — + // mid-animation the content box is still transforming and the probe + // point can land outside it. + await content.evaluate(el => Promise.all(el.getAnimations().map(a => a.finished))); + const isOnTop = await content.evaluate(el => { + const rect = el.getBoundingClientRect(); + const top = document.elementFromPoint( + rect.left + rect.width / 2, + rect.top + rect.height / 2, + ); + return !!top && el.contains(top); + }); + expect(isOnTop).toBe(true); + + await page.getByRole('menuitem', { name: 'First action' }).click(); + await expect(content).toBeHidden(); + }); + test('Should open drawer when trigger button is clicked', async ({ page }) => { await drawerStory.goto(page, 'Basic'); await expect(getDrawerContent(page)).toBeHidden(); diff --git a/packages/design-system/src/components/Drawer/Drawer.e2e.ts-snapshots/Component-Drawer-Visual-Should-render-the-select-dropdown-above-a-nested-drawer-correctly-chromium.png b/packages/design-system/src/components/Drawer/Drawer.e2e.ts-snapshots/Component-Drawer-Visual-Should-render-the-select-dropdown-above-a-nested-drawer-correctly-chromium.png new file mode 100644 index 000000000..2c50b15c4 Binary files /dev/null and b/packages/design-system/src/components/Drawer/Drawer.e2e.ts-snapshots/Component-Drawer-Visual-Should-render-the-select-dropdown-above-a-nested-drawer-correctly-chromium.png differ diff --git a/packages/design-system/src/components/Drawer/Drawer.stories.tsx b/packages/design-system/src/components/Drawer/Drawer.stories.tsx index 1ac8f4c00..aa68b5aae 100644 --- a/packages/design-system/src/components/Drawer/Drawer.stories.tsx +++ b/packages/design-system/src/components/Drawer/Drawer.stories.tsx @@ -1,10 +1,34 @@ import { useState } from 'react'; +import { createListCollection } from '@ark-ui/react/collection'; import type { Meta, StoryFn } from 'storybook-react-rsbuild'; import { PanelRight } from '../../icons'; import { Attribute, AttributeLabel, AttributeValue } from '../Attribute'; import { Button } from '../Button'; +import { + Dialog, + DialogBody, + DialogContent, + DialogHeader, + DialogTitle, + DialogTrigger, +} from '../Dialog'; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '../DropdownMenu'; import { OverflowList } from '../OverflowList'; import { Popover, PopoverContent, PopoverTrigger } from '../Popover'; +import { + Select, + SelectButton, + SelectContent, + SelectOption, + SelectOptionIndicator, + SelectOptionText, + SelectPositioner, +} from '../Select'; import { HStack, VStack } from '../Stack'; import { Switch, SwitchControl, SwitchLabel } from '../Switch'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '../Tabs'; @@ -621,6 +645,122 @@ export const WithNested: StoryFn = () => { ); }; +/** A Dialog opened from a Drawer must NOT push the drawer back — only a nested Drawer does */ +export const WithNestedDialog: StoryFn = () => { + return ( + + + + + + + + Drawer stays in place + + + + + Opening the dialog below must not scale or shift this drawer. + + + + + + + + + Dialog over a drawer + + + + + + + + + + + + ); +}; + +/** Select and DropdownMenu opened inside a nested drawer must render above the nested drawer, not underneath it */ +export const WithNestedSelect: StoryFn = () => { + const collection = createListCollection({ + items: [ + { label: 'React', value: 'react' }, + { label: 'Vue', value: 'vue' }, + { label: 'Angular', value: 'angular' }, + ], + }); + + return ( + + + + + + + + [Level 1] Main Drawer + + + + + + + + + [Level 2] With Select + + + + + + + + + + + + First action + Second action + + + + + + + + + + + + + + ); +}; + /** With tabs */ export const WithTabs: StoryFn = () => { const [activeTab, setActiveTab] = useState('tab1'); diff --git a/packages/design-system/src/components/Drawer/Drawer.tsx b/packages/design-system/src/components/Drawer/Drawer.tsx index d5ee66023..f8806ff5f 100644 --- a/packages/design-system/src/components/Drawer/Drawer.tsx +++ b/packages/design-system/src/components/Drawer/Drawer.tsx @@ -2,10 +2,18 @@ import type { FC, ReactNode } from 'react'; import type { DialogInteractOutsideEvent as DrawerInteractOutsideEvent } from '@ark-ui/react/dialog'; import { type TestableProps, TestIdProvider } from '../../utils/testId'; import { DrawerProvider } from './DrawerContext'; +import type { DrawerKind } from './DrawerNestingContext'; import { DrawerRoot } from './DrawerRoot'; export interface DrawerProps extends TestableProps { children: ReactNode; + /** + * Which overlay family this root belongs to. Drives the same-kind nesting + * counter behind the pushed-back animation: a Drawer only pushes back for + * a nested Drawer, a Dialog only for a nested Dialog. Set to 'dialog' by + * the Dialog component — consumers should not need to touch this. + */ + kind?: DrawerKind; /** Controlled open state */ open?: boolean; /** Controlled open change handler */ @@ -32,6 +40,7 @@ export interface DrawerProps extends TestableProps { export const Drawer: FC = ({ children, + kind = 'drawer', open, onOpenChange, closeOnEscape = true, @@ -46,6 +55,7 @@ export const Drawer: FC = ({ 'data-testid': testId, }) => ( = ({ children, asChild, ref }) => { const testId = useTestId('content'); const { width, isResizing, overlay } = useDrawerContext(); + const nestedSameKindCount = useNestedSameKindCount(); return ( @@ -26,16 +28,20 @@ export const DrawerContent: FC = ({ children, asChild, ref } 0 ? '' : undefined} className={cn( drawerContentVariants({ isResizing }), 'h-full', 'slide-in-from-right-[25%] slide-out-to-right-[25%]', 'origin-left', - 'data-[has-nested=dialog]:scale-[calc(var(--drawer-pushed-back-scale)-(var(--nested-layer-count)*var(--drawer-pushed-back-ratio)))]', - 'data-[has-nested=dialog]:-translate-x-[calc((var(--drawer-pushed-back-offset)/(var(--drawer-pushed-back-scale)-(var(--nested-layer-count)*var(--drawer-pushed-back-ratio))))*var(--nested-layer-count))]', + 'data-[has-nested-same]:scale-[calc(var(--drawer-pushed-back-scale)-(var(--nested-same-kind-count)*var(--drawer-pushed-back-ratio)))]', + 'data-[has-nested-same]:-translate-x-[calc((var(--drawer-pushed-back-offset)/(var(--drawer-pushed-back-scale)-(var(--nested-same-kind-count)*var(--drawer-pushed-back-ratio))))*var(--nested-same-kind-count))]', )} - style={{ width }} + style={{ width, '--nested-same-kind-count': nestedSameKindCount } as CSSProperties} asChild={asChild} > {children} diff --git a/packages/design-system/src/components/Drawer/DrawerContext/DrawerProvider.tsx b/packages/design-system/src/components/Drawer/DrawerContext/DrawerProvider.tsx index 0c8779ba9..c3dd34b66 100644 --- a/packages/design-system/src/components/Drawer/DrawerContext/DrawerProvider.tsx +++ b/packages/design-system/src/components/Drawer/DrawerContext/DrawerProvider.tsx @@ -1,11 +1,14 @@ import { type FC, type ReactNode, useState } from 'react'; import { useControlled } from '../../../hooks'; import { DRAWER_SIZES, DRAWER_WIDTH_CONSTRAINTS } from '../constants'; +import { type DrawerKind, DrawerNestingProvider } from '../DrawerNestingContext'; import { DrawerContext } from './DrawerContext'; import type { DrawerContextValue } from './types'; interface DrawerContextProviderProps { children: ReactNode; + /** Which overlay family this root belongs to — drives same-kind nesting */ + kind: DrawerKind; open?: boolean; onOpenChange?: (open: boolean) => void; /** Whether ESC key closes the drawer (default: true) */ @@ -24,6 +27,7 @@ interface DrawerContextProviderProps { export const DrawerProvider: FC = ({ children, + kind, open, onOpenChange, closeOnEscape, @@ -64,5 +68,11 @@ export const DrawerProvider: FC = ({ maxWidth, }; - return {children}; + return ( + + + {children} + + + ); }; diff --git a/packages/design-system/src/components/Drawer/DrawerNestingContext.tsx b/packages/design-system/src/components/Drawer/DrawerNestingContext.tsx new file mode 100644 index 000000000..0084e30bc --- /dev/null +++ b/packages/design-system/src/components/Drawer/DrawerNestingContext.tsx @@ -0,0 +1,88 @@ +import { + createContext, + type FC, + type ReactNode, + useCallback, + useContext, + useEffect, + useMemo, + useState, +} from 'react'; + +export type DrawerKind = 'drawer' | 'dialog'; + +/** + * DS-level nesting tracker for the Drawer/Dialog family. + * + * Both components run on the same Ark/zag dialog machine, so zag's own + * `data-has-nested` / `--nested-layer-count` metadata cannot tell a nested + * Drawer from a nested Dialog — a Drawer opened from a Dialog wrongly + * triggered the Dialog's pushed-back animation (and vice versa). This + * context re-counts nesting per `kind`, and the pushed-back CSS keys on the + * DS-owned attribute instead of zag's. + * + * The registry (registration side) and the counter (render side) live in + * separate contexts on purpose: the registry value must stay referentially + * stable while counts change, otherwise a child's registration effect would + * re-run on every count update and loop. + */ +interface DrawerNestingRegistry { + kind: DrawerKind; + /** + * Registers an open descendant overlay of the given kind and forwards it + * up the ancestor chain, so every same-kind ancestor counts it (matching + * zag's transitive `countNestedLayersOfType` semantics, filtered by kind). + * Returns the unregister cleanup. + */ + registerNested: (kind: DrawerKind) => () => void; +} + +const DrawerNestingRegistryContext = createContext(null); +const NestedSameKindCountContext = createContext(0); + +interface DrawerNestingProviderProps { + kind: DrawerKind; + /** The overlay's open state — the ancestor registration lasts while open */ + open: boolean; + children: ReactNode; +} + +export const DrawerNestingProvider: FC = ({ kind, open, children }) => { + const parent = useContext(DrawerNestingRegistryContext); + const [nestedSameKindCount, setNestedSameKindCount] = useState(0); + + const registerNested = useCallback( + (childKind: DrawerKind) => { + const unregisterFromParent = parent?.registerNested(childKind); + const matches = childKind === kind; + if (matches) setNestedSameKindCount(count => count + 1); + + return () => { + if (matches) setNestedSameKindCount(count => count - 1); + unregisterFromParent?.(); + }; + }, + [parent, kind], + ); + + // Announce this overlay to the ancestor chain while it is open. + useEffect(() => { + if (!open || !parent) return; + return parent.registerNested(kind); + }, [open, parent, kind]); + + const registry = useMemo(() => ({ kind, registerNested }), [kind, registerNested]); + + return ( + + + {children} + + + ); +}; + +DrawerNestingProvider.displayName = 'DrawerNestingProvider'; + +/** Count of currently open same-kind overlays nested under this one. */ +export const useNestedSameKindCount = () => useContext(NestedSameKindCountContext); diff --git a/packages/design-system/src/components/Drawer/classes.ts b/packages/design-system/src/components/Drawer/classes.ts index a29652dec..63ae71424 100644 --- a/packages/design-system/src/components/Drawer/classes.ts +++ b/packages/design-system/src/components/Drawer/classes.ts @@ -14,9 +14,9 @@ export const drawerContentVariants = cva( // Scroll Area paddings '**:data-[slot=drawer-scroll-area-content]:pb-24', - 'has-data-[slot=drawer-footer]:**:data-[slot=drawer-scroll-area-content]:pb-4', + 'has-data-[slot=drawer-footer]:**:data-[slot=drawer-scroll-area-content]:pb-8', // Content top padding for prevent outline/borders cutting against the scroll viewport's edge - '**:data-[slot=drawer-scroll-area-content]:pt-4', + '**:data-[slot=drawer-scroll-area-content]:pt-8', // Scroll Area borders (applied to drawer-body based on scroll position) // Border shows when content is hidden in that direction diff --git a/packages/design-system/src/components/Drawer/index.ts b/packages/design-system/src/components/Drawer/index.ts index 9c2a57d98..28f56caae 100644 --- a/packages/design-system/src/components/Drawer/index.ts +++ b/packages/design-system/src/components/Drawer/index.ts @@ -11,6 +11,7 @@ export { type DrawerFooterControlsProps, } from './DrawerFooterControls'; export { DrawerHeader, type DrawerHeaderProps } from './DrawerHeader'; +export { type DrawerKind, useNestedSameKindCount } from './DrawerNestingContext'; export { DrawerOverlay } from './DrawerOverlay'; export { DrawerPortal } from './DrawerPortal'; export { diff --git a/packages/design-system/src/components/DropdownMenu/classes.ts b/packages/design-system/src/components/DropdownMenu/classes.ts index 04f39a063..769e1ab9f 100644 --- a/packages/design-system/src/components/DropdownMenu/classes.ts +++ b/packages/design-system/src/components/DropdownMenu/classes.ts @@ -4,8 +4,17 @@ import { cn } from '../../utils/cn'; export const dropdownMenuClassNames = cn( // Dimensions 'flex flex-col gap-1 min-w-128', - // Leveling and scrolling - 'z-50 overflow-y-auto overflow-x-hidden outline-none', + // Leveling: the content's computed z-index is what zag's popper mirrors + // into the positioner's --z-index, so the layer-aware value must live + // here, not on the positioner. Zag's dismissable layer stack sets + // --layer-index inline on this node when the menu opens (menus, selects, + // and dialogs share one global stack), so a menu opened inside a nested + // drawer/dialog lands above that dialog's positioner + // (50 + layer-index * 20). The ,0 fallback keeps the calc valid while + // the node is closed / not yet registered in the stack. + 'z-[calc(var(--drawer-positioner-z-index)+(var(--layer-index,0)*var(--drawer-level-ratio)))]', + // Scrolling + 'overflow-y-auto overflow-x-hidden outline-none', // Visual 'rounded-12 border border-border-primary-light bg-bg-surface-2 p-8 font-sans text-text-primary shadow-md outline-none', // Animation opened diff --git a/packages/design-system/src/components/InlineEdit/InlineEdit.e2e.ts-snapshots/Component-InlineEdit-Visual-Should-render-the-commit-confirmation-dialog-correctly-chromium.png b/packages/design-system/src/components/InlineEdit/InlineEdit.e2e.ts-snapshots/Component-InlineEdit-Visual-Should-render-the-commit-confirmation-dialog-correctly-chromium.png index 24b2288b8..eda3bd41d 100644 Binary files a/packages/design-system/src/components/InlineEdit/InlineEdit.e2e.ts-snapshots/Component-InlineEdit-Visual-Should-render-the-commit-confirmation-dialog-correctly-chromium.png and b/packages/design-system/src/components/InlineEdit/InlineEdit.e2e.ts-snapshots/Component-InlineEdit-Visual-Should-render-the-commit-confirmation-dialog-correctly-chromium.png differ diff --git a/packages/design-system/src/components/Page/PageHeader.tsx b/packages/design-system/src/components/Page/PageHeader.tsx index 38e2ad29b..c16fce3a4 100644 --- a/packages/design-system/src/components/Page/PageHeader.tsx +++ b/packages/design-system/src/components/Page/PageHeader.tsx @@ -17,7 +17,7 @@ export const PageHeader: FC = ({ ref, sticky, children, classNa data-testid={testId} data-slot='page-header' className={cn( - 'flex items-center px-16 pb-8 gap-12', + 'flex items-center px-16 py-8 gap-12', sticky && 'sticky top-0 z-10 bg-bg-surface-2', className, )} diff --git a/packages/design-system/src/components/Popover/PopoverContent.tsx b/packages/design-system/src/components/Popover/PopoverContent.tsx index 8381bd76c..a019bac18 100644 --- a/packages/design-system/src/components/Popover/PopoverContent.tsx +++ b/packages/design-system/src/components/Popover/PopoverContent.tsx @@ -68,7 +68,14 @@ export const PopoverContent: FC = ({ 'min-h-(--popover-min-height)', 'max-h-[clamp(var(--popover-min-height),var(--available-height),var(--popover-max-height))]', // Visual - 'p-12 bg-bg-surface-2 rounded-12 border border-border-primary-light shadow-md text-text-primary z-50', + 'p-12 bg-bg-surface-2 rounded-12 border border-border-primary-light shadow-md text-text-primary', + // Leveling: layer-aware so a popover opened inside a nested + // drawer/dialog stacks above that dialog's positioner — zag sets + // --layer-index inline on this node when the popover opens, and + // popper mirrors the computed z-index into the positioner's + // --z-index (same mechanism as DropdownMenu/Select, see + // DropdownMenu/classes.ts). + 'z-[calc(var(--drawer-positioner-z-index)+(var(--layer-index,0)*var(--drawer-level-ratio)))]', // Contain wheel chaining on the inner ScrollArea viewport so wheel // events past the scroll boundary don't propagate to whatever // scrollable lives below — popovers are portaled and frequently