Density-aware overlay close-button spacing

[tomhazledine]

Adjusts the right-hand spacing of <OverlayPanelContent> to match the width of the close button in all densities.

Touch density

Medium density

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
saltdesignsystem	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 9, 2024 11:40am

[changeset-bot]

🦋 Changeset detected

Latest commit: 1d19f54

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

This PR includes changesets to release 1 package

Name	Type
@salt-ds/core	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

[joshwooding]

Converted to draft due to the scrollbar overlapping the close button

[tomhazledine]

Converted to draft due to the scrollbar overlapping the close button

Talked to @bhoppers2008 about this while dev'ing, and we decided that the scrollbar/close-button overlap was the least bad of all the current options.

I guess our options are:

1. keep the overlap
2. make the scrollbar always visible
3. bump the margins so that the close button always leaves enough room for a scrollbar (whether needed or not)
4. work out if there is a scroll bar required, and only reposition the close button when there is a scroll bar

My view is that the happy-path is when all the content fits inside the overlay panel (thus no scrolling), so option 4 wouldn't be a sensible ROI 🤷

Thoughts?

[origami-z]

Talked to @bhoppers2008 about this while dev'ing, and we decided that the scrollbar/close-button overlap was the least bad of all the current options.

Given long content isn't a common scenario, and overlapping with close button looks particularly bad on Windows, can we do

• When not long, make it content | close
• When long, make it content | scrollbar | close

I think overflow: auto is enough to achieve this?

[origami-z]

Is this ready for review? Design decision made yet?

[tomhazledine]

After talking to @bhoppers2008 we're going to revisit the scrollbar position in the future, but for now we're going with content | scrollbar | closebutton (which means the scrollbar never overlaps the close button). The main focus for this PR is the fix to spacing (which previously did not respect changes in density).

[origami-z]

Tested in chrome/safari/firefox on Mac

[tomhazledine]

Closing now that we have a new approach from design: #3839

[origami-z]

Closing now that we have a new approach from design: #3839

That does not address our user needs. Also don't know yet whether #3839 would be a viable non-breaking change