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): express border width values #2367

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

jenndiaz
Copy link
Contributor

@jenndiaz jenndiaz commented Dec 13, 2023

Description

Addresses current issue with the express picker where text positioning and focus indicators did not match design specs.

Fix: the token values for border width on the picker was set to 0 for express. For any of the css where we were using calc() with the border width being 0 the calc() was returning 0. Created custom tokens for the border width and set the express values to 0px instead of 0

This was changed in the tokens repo relatively recently.

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

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

Before

Screenshot 2023-12-13 at 10 27 00 AM

After

Screenshot 2023-12-13 at 10 31 47 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. ✨

Copy link
Contributor

github-actions bot commented Dec 13, 2023

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

Copy link
Contributor

github-actions bot commented Dec 13, 2023

File metrics

Summary

Total size: 3.97 MB*
Total change (Δ): ⬇ 0.71 KB (0.02%)

Package Size Δ
picker 31.00 KB 🚨 deleted, moved, or renamed
Details

picker

File Size Base Δ
Total 31.19 KB 31.00 KB ⬇ 0.20 KB (0.61%)
index-base.css 29.31 KB 29.24 KB ⬇ 0.08 KB (0.26%)
index-theme.css 2.46 KB 2.35 KB ⬇ 0.12 KB (4.72%)
index-vars.css 31.19 KB 31.00 KB ⬇ 0.20 KB (0.61%)
index.css 31.19 KB 31.00 KB ⬇ 0.20 KB (0.61%)
mods.json 2.71 KB 2.71 KB -
themes/express.css 1.47 KB 1.43 KB ⬇ 0.04 KB (2.98%)
themes/spectrum.css 1.01 KB 0.94 KB ⬇ 0.07 KB (7.31%)
* 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.

@jenndiaz jenndiaz changed the title fix(picker): border width values fix(picker): express border width values Dec 13, 2023
@jenndiaz jenndiaz marked this pull request as ready for review December 13, 2023 17:36
@jenndiaz jenndiaz requested review from pfulton, mdt2 and jawinn December 13, 2023 17:36
Copy link
Collaborator

@pfulton pfulton left a comment

Choose a reason for hiding this comment

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

Great catch.

@pfulton pfulton added run_vrt For use on PRs looking to kick off VRT ready-to-merge labels Dec 13, 2023
@pfulton pfulton merged commit 4932f12 into main Dec 13, 2023
@pfulton pfulton deleted the jenndiaz/css-656-picker-express-layout-fix branch December 13, 2023 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge run_vrt For use on PRs looking to kick off VRT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants