Add modal launcher details to docs #2875

marcysutton · 2025-11-25T22:50:50Z

Summary:

After making bugfixes and updates to conditionally rendered modals, I figured we should update the docs.

Note: the ModalLauncher and DrawerLauncher descriptions are only rendered from stories files

Issue: WB-2103

Test plan:

Review Modal overview, ModalLauncher, and DrawerLauncher docs!

changeset-bot · 2025-11-25T22:50:53Z

🦋 Changeset detected

Latest commit: 9d29c92

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

khan-actions-bot · 2025-11-25T22:51:04Z

Gerald

Required Reviewers

@Khan/wonder-blocks for changes to .changeset/grumpy-dingos-confess.md, __docs__/wonder-blocks-modal/_overview_.mdx, __docs__/wonder-blocks-modal/drawer-launcher.stories.tsx, __docs__/wonder-blocks-modal/flexible-dialog.stories.tsx, __docs__/wonder-blocks-modal/modal-launcher.stories.tsx, __docs__/wonder-blocks-modal/modal-panel.stories.tsx, packages/wonder-blocks-modal/src/components/drawer-launcher.tsx

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

github-actions · 2025-11-25T22:52:25Z

Size Change: 0 B

Total Size: 109 kB

ℹ️ View Unchanged

Filename	Size
`packages/wonder-blocks-accordion/dist/es/index.js`	3 kB
`packages/wonder-blocks-announcer/dist/es/index.js`	1.74 kB
`packages/wonder-blocks-badge/dist/es/index.js`	2.02 kB
`packages/wonder-blocks-banner/dist/es/index.js`	2.01 kB
`packages/wonder-blocks-birthday-picker/dist/es/index.js`	1.92 kB
`packages/wonder-blocks-breadcrumbs/dist/es/index.js`	755 B
`packages/wonder-blocks-button/dist/es/index.js`	4.25 kB
`packages/wonder-blocks-card/dist/es/index.js`	1.06 kB
`packages/wonder-blocks-cell/dist/es/index.js`	2.19 kB
`packages/wonder-blocks-clickable/dist/es/index.js`	2.66 kB
`packages/wonder-blocks-core/dist/es/index.js`	2.48 kB
`packages/wonder-blocks-data/dist/es/index.js`	5.48 kB
`packages/wonder-blocks-dropdown/dist/es/index.js`	19.4 kB
`packages/wonder-blocks-form/dist/es/index.js`	6.2 kB
`packages/wonder-blocks-grid/dist/es/index.js`	1.24 kB
`packages/wonder-blocks-icon-button/dist/es/index.js`	3.2 kB
`packages/wonder-blocks-icon/dist/es/index.js`	1.91 kB
`packages/wonder-blocks-labeled-field/dist/es/index.js`	3.48 kB
`packages/wonder-blocks-layout/dist/es/index.js`	1.63 kB
`packages/wonder-blocks-link/dist/es/index.js`	1.52 kB
`packages/wonder-blocks-modal/dist/es/index.js`	7.06 kB
`packages/wonder-blocks-pill/dist/es/index.js`	1.31 kB
`packages/wonder-blocks-popover/dist/es/index.js`	4.3 kB
`packages/wonder-blocks-progress-spinner/dist/es/index.js`	1.48 kB
`packages/wonder-blocks-search-field/dist/es/index.js`	1.1 kB
`packages/wonder-blocks-styles/dist/es/index.js`	464 B
`packages/wonder-blocks-switch/dist/es/index.js`	1.55 kB
`packages/wonder-blocks-tabs/dist/es/index.js`	3.71 kB
`packages/wonder-blocks-testing-core/dist/es/index.js`	3.25 kB
`packages/wonder-blocks-testing/dist/es/index.js`	978 B
`packages/wonder-blocks-theming/dist/es/index.js`	384 B
`packages/wonder-blocks-timing/dist/es/index.js`	1.37 kB
`packages/wonder-blocks-tokens/dist/es/index.js`	5.01 kB
`packages/wonder-blocks-toolbar/dist/es/index.js`	906 B
`packages/wonder-blocks-tooltip/dist/es/index.js`	6.4 kB
`packages/wonder-blocks-typography/dist/es/index.js`	1.57 kB

_{compressed-size-action}

github-actions · 2025-11-25T22:54:07Z

A new build was pushed to Chromatic! 🚀

https://5e1bf4b385e3fb0020b7073c-kditebicgj.chromatic.com/

Chromatic results:

Metric	Total
Captured snapshots	285
Tests with visual changes	0
Total stories	742
Inherited (not captured) snapshots [TurboSnap]	157
Tests on the build	442

beaesguerra

Thanks for adding these docs!

__docs__/wonder-blocks-modal/modal-launcher.stories.tsx

marcysutton · 2025-12-01T22:27:35Z

Added fixes for the Modal a11y issues in stories! https://khanacademy.atlassian.net/browse/WB-2161

beaesguerra

Thanks for taking a look at the modal a11y issues! I left some questions around whether the original examples were valid use cases. I wanted to check first since this may signal underlying a11y issues for existing usage that follow the same patterns in the original examples. Let me know what you think!

(Also, I see there are similar a11y issues in other modal stories. If it's something we can fix at the component level, then we can address the others too! Sorry I didn't list out all the modal stories with issues in the ticket!)

beaesguerra · 2025-12-01T22:56:09Z

__docs__/wonder-blocks-modal/flexible-dialog.stories.tsx

            <View style={styles.modalPositioner}>
                <FlexibleDialog
                    aria-labelledby="main-heading"
+                    title={


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"]}

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

beaesguerra · 2025-12-01T23:01:48Z

__docs__/wonder-blocks-modal/modal-panel.stories.tsx

+                    content={
+                        <>
+                            {longBody}
+                            {button}


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?

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.

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

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/

github-actions · 2025-12-02T22:43:13Z

npm Snapshot: NOT Published

🤕 Oh noes!! We couldn't find any changesets in this PR (8a2046f). As a result, we did not publish an npm snapshot for you.

beaesguerra

Approving again for the modal a11y story fixes! Thanks for creating follow up tickets :)

codecov · 2025-12-04T00:41:17Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (c6b38e2) to head (9d29c92).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@     Coverage Diff      @@
##   main   #2875   +/-   ##
============================
============================

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6b38e2...9d29c92. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

marcysutton self-assigned this Nov 25, 2025

marcysutton added the olc-5.0.d873a label Nov 25, 2025

khan-actions-bot requested a review from a team November 25, 2025 22:51

beaesguerra approved these changes Nov 26, 2025

View reviewed changes

__docs__/wonder-blocks-modal/modal-launcher.stories.tsx Outdated Show resolved Hide resolved

marcysutton added 3 commits December 1, 2025 14:12

[modal-docs] Add modal details to docs

ee1b57b

[modal-docs] docs(changeset):

1dbac8a

[modal-docs] Update docs comment

684802c

marcysutton force-pushed the modal-docs branch from 14cb685 to 684802c Compare December 1, 2025 22:16

[modal-docs] Fix a11y issues

657d698

beaesguerra reviewed Dec 1, 2025

View reviewed changes

[modal-docs] Update modal stories for a11y

9d29c92

beaesguerra approved these changes Dec 3, 2025

View reviewed changes

marcysutton merged commit 5036747 into main Dec 4, 2025
15 checks passed

marcysutton deleted the modal-docs branch December 4, 2025 00:41

	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",
	})
	);

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

Add modal launcher details to docs #2875

Add modal launcher details to docs #2875

Uh oh!

Conversation

marcysutton commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Test plan:

Uh oh!

changeset-bot bot commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

khan-actions-bot commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Gerald

Uh oh!

github-actions bot commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A new build was pushed to Chromatic! 🚀

Chromatic results:

Uh oh!

beaesguerra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marcysutton commented Dec 1, 2025

Uh oh!

beaesguerra left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beaesguerra Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

marcysutton Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

beaesguerra Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

marcysutton Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

marcysutton Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

beaesguerra Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 2, 2025

npm Snapshot: NOT Published

Uh oh!

beaesguerra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Dec 4, 2025

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

marcysutton commented Nov 25, 2025 •

edited

Loading

changeset-bot bot commented Nov 25, 2025 •

edited

Loading

khan-actions-bot commented Nov 25, 2025 •

edited

Loading

github-actions bot commented Nov 25, 2025 •

edited

Loading

github-actions bot commented Nov 25, 2025 •

edited

Loading

beaesguerra left a comment •

edited

Loading