-
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(slider,radio): dir before pseudo element #2744
Conversation
🦋 Changeset detectedLatest commit: 9ccdc7a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
🚀 Deployed on https://pr-2744--spectrum-css.netlify.app |
File metricsSummaryTotal size: 4.65 MB* Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.
Detailsradio
slider
* Results are not gzipped or minified. * An ASCII character in UTF-8 is 8 bits or 1 byte. |
12d601e
to
3edb65e
Compare
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.
Looks good! I validated the changes in the dist output with yarn compare
and looked at those two components in Storybook in RTL/LTR.
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.
Can you take a look at the impact of changing the dir selector from querying locally to the component vs. looking to the root of the document? My thought is that if a product has a region of the page localized for example in a language requiring rtl, looking to the root element in the DOM tree will give the wrong result.
3edb65e
to
fa77908
Compare
fa77908
to
9ccdc7a
Compare
Description
Per issue #2703 and #2704, some parsers see
:pseudo:dir
as invalid, so we've changed it so that the pseudo element comes last:dir :pseudo
.Before, the dist output was:
After, with this improvement, the dist output is:
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
Slider
Radio
To-do list