fix(slider ramp): correct handle and ramp vertical alignment

[cdransf]

Description

This applies a flex layout to the spectrum slider controls to consistently align the enclosed handle and ramp, while also removing the margins that might otherwise interfere with alignment. This aims to resolve the issues with the calc-based approach that exhibited variations in vertical alignment.

How and where has this been tested?

Checked locally in Storybook.

Validation steps

1. Pull down the branch and run Storybook locally or access the Storybook URL for the PR.
2. Open the Slider component and select Default.
3. Verify that the handle is vertically aligned over the track.

Regression testing

Validate:

1. The documentation pages for at least two other components are still loading, including:

• The pages render correctly, are accessible, and are responsive.

4. If components have been modified, VRTs have been run on this branch:

• VRTs have been run and looked at.
• Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

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. ✨

[changeset-bot]

🦋 Changeset detected

Latest commit: 6ccc2a4

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/slider	Minor

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

[github-actions]

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

[github-actions]

File metrics

Summary

Total size: 4.31 MB^*
Total change (Δ): 🔴 ⬆ 0.73 KB (0.02%)

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

Package	Size	Δ
slider	32.41 KB	🔴 ⬆ 0.24 KB

Details

slider

Filename	Head	Compared to base
index-base.css	30.10 KB	🔴 ⬆ 0.24 KB (0.79%)
index-theme.css	2.92 KB	-
index-vars.css	32.41 KB	🔴 ⬆ 0.24 KB (0.74%)
index.css	32.41 KB	🔴 ⬆ 0.24 KB (0.74%)
themes/express.css	1.78 KB	-
themes/spectrum.css	1.75 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.

[marissahuysentruyt]

Do we need to preserve this spacing between the ramp container and the labels? Without it, both containers touch. To me, it looks like this spacing will need to be defined in S2 as well (just based on the "spacing (field label to slider control" and the "spacing (top/bottom edge to label)" headings). Maybe instead of removing it completely, we can move it elsewhere? Into a different selector block? I don't even know if that's possible, you probably know better than me at this point.

Screen.Recording.2024-09-30.at.9.29.45.AM.mov

[cdransf]

Alrighty! I've added the following to the root .spectrum-Slider class:

&:not(.spectrum-Slider--sideLabel) .spectrum-Slider-labelContainer + .spectrum-Slider-controls:has(.spectrum-Slider-ramp) {
  margin-block-start: var(--mod-spectrum-slider-value-side-padding-block, var(--spectrum-slider-value-side-padding-block));
}

Adding the 8px gap called for in the Figma for S2 to the .spectrum-Slider-controls provided the component is not in the side label configuration but is using the ramp design. I'm wondering if that selector is too verbose or perhaps too fragile, but it does need to balance all of those different cases and preserve the spacing without being applied to the .spectrum-Slider-ramp class directly as that does cause the misalignment with the slider control to crop back up.

[rise-erpelding]

Looked at all sizes, small and XL particularly are a great improvement!

[marissahuysentruyt]

Does this --spectrum-slider-value-side-padding-block actually need to be that whole calc from the margin-block-start instead of just var(--spectrum-spacing-100)? My thought is that the margin we removed was being calculated with --spectrum-slider-ramp-track-height, which is 16px for desktop and 20px for mobile. So I believe the margin was supposed to change when we toggle the "large" platform scale, so it would need to be 10px on mobile instead of 8px.

We should probably double check with Patrick & Cassondra (maybe even design) because it does look like this spacing might be staying at 8px for the ramp slider in S2 (regardless of desktop vs mobile). Am I looking at everything right? Is preserving that padding now worth it, if it's going to change (basically to how you have it currently with spacing-100) soon-ish?

[marissahuysentruyt]

Thanks for digging into this one! When I wrote up that bug card, I had no idea how many scenarios we'd be trying to account for, so thanks for the persistence!

I'm going to approve, but I do believe there are some selectors and mods in the metadata files that were built that no longer apply (since we removed that created custom property). I'm assuming those should probably be removed.