Skip to content

chore(popover): adjust/enhance popover spacing by default [CSS-845]#4174

Merged
castastrophe merged 1 commit intospectrum-twofrom
castastrophe/chore-popoover-story-placement
Aug 29, 2025
Merged

chore(popover): adjust/enhance popover spacing by default [CSS-845]#4174
castastrophe merged 1 commit intospectrum-twofrom
castastrophe/chore-popoover-story-placement

Conversation

@castastrophe
Copy link
Contributor

@castastrophe castastrophe commented Aug 27, 2025

Description

This PR simplifies the Popover component's Storybook implementation by removing complex positioning JavaScript and hardcoded dimensions that were causing VRT issues and didn't represent the actual shipped component behavior.

Motivation and context

The current Popover stories attempt to emulate actual positioning behavior with JavaScript, which has caused ongoing VRT headaches. Since we're not shipping this Storybook implementation, simplifying it will provide a more reliable and maintainable experience.

In this update, I separated the Popover code from the Popover wrapper because the wrapper is a utility for alignment in VRTs and documentation but not representative of how the component is shipped in SWC.

The wrapper component for popover now performs an init height/width check for popover and trigger as well as a resize observer to ensure accuracy. Those values are added to the wrapper as custom properties so that positioning can use them for absolute alignment.

Related issue(s)

  • Addresses ongoing VRT positioning issues with Popover stories
  • Simplifies Storybook implementation to better represent actual component usage

Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed the Accessibility Practices for this feature, see: Aria Practices
  • I have added automated tests to cover my changes.
  • I have included a well-written changeset if my change needs to be published.
  • I have included updated documentation if my change required it.

Reviewer's checklist

  • Includes a Github Issue with appropriate flag or Jira ticket number without a link
  • Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
  • Automated tests cover all use cases and follow best practices for writing
  • Validated on all supported browsers
  • All VRTs are approved before the author can update Golden Hash

Manual review test cases

  • Popover stories display correctly without positioning JavaScript

    • Go to Popover component stories
    • Verify Default and Default with tip stories show in top-left corner
    • Confirm popover appears at bottom of source element
    • Test at different scales (Small, Medium, Large, Extra Large)
  • ContextualHelp stories maintain proper popover positioning

    • Go to ContextualHelp component stories
    • Verify popover placement options work correctly
    • Test RTL and large scale positioning
  • Coachmark stories display without hardcoded dimensions

    • Go to Coachmark component stories
    • Verify stories render without positioning issues
    • Test action menu states display properly
  • Combobox stories maintain proper spacing

    • Go to Combobox component stories
    • Verify dropdown positioning works correctly
    • Test different sizes and states

@changeset-bot
Copy link

changeset-bot bot commented Aug 27, 2025

🦋 Changeset detected

Latest commit: e602f75

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

This PR includes changesets to release 3 packages
Name Type
@spectrum-css/popover Patch
@spectrum-css/bundle Patch
@spectrum-css/preview 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

@castastrophe castastrophe force-pushed the castastrophe/chore-popoover-story-placement branch from 581de70 to 0b6bf81 Compare August 27, 2025 20:58
@github-actions
Copy link
Contributor

github-actions bot commented Aug 27, 2025

File metrics

Summary

Total size: 1.43 MB*

Package Size Minified Gzipped
popover 16.44 KB 15.86 KB 2.00 KB

popover

Filename Head Minified Gzipped Compared to base
index.css 16.44 KB 15.86 KB 2.00 KB 🔴 ⬆ 0.46 KB
metadata.json 7.30 KB - - -
* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 27, 2025

📚 Branch preview

PR #4174 has been deployed to Azure Blob Storage: https://spectrumcss.z13.web.core.windows.net/pr-4174/index.html.

@castastrophe castastrophe added bug Results from a bug in the CSS implementation size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete. run_vrt For use on PRs looking to kick off VRT storybook low priority Not a critical update or fix; can be deprioritized if necessary S2 Spectrum 2 labels Aug 27, 2025
@castastrophe castastrophe changed the title chore(popover): adjust/enhance popover spacing by default chore(popover): adjust/enhance popover spacing by default [CSS-845] Aug 28, 2025
@castastrophe castastrophe force-pushed the castastrophe/chore-popoover-story-placement branch from 0b6bf81 to 552bd90 Compare August 28, 2025 15:48
@castastrophe castastrophe self-assigned this Aug 28, 2025
@castastrophe castastrophe force-pushed the castastrophe/chore-popoover-story-placement branch 4 times, most recently from f3eced7 to a4700c5 Compare August 28, 2025 16:46
@aramos-adobe
Copy link
Contributor

@castastrophe It feels nice to not have those customStyle properties added anymore now that everything is consistent. Thank you!

],
}, context),
],
popoverHeight: 42,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer needed! This is calculated now on the fly (and more accurately) by the popover wrapper.

},
iconSet: { table: { disable: true } },
popoverPlacement: {
name: "Popover Placement",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated this to use the existing Popover storybook knob so that if the data changes from the source, this inherits the update.

@castastrophe castastrophe force-pushed the castastrophe/chore-popoover-story-placement branch 3 times, most recently from 7a52c29 to b7abb4a Compare August 29, 2025 13:25
@castastrophe castastrophe force-pushed the castastrophe/chore-popoover-story-placement branch from b7abb4a to e602f75 Compare August 29, 2025 13:34
@castastrophe castastrophe merged commit 1895ddb into spectrum-two Aug 29, 2025
15 checks passed
@castastrophe castastrophe deleted the castastrophe/chore-popoover-story-placement branch August 29, 2025 13:42
@github-actions github-actions bot mentioned this pull request Aug 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Results from a bug in the CSS implementation low priority Not a critical update or fix; can be deprioritized if necessary run_vrt For use on PRs looking to kick off VRT S2 Spectrum 2 size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete. storybook

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants