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

docs(dial,dialog,divider,dropindicator): Migrate docs to storybook pt7 #2833

Merged
merged 18 commits into from
Aug 23, 2024

Conversation

marissahuysentruyt
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt commented Jun 10, 2024

Description

This PR continues migrating documentation from the static docs site into Storybook. The focus is on dial, dialog (standard dialog), divider and dropindicator. All variants of the components should now be displayed on the Storybook Docs page.

  • dial: new MDX docs page new stories
  • dialog: !!new MDX docs page~~ new stories and controls (plus an additional note regarding alert dialog)
  • divider: new MDX docs page new stories
  • dropindicator: new MDX docs page

This PR doesn't need a changeset since it's docs-only.

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

Regression testing

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • The pages render correctly, are accessible, and are responsive.
  1. If components have been modified, VRTs have been run on this branch:
  • VRTs have been run and looked at.
  • Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • I have tested these changes in Windows High Contrast mode.
  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

Copy link

changeset-bot bot commented Jun 10, 2024

⚠️ No Changeset found

Latest commit: 4146e5e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

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

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Jun 10, 2024

File metrics

Summary

Total size: 4.63 MB*

🎉 No changes detected in any packages

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

Copy link
Contributor

github-actions bot commented Jun 10, 2024

🚀 Deployed on https://pr-2833--spectrum-css.netlify.app

@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-759-docs-to-storybook-pt7 branch 2 times, most recently from 31c9260 to 297b64e Compare June 11, 2024 17:15
@marissahuysentruyt marissahuysentruyt marked this pull request as ready for review June 11, 2024 18:27
@marissahuysentruyt marissahuysentruyt added the run_vrt For use on PRs looking to kick off VRT label Jun 11, 2024
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-759-docs-to-storybook-pt7 branch from 5b3e7a9 to c165aed Compare June 12, 2024 13:42
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-759-docs-to-storybook-pt7 branch 5 times, most recently from 1ec5878 to 60d8a70 Compare June 12, 2024 16:26
@marissahuysentruyt marissahuysentruyt added ready-for-review and removed run_vrt For use on PRs looking to kick off VRT labels Jun 12, 2024
components/dial/stories/dial.stories.js Outdated Show resolved Hide resolved
components/dial/stories/dial.stories.js Outdated Show resolved Hide resolved
components/dialog/stories/dialog.stories.js Outdated Show resolved Hide resolved
components/dialog/stories/dialog.stories.js Outdated Show resolved Hide resolved
components/dialog/stories/dialog.stories.js Outdated Show resolved Hide resolved
components/dialog/stories/dialog.mdx Outdated Show resolved Hide resolved
components/dialog/stories/dialog.mdx Outdated Show resolved Hide resolved
components/dialog/stories/dialog.stories.js Outdated Show resolved Hide resolved
components/dialog/stories/dialog.stories.js Outdated Show resolved Hide resolved
components/dialog/stories/dialog.stories.js Outdated Show resolved Hide resolved
@pfulton pfulton requested review from aramos-adobe and cdransf and removed request for pfulton August 22, 2024 20:28
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-759-docs-to-storybook-pt7 branch from 5f3f15e to b84e78e Compare August 23, 2024 00:15
Copy link
Collaborator

@aramos-adobe aramos-adobe left a comment

Choose a reason for hiding this comment

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

Migration is there and looks great in the preview link 🎉 Everything looks on point!

Just a few comments:

  • Dialog Docs renders perfectly in the preview link, but has an error in localhost on my end. The preview.js templates doesn't render.
  • The dragging on Dial feels different on the Storybook than the OG docs version
Screen.Recording.2024-08-22.at.10.33.50.PM.mov
Screen.Recording.2024-08-22.at.10.34.51.PM.mov
Screen.Recording.2024-08-22.at.10.35.35.PM.mov
Screenshot 2024-08-22 at 10 34 27 PM

- adds static color stories, a sizing story and vertical story
- removes customStyles arg from default args because they were
overwriting other styles that are dependent on the orientation (i.e.
vertical)
- refactors chromatic coverage to reflect removal of custom styles in
default template
- adds all sizing stories, with hero, no divider, fullscreen/fullscreen-
takeover, scorlling example stories
- refactors template to accommodate buttons when the dialog is
dismissible (no buttons based on guidance/documentation), vs. non-
dismissible
- adds test coverage example for new stories
- fixes sizing template issues in Variants() for showModal: false
- renames heading for dialog arg to dialogHeading (to avoid variable
naming conflicts between the component and Sizing function used within
Variants())
revert arg name "dialogHeading" back to "heading." It's a known issue
that the Variant() function is mixing the dialog's heading arg with the
Sizing() heading arg. That refactoring and fix will be done in a new
branch.
instead of deleting the isFocused type just to replace it with an
identical isFocusVisible type & control, this extends isFocused
definition to allow the isFocusVisible variable name
VRT diffs were found for Menu and Picker, so tracing them back, were
caused by an incorrect inline-size property of 100% (as opposed to the
corrected "auto")
To work better with React's hot module reloader, this should only call a
template once and toggle the inputs based on settings.
- use the presence of the hero image url as a boolean, instead of an
actual hasHeroImage boolean
- removes layout !== default since it doesn't do anything anyways
- use .every() array method for layout class modifiers
- use the presence of the hero image url as a boolean, instead of actual
hasHeroImage boolean
this naming matches the other Sizing story, as well as indicates it is
going through the Sizes() function.
- enforces orientation for divider test cases
- creates new minDimensionValues arg to use as visualization. Because
the inline and block sizes of the divider are dependant on content
(which doesn't exist on this component), this new arg sets example values
for min-inline-size and min-block-size. Any other customStyles passed to
the component should still render as before, and we are relying on the
CSS now instead of the styleMap in the default template.
- refactors the min-inline-size/min-block-size in styleMap to use new
minDimensionValues arg
- adds extra documentation
- corrects the default size of divider to small
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-759-docs-to-storybook-pt7 branch from b84e78e to 4146e5e Compare August 23, 2024 18:15
@pfulton pfulton merged commit aacdec5 into main Aug 23, 2024
14 checks passed
@pfulton pfulton deleted the marissahuysentruyt/css-759-docs-to-storybook-pt7 branch August 23, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Because documentation is important and shouldn't be broken high priority An important PR or issue requiring immediate attention ready-for-review run_vrt For use on PRs looking to kick off VRT storybook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants