fix(picker): prevent diacritic text cutoff

[rise-erpelding]

Description

Addressing CSS-810, this fix addresses a bug that was called out recently, similar to what was seen in Textfield for both Spectrum CSS and React Spectrum with text with diacritics being cut off vertically. In this case, as the affected text occurs in a <span> rather than an <input> tag, the line-height does not need an adjustment, and changing the margin to padding appears to be sufficient to allow the vertical cutoff to be displayed.

There is also a draft PR to fix the same issue that uses the same line-height fix that is seen in textfield.

Note: When testing with Assistiv Labs, this cutoff issue did not appear to be present (in production)--this fix has been tested in Assistiv Labs (Chrome, Firefox, Edge) and does not appear to have an impact there.

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

1. Check out the Picker in Storybook locally or in the deploy preview
2. Change text to Thai:
   • In the Storybook controls, you can change the text for both "Label" and "Placeholder" to the Thai phrase that was causing an issue previously, "ทั้งหมด". You can also do this in the deployed version of Picker or from the main branch to see the cutoff issue.
   • If checking out Picker locally, you can alternatively cherry pick commit a1fab905fba48b9d3cf641a19160bee1b07e376a to temporarily change the English in Picker to Thai. This was a temporary commit I'd put in the PR but reverted in order to run VRTs.
3. Confirm that the text within the Picker is not being cut off vertically. The best way to know is to compare the field label ("Label" text, which has not been experiencing cutoff) with the picker label ("Placeholder" text, which is the element experiencing cutoff), applying zoom as needed.
4. Confirm that this continues to be true for:
   • all sizes of Picker
   • all variants of Picker
   • in more than one browser (I checked Chrome, Firefox, Edge, and Safari, all for Mac)
5. Check the Tabs component in Storybook:
   • confirm that there do not appear to be any regressions or issues as the result of this change to Picker

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.

9. 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--there is a 2px length difference in the snapshots for the Tabs component, this cannot be replicated in the browser, and I am unsure if it's an indication of significant diffs that might be visible downstream.

Screenshots

Before:

After:

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 (and AssistivLabs).
• If my change impacts other components, I have tested to make sure they don't break.
• ✨ This pull request is ready to merge. ✨

[changeset-bot]

🦋 Changeset detected

Latest commit: 4b82b96

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/picker	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

[github-actions]

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

[github-actions]

File metrics

Summary

Total size: 4.63 MB^*
Total change (Δ): ⬆ 0.01 KB (0.00%)

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

Package	Size	Δ
picker	30.01 KB	⬆ < 0.01 KB

Details

picker

File	Head	Base	Δ
index-base.css	28.13 KB	28.13 KB	⬆ < 0.01 KB (0.01%)
index-theme.css	2.50 KB	2.50 KB	-
index-vars.css	30.01 KB	30.01 KB	⬆ < 0.01 KB (0.01%)
index.css	30.01 KB	30.01 KB	⬆ < 0.01 KB (0.01%)
metadata.json	14.88 KB	14.88 KB	-
themes/express.css	1.51 KB	1.51 KB	-
themes/spectrum.css	1.60 KB	1.60 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]

This seems like a nice, simple fix! I didn't see any cutoff in the span text in any of the sizes, I tried to combine a bunch of the states to see if I could break it. I also tested in Chrome, Firefox, Safari and Edge on Mac, but it all looks good!

My only question now is do we drop those 2 temp commits (changing the text to Thai, and then the corresponding revert)? If we merged it right now, it would be ok, but do we have a preference for "temporary" commits?

Great catch on the spelling correction too! 🎉

[rise-erpelding]

My only question now is do we drop those 2 temp commits (changing the text to Thai, and then the corresponding revert)? If we merged it right now, it would be ok, but do we have a preference for "temporary" commits?

My original intention was to not have reverted the temporary commit for validation purposes and then to drop the commit before merging, but then I reverted it to run VRTs and ran into that unexpected diff that I wanted feedback on, so I left it that way. I think I would probably just squash/fixup those commits. Also was thinking there wasn't a lot of reason to keep the spelling change for "country" as a separate commit, so that'd be squashed in with the fix here.

[pfulton]

Thanks for digging into this one and I'm glad it was a pretty simple fix.

[pfulton]

Thanks for catching this!

[snowystinger]

Checking if there is any action I need to take in RSP.
Where was this reported against React Spectrum? it looks like we've already implemented this? https://react-spectrum.adobe.com/react-spectrum/Picker.html
Does anyone have the actual string which is cutoff? I have no idea what those characters are in the screenshot, can't copy and paste them :-/