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

chore: storybook context application #2696

Merged
merged 2 commits into from May 2, 2024

Conversation

castastrophe
Copy link
Collaborator

@castastrophe castastrophe commented Apr 25, 2024

Description

Leveraging the story's id context in order to attempt to more accurately apply a story's individual contexts. In addition, this update uses querySelectors to determine if a context includes static(Black|White) or overBackground classes in it's content and applies the styles based on those settings.

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

  • Expect to see a new baseline for Static white progress circle
  • Expect the default progress circle test to focus on the standard and indeterminate states and not the static variant.
  • Expect no other regressions.

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. ✨

@castastrophe castastrophe added the run_vrt For use on PRs looking to kick off VRT label Apr 25, 2024
@castastrophe castastrophe self-assigned this Apr 25, 2024
Copy link

changeset-bot bot commented Apr 25, 2024

⚠️ No Changeset found

Latest commit: c3a36e0

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 Apr 25, 2024

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

Copy link
Contributor

github-actions bot commented Apr 25, 2024

File metrics

Summary

Total size: 4.57 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.

@castastrophe castastrophe force-pushed the chore-context-storybook-update branch from ce4cefc to c9a4820 Compare April 25, 2024 16:09
@castastrophe castastrophe marked this pull request as ready for review April 25, 2024 16:16
@castastrophe castastrophe force-pushed the chore-context-storybook-update branch 3 times, most recently from d080af5 to ebd5957 Compare April 29, 2024 16:54
Copy link
Collaborator

@jawinn jawinn left a comment

Choose a reason for hiding this comment

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

I tested these changes to contextsWrapper.js in the S2 button branch and this does appear to fix the issue with first load of the custom MDX page, along with making sure that all the stories on that page have their gray layer 1 background color coming from the .spectrum class 👍

One place I am not seeing the classes on are the auto "Docs" pages for the components. In particular the first story with controls does not show the background color. Changing the color theme to dark highlights this:
Screenshot 2024-04-29 at 2 05 25 PM
It looks like the container querying may need adjustment to include these auto docs stories? Inspecting, I see that it does still have a container with the .docs-story class on it.

Comment on lines 36 to 44
const roots = [...document.querySelectorAll(`#story--${id}`)];
if (viewMode === "docs" && roots.length > 0) {
containers = roots.map(root => root.closest(".docs-story") ?? root);
}

for (const s of scales) {
container.classList.toggle(`spectrum--${s}`, s === scale);
}
for (const container of containers) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice work finding a better way to handle these different .docs-story containers that we see on the Docs pages.

@castastrophe castastrophe force-pushed the chore-context-storybook-update branch from ebd5957 to 78ea356 Compare April 30, 2024 13:30
@castastrophe
Copy link
Collaborator Author

It looks like the container querying may need adjustment to include these auto docs stories? Inspecting, I see that it does still have a container with the .docs-story class on it.

Great catch @jawinn! Yes, the first container on the page adds a --primary postfix to the id that I was missing which was why it was skipping that one box. I've updated it and reviewed several auto docs pages to confirm. Thanks!

Copy link
Collaborator

@jawinn jawinn left a comment

Choose a reason for hiding this comment

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

That worked! The auto docs pages have the right backgrounds now. There is just one leftover console.log to remove, then I think this is good to go.

.storybook/decorators/contextsWrapper.js Outdated Show resolved Hide resolved
castastrophe and others added 2 commits May 2, 2024 11:53
Co-authored-by: Josh Winn <965114+jawinn@users.noreply.github.com>
@castastrophe castastrophe force-pushed the chore-context-storybook-update branch from 6064c09 to c3a36e0 Compare May 2, 2024 15:53
@castastrophe castastrophe enabled auto-merge (squash) May 2, 2024 15:54
@castastrophe castastrophe merged commit f793928 into main May 2, 2024
13 checks passed
@castastrophe castastrophe deleted the chore-context-storybook-update branch May 2, 2024 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review run_vrt For use on PRs looking to kick off VRT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants