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

fix(popover): no more duplicate class selectors in dist #2745

Merged
merged 1 commit into from
May 14, 2024

Conversation

mdt2
Copy link
Collaborator

@mdt2 mdt2 commented May 10, 2024

Description

Fix for issue #2705. Previously, there were duplicate class selectors in dist. This update adjusts the selectors to remove the duplicates.

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

  • Validate that there aren't any duplicate classes in the dist output locally
  • Check that popover tip placement still looks good in Storybook

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 May 10, 2024

🦋 Changeset detected

Latest commit: a7b6f94

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@spectrum-css/popover Patch

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

@mdt2 mdt2 added the run_vrt For use on PRs looking to kick off VRT label May 10, 2024
Copy link
Contributor

github-actions bot commented May 10, 2024

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

Copy link
Contributor

github-actions bot commented May 10, 2024

File metrics

Summary

Total size: 4.42 MB*
Total change (Δ): ⬇ 64.78 KB (-1.41%)

Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.

Package Size Δ
popover 15.93 KB ⬇ 16.16 KB

Details

popover

File Head Base Δ
index-base.css 15.78 KB 31.93 KB ⬇ 16.16 KB (-50.60%)
index-theme.css 0.75 KB 0.75 KB -
index-vars.css 15.93 KB 32.09 KB ⬇ 16.16 KB (-50.35%)
index.css 15.93 KB 32.09 KB ⬇ 16.16 KB (-50.35%)
metadata.json 10.32 KB 26.63 KB ⬇ 16.31 KB (-61.24%)
themes/express.css 0.66 KB 0.66 KB -
themes/spectrum.css 0.68 KB 0.68 KB -
* 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
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.

Oh the trouble that a misplaced curly brace can cause 😆 This seems to have fixed it. It's a little tricky to validate with how Storybook currently handles the popover interaction, but I tried all the positioning options + a browser refresh each time and did not see anything unusual for the "with tip" variant.

@castastrophe castastrophe force-pushed the mdt2/css-750-popover-duplicate-selector branch from 16b75d7 to 2d4f9d3 Compare May 14, 2024 15:44
@castastrophe castastrophe enabled auto-merge (squash) May 14, 2024 15:45
@mdt2 mdt2 force-pushed the mdt2/css-750-popover-duplicate-selector branch from 2d4f9d3 to a7b6f94 Compare May 14, 2024 16:41
@castastrophe castastrophe merged commit 2c13cdc into main May 14, 2024
13 checks passed
@castastrophe castastrophe deleted the mdt2/css-750-popover-duplicate-selector branch May 14, 2024 16:46
@github-actions github-actions bot mentioned this pull request May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge run_vrt For use on PRs looking to kick off VRT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants