-
Notifications
You must be signed in to change notification settings - Fork 195
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
fix(storybook): body classes for story and docs pages #2617
fix(storybook): body classes for story and docs pages #2617
Conversation
🚀 Deployed on https://pr-2617--spectrum-css.netlify.app |
File metricsSummaryTotal size: 4.48 MB* Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.
Detailsactionbutton
actiongroup
assetcard
button
buttongroup
checkbox
closebutton
dial
fieldlabel
logicbutton
modal
page
picker
slider
splitview
table
tokens
* Results are not gzipped or minified. * An ASCII character in UTF-8 is 8 bits or 1 byte. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The component styles do load now when visiting the two foundation pages first. I see the button and checkbox styles now on those pages. 👍
Issue with background colors also on spectrum-two
I am seeing one difference with the background colors. I tested on the spectrum-two branch though and am seeing the same thing. So this could be a problem for a separate PR, unless you're aware of a fix related to the changes you made. The problem seems somewhat connected.
On first load of a foundations page, I see these two sections with a different grey background:
If I click around to another component story and then back to a foundation page, the extra background colors go away and it's back to normal.
@jawinn weird. I was also seeing this initially while working on this PR, but after making the changes to the contextWrapper, I'm not seeing the backgrounds anymore 😵 Locally I:
Did you take different steps when you saw the weird backgrounds? Edit: Confirming that I do see this happening locally on the spectrum-two branch, but not on this branch even after running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mdt2 Should we be adding this change to main and then bringing it back into spectrum-two or is it a spectrum-two specific infrastructure change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm no longer seeing the issue I was seeing before. LGTM! 👍
Description
With this fix, Foundation documentation pages will receive the global classes required for styles to show correctly. The fix was to exclude foundation pages from the contextsWrapper. This allows the existing code in
manager.js
to add thespectrum spectrum--light spectrum--medium
class stack to the<body>
tag inside the foundations page iframe instead of adding it to only the first<div id="root-inner>
(it works to add it here for our Story pages because we only have one Story instance on those pages, but in the docs pages we have 1+ story instances so we need the classes to be higher up on the iframe body).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
To-do list