Skip to content

Add modal launcher details to docs #2875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcysutton merged 5 commits into from
Dec 4, 2025
Merged

Add modal launcher details to docs #2875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ee1b57b
[modal-docs] Add modal details to docs
marcysutton Nov 25, 2025
1dbac8a
[modal-docs] docs(changeset):
marcysutton Nov 25, 2025
684802c
[modal-docs] Update docs comment
marcysutton Dec 1, 2025
657d698
[modal-docs] Fix a11y issues
marcysutton Dec 1, 2025
9d29c92
[modal-docs] Update modal stories for a11y
marcysutton Dec 2, 2025
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/grumpy-dingos-confess.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
4 changes: 4 additions & 0 deletions __docs__/wonder-blocks-modal/_overview_.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ A specialized dialog for drawer interfaces that slides in from the side. Built o

## Launcher Components

Modal dialogs are typically rendered using WB launcher components that handle modal dialog lifecycles, focus management, and user interactions.

For conditionally rendering modals, ensure there is only one `ModalLauncher` or `DrawerLauncher` in your component tree. A launcher needs to stay mounted on the current page to properly handle the user's keyboard focus on close of modals. Read [more details on Confluence](https://khanacademy.atlassian.net/wiki/spaces/FRONTEND/blog/2025/11/24/4454383789/Wonder+Blocks+Modal+Tips+Tricks).

### ModalLauncher

The primary component for launching modals. Handles backdrop clicks, focus management, keyboard navigation, and modal lifecycle.
Expand Down
5 changes: 5 additions & 0 deletions __docs__/wonder-blocks-modal/drawer-launcher.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ It can align a dialog on the \`inlineStart\` (left), \`inlineEnd\` (right), or
**IMPORTANT**: This component should only be used with \`DrawerDialog\`. Using it with other
dialog components may result in incorrect animations, positioning, and styling.
For conditionally rendering modals, ensure there is only one \`DrawerLauncher\` in
your component tree. A launcher needs to stay mounted on the current page to
properly handle the user's keyboard focus on close of modals.
Read [more details on Confluence](https://khanacademy.atlassian.net/wiki/spaces/FRONTEND/blog/2025/11/24/4454383789/Wonder+Blocks+Modal+Tips+Tricks).
See available styling customizations in \`DrawerDialog\` docs.
### Usage
Expand Down
18 changes: 14 additions & 4 deletions __docs__/wonder-blocks-modal/flexible-dialog.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,19 +235,19 @@ export const WithAriaLabel: StoryComponentType = {

/**
*
* A FlexibleDialog can have an aria-label as its accessible name.
* A FlexibleDialog can derive its accessible name from aria-labelledby.
*/
export const WithAriaLabelledby: StoryComponentType = {
render: () => (
<View style={styles.previewSizer}>
<View style={styles.modalPositioner}>
<FlexibleDialog
aria-labelledby="main-heading"
title={
Copy link
Member

@beaesguerra beaesguerra Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean the heading should always be in the title prop? Are there use cases that consumers would need where it is part of the content?

I was assuming manually configuring aria-labelledby meant a custom implementation of the title was needed, since using the title prop will auto-wire up the attributes already!

wonder-blocks/packages/wonder-blocks-modal/src/components/flexible-dialog.tsx

Lines 143 to 152 in 7065e96

const renderedTitle =
title == null ? null : typeof title === "string" ? (
<Heading id={headingId}>{title}</Heading>
) : (
// Augment heading element with ID/testId
React.cloneElement(title, {
id: headingId,
testId: "title-heading-wrapper",
})
);

I also noticed that the aria-labelledby prop provided by the consumer doesn't end up getting used (other aria attributes are applied though!). This might be what was causing an issue with the original example?

wonder-blocks/packages/wonder-blocks-modal/src/components/flexible-dialog.tsx

Lines 159 to 161 in 7065e96

aria-label={accessibilityProps["aria-label"]}
aria-labelledby={headingId}
aria-describedby={accessibilityProps["aria-describedby"]}

Copy link
Member Author

@marcysutton marcysutton Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The component is intended to use the title prop so naming can be handled automatically, with the option for it to be inserted into content with the title render prop. But the story originally included an aria-labelledby example that didn't work properly, and it was a little confusing outside of the happy path. I opened a separate ticket to revisit naming with aria-labelledby, since it's a little more involved than fixing up some stories: https://khanacademy.atlassian.net/browse/WB-2168

<Heading id="main-heading">Dogz are the best</Heading>
}
content={
<View>
<Heading id="main-heading">
Dogz are the best
</Heading>
<BodyText>This is some text</BodyText>
</View>
}
Expand Down Expand Up @@ -310,6 +310,16 @@ export const WithLongContents: StoryComponentType = {
<BodyText>{reallyLongText}</BodyText>
<BodyText>{reallyLongText}</BodyText>
<BodyText>{reallyLongText}</BodyText>
<BodyText style={{display: "flex"}}>
<Button
style={{
marginInlineStart: "auto",
marginBlockStart: sizing.size_100,
}}
>
A button
</Button>
</BodyText>
</>
}
/>
Expand Down
26 changes: 25 additions & 1 deletion __docs__/wonder-blocks-modal/modal-launcher.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,31 @@ export default {
),
docs: {
description: {
component: null,
component: `A component that enables you to launch a modal, covering the screen.

Intended for use with \`OnePaneDialog\`, \`FlexibleDialog\`, or modal Building Blocks.

For conditionally rendering modals, ensure there is only one \`ModalLauncher\` in
your component tree. A launcher needs to stay mounted on the current page to
properly handle the user's keyboard focus on close of modals.
Read [more details on Confluence](https://khanacademy.atlassian.net/wiki/spaces/FRONTEND/blog/2025/11/24/4454383789/Wonder+Blocks+Modal+Tips+Tricks).

### Usage

\`\`\`jsx
import {ModalLauncher} from "@khanacademy/wonder-blocks-modal";
import {FlexibleDialog} from "@khanacademy/wonder-blocks-modal";
import {BodyText} from "@khanacademy/wonder-blocks-typography";

<ModalLauncher
onClose={handleClose}
opened={opened}
animated={animated}
modal={({closeModal}) => (
<FlexibleDialog />
)}
/>
\`\`\``,
},
source: {
// See https://github.com/storybookjs/storybook/issues/12596
Expand Down
39 changes: 35 additions & 4 deletions __docs__/wonder-blocks-modal/modal-panel.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import packageConfig from "../../packages/wonder-blocks-modal/package.json";
import ComponentInfo from "../components/component-info";
import modalPanelArgtypes from "./modal-panel.argtypes";
import {allModes} from "../../.storybook/modes";
import {focusStyles} from "@khanacademy/wonder-blocks-styles";

const longBody = (
<View style={{gap: sizing.size_160}}>
Expand Down Expand Up @@ -135,7 +136,10 @@ export const Default: StoryComponentType = {
<ModalPanel
{...args}
content={
<View style={styles.content}>
<View
style={[styles.content, styles.scrollContainer]}
tabIndex={0}
>
<Heading size="xxlarge" id="modal-title-0">
Modal Title
</Heading>
Expand Down Expand Up @@ -165,7 +169,11 @@ export const WithHeader: StoryComponentType = {
header={
<ModalHeader titleId="modal-title-2" title="Modal Title" />
}
content={longBody}
content={
<View tabIndex={0} style={styles.scrollContainer}>
{longBody}
</View>
}
/>
</ModalDialog>
),
Expand All @@ -186,7 +194,10 @@ export const WithFooter: StoryComponentType = {
<ModalDialog aria-labelledby="modal-title-3" style={styles.dialog}>
<ModalPanel
content={
<View style={styles.content}>
<View
style={[styles.content, styles.scrollContainer]}
tabIndex={0}
>
<Heading size="xxlarge" id="modal-title-3">
Modal Title
</Heading>
Expand Down Expand Up @@ -303,6 +314,18 @@ export const WithStyle: StoryComponentType = {
borderRadius: 20,
} as const;

const button = (
<BodyText style={{display: "flex"}}>
<Button
style={{
marginInlineStart: "auto",
marginBlockStart: sizing.size_100,
}}
>
A button
</Button>
</BodyText>
);
return (
<ModalDialog aria-labelledby="modal-title-1" style={styles.dialog}>
<ModalPanel
Expand All @@ -312,7 +335,12 @@ export const WithStyle: StoryComponentType = {
title="Modal Title"
/>
}
content={longBody}
content={
<>
{longBody}
{button}
Copy link
Member

@beaesguerra beaesguerra Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is scrollable content in a modal and no interactive elements within it (like the original example without the button), should we add tabIndex=0 to the scrollable container, similar to what's done in frontend with ScrollableView?

I'm wondering if there are valid use cases where there might not be an interactive element within the scrollable modal content that would prevent scrolling via keyboard! What do you think?

Copy link
Member Author

@marcysutton marcysutton Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some updates to ensure scrollability with the keyboard on those internal View components, and added the global focus style.

I will add that I don't love wrapping an internal View with tabIndex=0 though, because it needs an interactive role and accessible name for screen reader users as well. But these are already provided at the dialog level, and using the same ID reference for this scrollable container is doubly confusing...so I'm at a loss for how to name it. Maybe "scroll container" or something? That seems more like an aria-roledescription than a name, since it wouldn't really be unique.

What I'd love to see is some dynamic keyboard scroll handling so consumers don't have to manually add tabIndex attributes, accessible names, and widget roles. Ideally the browser would deal with it. But maybe ModalPanel (and ScrollableView in frontend, for that matter) could detect overflow scrolling as well as interactive components to deal with it conditionally.

Copy link
Member Author

@marcysutton marcysutton Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an issue for it: https://khanacademy.atlassian.net/browse/FEI-7356

Copy link
Member

@beaesguerra beaesguerra Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for creating a ticket! I agree that this would probably need a broader solution! The keyboard scrolling issue is also related to the SimpleTable a11y fixes I'm working on in https://github.com/Khan/frontend/pull/5786

Ideally the browser would deal with it. But maybe ModalPanel (and ScrollableView in frontend, for that matter) could detect overflow scrolling as well as interactive components to deal with it conditionally.

FWIW, Chrome + Firefox on macOS will automatically make a scrollable div focusable! Safari does not 😢 https://jsfiddle.net/qorx1a5b/3/

</>
}
style={modalStyles}
/>
</ModalDialog>
Expand Down Expand Up @@ -352,4 +380,7 @@ const styles = StyleSheet.create({
content: {
gap: sizing.size_240,
},
scrollContainer: {
":focus-visible": focusStyles.focus[":focus-visible"],
},
});
5 changes: 5 additions & 0 deletions packages/wonder-blocks-modal/src/components/drawer-launcher.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,11 @@ function DrawerLauncherKeypressListener({onClose}: {onClose: () => unknown}) {
* enter and exit animations. It is also used to coordinate timing of focus management
* on open and close.
*
* For conditionally rendering modals, ensure there is only one `DrawerLauncher` in
* your component tree. A launcher needs to stay mounted on the current page to
* properly handle the user's keyboard focus on close of modals.
* Read [more details on Confluence](https://khanacademy.atlassian.net/wiki/spaces/FRONTEND/blog/2025/11/24/4454383789/Wonder+Blocks+Modal+Tips+Tricks).
*
* ### Usage
*
* ```jsx
Expand Down