Skip to content
Open
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
5 changes: 5 additions & 0 deletions .changeset/alert-nested-overlay-slot-leak.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@launchpad-ui/components": patch
---

Fix `Alert` text styling and semantics leaking when nested inside an overlay (`Dialog`/`Modal`). `Alert` now establishes its own `TextContext`, so an ancestor overlay's `subtitle` slot no longer captures the Alert's `Text` (which previously forced `slot="subtitle"`, inherited the overlay's `id`/`h3` element, and picked up the overlay's subtitle CSS). The `Modal` subtitle rule is also scoped to skip alert-nested text. A bare `<Text>` now works inside an `Alert`, including within a `Dialog`.
41 changes: 40 additions & 1 deletion packages/components/__tests__/Alert.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { describe, expect, it, vi } from 'vitest';

import { render, screen, userEvent } from '../../../test/utils';
import { Alert, Heading, Text } from '../src';
import { Alert, Dialog, Heading, Text } from '../src';

describe('Alert', () => {
it('renders', () => {
Expand All @@ -28,4 +28,43 @@ describe('Alert', () => {
expect(spy).toHaveBeenCalledTimes(1);
expect(screen.queryByRole('alert')).toBeNull();
});

// Regression coverage for DSYS-157: a Dialog publishes a `subtitle` slot context
// (with its own id/elementType/CSS). Alert must establish its own Text context so a
// nested <Text> isn't captured by the Dialog's subtitle slot.
describe('nested inside a Dialog (DSYS-157)', () => {
it('does not let the Dialog subtitle context leak into the Alert', () => {
render(
<Dialog>
Dialog body
<Alert status="warning">
<Heading>Alert heading</Heading>
<Text slot="subtitle">Alert body text</Text>
</Alert>
</Dialog>,
);

// The Dialog must not adopt the Alert's text as its accessible description.
expect(screen.queryByRole('dialog', { description: 'Alert body text' })).toBeNull();

// The Alert's text must not be promoted to the Dialog's subtitle element (h3).
expect(screen.getByText('Alert body text').tagName).not.toBe('H3');
});

it('allows a bare <Text> inside a nested Alert without a forced slot', () => {
expect(() =>
render(
<Dialog>
Dialog body
<Alert status="warning">
<Heading>Alert heading</Heading>
<Text>Bare alert text</Text>
</Alert>
</Dialog>,
),
).not.toThrow();

expect(screen.getByText('Bare alert text')).toBeVisible();
});
});
});
12 changes: 11 additions & 1 deletion packages/components/src/Alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { StatusIcon } from '@launchpad-ui/icons';
import { useControlledState } from '@react-stately/utils';
import { cva } from 'class-variance-authority';
import { HeadingContext } from 'react-aria-components/Heading';
import { Provider } from 'react-aria-components/slots';
import { DEFAULT_SLOT, Provider } from 'react-aria-components/slots';
import { TextContext } from 'react-aria-components/Text';

import { ButtonGroupContext } from './ButtonGroup';
import { IconButton } from './IconButton';
Expand Down Expand Up @@ -99,6 +100,15 @@ const Alert = ({
values={[
[HeadingContext, { className: styles.heading }],
[ButtonGroupContext, { className: styles.buttonGroup }],
[
TextContext,
{
slots: {
[DEFAULT_SLOT]: {},
subtitle: {},
},
},
],
]}
>
{children}
Expand Down
19 changes: 12 additions & 7 deletions packages/components/src/styles/Modal.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,18 @@
height: 100%;
}

& [role='dialog'] > [slot='subtitle'],
& [slot='header'] > [slot='subtitle'] {
font: var(--lp-text-body-1-regular);
color: var(--lp-color-text-ui-secondary);
grid-row: 2;
grid-column: 1 / span 2;
}

> [slot='header'] {
margin-bottom: var(--lp-size-20);
}

& [slot='header'] {
display: grid;
grid-template-columns: 1fr;
Expand All @@ -83,13 +95,6 @@
align-items: center;
gap: var(--lp-size-10);
}

& [slot='subtitle'] {
font: var(--lp-text-body-1-regular);
color: var(--lp-color-text-ui-secondary);
grid-row: 2;
grid-column: 1 / span 2;
}
}

.defaultSmall {
Expand Down
63 changes: 63 additions & 0 deletions packages/components/stories/Modal.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { Meta, ReactRenderer, StoryObj } from '@storybook/react-vite';
import type { ComponentType } from 'react';
import type { PlayFunction } from 'storybook/internal/types';

import { Alert } from '@launchpad-ui/components';
import { expect, userEvent, waitFor, within } from 'storybook/test';

import { allModes } from '../../../.storybook/modes';
Expand Down Expand Up @@ -112,6 +113,60 @@ export const Drawer: Story = {
},
};

/**
* A Modal with a slotted `header`. The `header` groups the `title`, an optional
* `subtitle`, and the close button into the overlay's header grid, separate from the
* `body` and `footer`.
*/
export const WithHeader: Story = {
render: (args) => (
<DialogTrigger>
<Button>Trigger</Button>
<ModalOverlay>
<Modal {...args}>
<Dialog>
{({ close }) => (
<>
<div slot="header">
<Heading slot="title">Invite teammates</Heading>
<IconButton
aria-label="close"
icon="cancel"
size="small"
variant="minimal"
onPress={close}
/>
<Text slot="subtitle">They'll get an email with a link to join.</Text>
<Alert status="warning">
<Heading>Heads up before archiving</Heading>
<Text slot="subtitle">
Scenario alert body. Inside the dialog this must use the Alert's text styling,
not the Dialog's subtitle styling.
</Text>
</Alert>
</div>
<div slot="body">Body text</div>
<div slot="footer">
<Button slot="close">Cancel</Button>
<Button variant="primary">Send invites</Button>
</div>
</>
)}
</Dialog>
</Modal>
</ModalOverlay>
</DialogTrigger>
),
play,
parameters: {
chromatic: {
modes: {
mobile: allModes.mobile,
},
},
},
};

/**
* Bug reproduction: Dialog closes when clicking a button inside it while Toast is active.
*
Expand Down Expand Up @@ -146,6 +201,14 @@ export const DialogWithActiveToast: Story = {
onPress={close}
/>
<Text slot="subtitle">Try clicking the button below with toast active</Text>
<Alert status="info">
<Heading>No critical environments to check</Heading>
{/* Bare <Text> now works inside an Alert, even within a Dialog. */}
<Text>
No checks were run because you don't have any critical environments set
up.
</Text>
</Alert>
</div>
<div slot="body">
<p>
Expand Down
Loading