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(picker): update picker display and icon flex properties to resolve layout bugs #3220

Merged
merged 2 commits into from
Oct 11, 2024

Conversation

cdransf
Copy link
Member

@cdransf cdransf commented Oct 8, 2024

Description

Addresses issue with vertical alignment of picker text/placeholder by applying inline-flex in template rather than inline-block. Applies flex-shrink to validation icons to prevent icon from resizing when label is long enough to be truncated.

CSS-977

How and where has this been tested?

Verified locally in Storybook.

Validation steps

  1. Fetch branch and run locally or access the Storybook URL for the PR.
  2. Navigate to the picker component.
  3. Verify that the placeholder text is properly aligned within the picker.
  4. Verify that the validation icon does not resize when a long label is supplied as an arg.

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

Screenshot 2024-10-08 at 8 56 30 AM Screenshot 2024-10-08 at 8 56 35 AM

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

@cdransf cdransf added skip_vrt Add to a PR to skip running VRT (but still pass the action) ready-for-review labels Oct 8, 2024
@cdransf cdransf self-assigned this Oct 8, 2024
Copy link

changeset-bot bot commented Oct 8, 2024

🦋 Changeset detected

Latest commit: 9d94081

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

Copy link
Contributor

github-actions bot commented Oct 8, 2024

🎉 Published on https://spectrum-css.netlify.app as production
🚀 Deployed on https://670968425c531f1cf3288dba--spectrum-css.netlify.app

Copy link
Contributor

github-actions bot commented Oct 8, 2024

File metrics

Summary

Total size: 4.31 MB*
Total change (Δ): 🔴 ⬆ 0.05 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.17 KB 🔴 ⬆ 0.02 KB

Details

picker

Filename Head Compared to base
index-base.css 28.28 KB 🔴 ⬆ 0.02 KB (0.06%)
index-theme.css 2.50 KB -
index-vars.css 30.17 KB 🔴 ⬆ 0.02 KB (0.06%)
index.css 30.17 KB 🔴 ⬆ 0.02 KB (0.06%)
themes/express.css 1.51 KB -
themes/spectrum.css 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.

@cdransf cdransf force-pushed the cdransf/picker-fixes branch from 14cd46a to e373137 Compare October 8, 2024 17:11
@cdransf cdransf added the blocked See description and comments for what is blocking this issue label Oct 8, 2024
@cdransf cdransf force-pushed the cdransf/picker-fixes branch 3 times, most recently from 8e9ce4c to 98af18b Compare October 9, 2024 23:09
Copy link
Collaborator

@rise-erpelding rise-erpelding left a comment

Choose a reason for hiding this comment

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

Validated that this works with value and placeholder, with icons, and is not an issue with the loading progress circle!

@cdransf cdransf requested a review from jawinn October 10, 2024 19:13
@cdransf cdransf enabled auto-merge (squash) October 10, 2024 19:14
@cdransf cdransf removed the blocked See description and comments for what is blocking this issue label Oct 10, 2024
@cdransf cdransf force-pushed the cdransf/picker-fixes branch from 98af18b to 9d94081 Compare October 11, 2024 17:48
@cdransf cdransf requested a review from jawinn October 11, 2024 17:49
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.

Looks good, thanks. It just needs the VRTs checked before it merges.

@cdransf cdransf merged commit b28e1d4 into main Oct 11, 2024
12 checks passed
@cdransf cdransf deleted the cdransf/picker-fixes branch October 11, 2024 17:54
@github-actions github-actions bot mentioned this pull request Oct 11, 2024
@cdransf cdransf removed the skip_vrt Add to a PR to skip running VRT (but still pass the action) label Oct 11, 2024
@cdransf cdransf added the run_vrt For use on PRs looking to kick off VRT label Oct 11, 2024
@cdransf cdransf restored the cdransf/picker-fixes branch October 11, 2024 17:55
@cdransf cdransf deleted the cdransf/picker-fixes branch October 11, 2024 17:58
@cdransf cdransf restored the cdransf/picker-fixes branch October 11, 2024 17:58
@cdransf cdransf added skip_vrt Add to a PR to skip running VRT (but still pass the action) and removed run_vrt For use on PRs looking to kick off VRT labels Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review skip_vrt Add to a PR to skip running VRT (but still pass the action)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants