Skip to content
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.

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

Migrate ModalDialog to Storybook #1574

Merged
merged 3 commits into from
Aug 26, 2022
Merged

Migrate ModalDialog to Storybook #1574

merged 3 commits into from
Aug 26, 2022

Conversation

nishasy
Copy link
Contributor

@nishasy nishasy commented Aug 26, 2022

Summary:

Using Styleguidist as a foundation, migrate the documentation for
ModalDialog to Storybook.

Styleguidist does not have specific examples for each building
block under modal, but I decided to start adding them all with their own
examples (as opposed to using subcomponents), particularly so that
users can experiment with the control panel. This one was a little
tricky since ModalDialog does not have a visual element.

Storybook publish:
https://5e1bf4b385e3fb0020b7073c-sbvqecqsiq.chromatic.com/?path=/docs/modal-building-blocks-modaldialog--default

Issue: https://khanacademy.atlassian.net/browse/WB-1141

Test plan:

  • Check that all the documentation makes sense.
  • Check there are no typos.
  • Interact with the modals to confirm they all behave as expected.

@nishasy nishasy self-assigned this Aug 26, 2022
@nishasy nishasy requested a review from jandrade August 26, 2022 00:30
@changeset-bot
Copy link

changeset-bot bot commented Aug 26, 2022

🦋 Changeset detected

Latest commit: 8b987bc

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 khan-actions-bot requested a review from a team August 26, 2022 00:30
@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/wonder-blocks for changes to .changeset/swift-apricots-double.md, packages/wonder-blocks-modal/src/components/__docs__/modal-dialog.stories.js, packages/wonder-blocks-modal/src/components/__docs__/one-pane-dialog.stories.js

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

@netlify
Copy link

netlify bot commented Aug 26, 2022

Deploy Preview for wonder-blocks ready!

Name Link
🔨 Latest commit 8b987bc
🔍 Latest deploy log https://app.netlify.com/sites/wonder-blocks/deploys/63093b016f60fa0009cec1e4
😎 Deploy Preview https://deploy-preview-1574--wonder-blocks.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 26, 2022

Size Change: 0 B

Total Size: 74.2 kB

ℹ️ View Unchanged
Filename Size
packages/wonder-blocks-birthday-picker/dist/es/index.js 1.52 kB
packages/wonder-blocks-breadcrumbs/dist/es/index.js 894 B
packages/wonder-blocks-button/dist/es/index.js 2.82 kB
packages/wonder-blocks-cell/dist/es/index.js 1.92 kB
packages/wonder-blocks-clickable/dist/es/index.js 3.07 kB
packages/wonder-blocks-color/dist/es/index.js 1.01 kB
packages/wonder-blocks-core/dist/es/index.js 3.17 kB
packages/wonder-blocks-data/dist/es/index.js 6.01 kB
packages/wonder-blocks-dropdown/dist/es/index.js 11.5 kB
packages/wonder-blocks-form/dist/es/index.js 4.88 kB
packages/wonder-blocks-grid/dist/es/index.js 1.37 kB
packages/wonder-blocks-i18n/dist/es/index.js 4.46 kB
packages/wonder-blocks-icon-button/dist/es/index.js 1.75 kB
packages/wonder-blocks-icon/dist/es/index.js 2.43 kB
packages/wonder-blocks-layout/dist/es/index.js 1.74 kB
packages/wonder-blocks-link/dist/es/index.js 1.62 kB
packages/wonder-blocks-modal/dist/es/index.js 4.92 kB
packages/wonder-blocks-popover/dist/es/index.js 4.14 kB
packages/wonder-blocks-progress-spinner/dist/es/index.js 1.51 kB
packages/wonder-blocks-search-field/dist/es/index.js 1.24 kB
packages/wonder-blocks-spacing/dist/es/index.js 158 B
packages/wonder-blocks-testing/dist/es/index.js 3.64 kB
packages/wonder-blocks-timing/dist/es/index.js 1.61 kB
packages/wonder-blocks-toolbar/dist/es/index.js 855 B
packages/wonder-blocks-tooltip/dist/es/index.js 4.75 kB
packages/wonder-blocks-typography/dist/es/index.js 1.24 kB

compressed-size-action

@codecov
Copy link

codecov bot commented Aug 26, 2022

Codecov Report

Merging #1574 (8b987bc) into main (68c85d0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1574   +/-   ##
=======================================
  Coverage   97.14%   97.14%           
=======================================
  Files         205      205           
  Lines        4521     4521           
  Branches     1351     1351           
=======================================
  Hits         4392     4392           
  Misses        129      129           

Continue to review full report at Codecov.

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

Copy link
Member

@jandrade jandrade left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks for adding all these examples that will be super helpful for creating custom Dialogs. Just added a note for you to consider.

export const Default: StoryComponentType = (args) => (
<View style={styles.previewSizer}>
<View style={styles.modalPositioner}>
<ModalDialog {...args}>
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: What do you think about adding aria-labelledby referencing to a title within the ModalPanel contents?

That could be a good way to show the a11y case presented in the component description and we could show engineers how to use custom Dialogs while preserving accessibility.

Screen Shot 2022-08-26 at 8 52 28 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea!

return (
<View style={styles.previewSizer}>
<View style={styles.modalPositioner}>
<ModalDialog
Copy link
Member

Choose a reason for hiding this comment

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

Same comment related to aria-describedby here.

@nishasy nishasy requested a review from jandrade August 26, 2022 21:39
Copy link
Member

@jandrade jandrade left a comment

Choose a reason for hiding this comment

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

Awesome! thanks for adding aria-describedby to the examples :shipit:

@nishasy nishasy merged commit 5d16ee8 into main Aug 26, 2022
@nishasy nishasy deleted the sb-modal-dialog branch August 26, 2022 21:52
jandrade added a commit that referenced this pull request Sep 18, 2023
…ent (#1574) (#1986)

## Summary:

- Adds an overview section to the theming docs.
- Adds guidelines for adding theming support to a component.
- Modifies the sorting order of the storybook stories to put the overview
sections at the top.

Issue: WB-1573

## Test plan:

Verify that the new page is clear and documents how to add themes to a given component:

https://5e1bf4b385e3fb0020b7073c-jsshumrrxg.chromatic.com/?path=/docs/theming-guides-adding-a-theme--docs

Author: jandrade

Reviewers: #wonder-blocks, nishasy

Required Reviewers:

Approved By:

Checks: ✅ codecov/project, ✅ Test (ubuntu-latest, 16.x, 2/2), ✅ Check build sizes (ubuntu-latest, 16.x), ✅ Lint (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 1/2), ✅ gerald, ⏭  Publish npm snapshot, ✅ gerald, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 2/2), ✅ Test (ubuntu-latest, 16.x, 1/2), ✅ Lint (ubuntu-latest, 16.x), ✅ Check build sizes (ubuntu-latest, 16.x), ⏭  Publish npm snapshot, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ✅ gerald, ⏭  dependabot, ✅ Chromatic - Build on review PR (push) / chromatic (ubuntu-latest, 16.x), ⏭  Chromatic - Skip on dependabot PRs (push)

Pull Request URL: #1986
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants