fix(components): prevent overlay subtitle slot from leaking into nested Alert#1939
fix(components): prevent overlay subtitle slot from leaking into nested Alert#1939ShreyasGit51283 wants to merge 6 commits into
Conversation
…ed Alert When an Alert is nested inside a Dialog/Modal, the overlay's `subtitle` slot context captured the Alert's Text: it forced `slot="subtitle"`, rendered the text as the Dialog's `h3` description, and applied the overlay's subtitle CSS (wrong color/size/position). Alert now establishes its own TextContext (default + subtitle slots), and the Modal subtitle rule is scoped to skip alert-nested text. Adds regression tests and a nested-overlay story. DSYS-157 Co-authored-by: Cursor <cursoragent@cursor.com>
🦋 Changeset detectedLatest commit: 30fc2cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
| } | ||
|
|
||
| & [slot='subtitle'] { | ||
| /* Scope to the overlay's own subtitle. The `:not()` keeps this from reaching into a |
There was a problem hiding this comment.
mentioned this during the meeting, but let's undo this change for now
There was a problem hiding this comment.
I see where you're coming from @nhironaka. I refactored the selectors (commit) to make it explicit that we support slot="header" as a direct child of Modal as well as nested in Dialog, but I made a ticket to deprecate the former, as I see a few usages in Gonfalon
…wn slots Replace the global `[slot='subtitle']:not([role='alert'] *)` selector with dialog-scoped selectors (`[role='dialog'] > [slot='subtitle']` and `[slot='header'] > [slot='subtitle']`), declared once and shared via a single selector list. This targets the overlay's own subtitle in both the header and headerless layouts without the `:not()` guard, and cannot reach a nested Alert that reuses the `subtitle` slot.
Remove the nested-overlay regression story (and its now-unused imports) from Alert.stories, and add a `WithHeader` story to Modal.stories so the Dialog/Modal header layout is demonstrated alongside the Modal component. The header lives inside the Dialog render prop so the `subtitle` slot context and the dialog's close handler resolve correctly.
| * text. The Dialog's own subtitle keeps the overlay styling, while both nested Alerts | ||
| * render with the Alert's body-text styling — identical to how they look outside a modal. | ||
| */ | ||
| export const NestedInDialog: Story = { |
There was a problem hiding this comment.
I moved this to the Modal stories as a "With header" story because I realized we haven't documented the header slot at all. Also removed the ticket from the story name so we don't conflate LP with internal data
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 74b91da. Configure here.
3 participants
Summary
Fixes DSYS-157. When an
Alertis nested inside an overlay (Dialog/Modal), the overlay'ssubtitleslot context leaked into the Alert'sText:slot="subtitle"on the Alert'sText(a bare<Text>threw"A slot prop is required...").id+elementType: 'h3', so the Alert body became the Dialog'saria-describedbydescription rendered as an<h3>(an accessibility bug).[slot="subtitle"]CSS (wrong color/size/position).Root cause
Dialogprovides a RACTextContextwithslots: { subtitle: { id, elementType: 'h3' } }.Alertonly overrodeHeadingContext/ButtonGroupContext, so its descendants inherited the Dialog'sTextContext.Modal.module.cssstyled[slot="subtitle"]via a descendant selector, reaching into nested alerts.Changes
Alert.tsx— establish Alert's ownTextContext(default slot +subtitleslot). Insulates Alert content from ancestor overlays, fixes the a11y leak, and allows a bare<Text>inside an Alert. Backward-compatible: existing<Text slot="subtitle">usage still works.Modal.module.css— scope the subtitle rule with:not([role='alert'] *)so it no longer reaches into nested alerts. The overlay's own subtitle is unaffected.Alert.spec.tsxadds nested-in-Dialog regression coverage (verified to fail without the fix with the exact"A slot prop is required"error).Alert.stories.tsxadds "Nested in Dialog (DSYS-157)" for visual + Chromatic regression coverage.Test plan
Components/Status/Alert-> "Nested in Dialog (DSYS-157)": both nested alerts render with Alert text styling; the Dialog's own subtitle keeps overlay styling.pnpm --filter @launchpad-ui/components test).<Text>inside an Alert (in and out of a Dialog) renders without throwing.Made with Cursor