fix(sp-picker): s2 theme update - no borders for picker #5733

iuliauta · 2025-09-16T13:17:43Z

Description

Picker border color should be hidden in S2 theme

Motivation and context

Fix the regressions found after changing from the express theme to the spectrum-two theme.

Related issue(s)

fixes SWC-1074
CCEX-230904

Screenshots (if appropriate)

Spectrum 2

Spectrum 1

Express

Author's checklist

I have read the CONTRIBUTING and PULL_REQUESTS documents.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
I have added automated tests to cover my changes.
I have included a well-written changeset if my change needs to be published.
I have included updated documentation if my change required it.

Reviewer's checklist

Includes a Github Issue with appropriate flag or Jira ticket number without a link
Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
Automated tests cover all use cases and follow best practices for writing
Validated on all supported browsers
All VRTs are approved before the author can update Golden Hash

Manual review test cases

Descriptive Test Statement
1. Go picker component
2. Select Spectrum 2 theme.
3. Observe that the picker component do not have any borders anymore (the same as the Express theme).

Device review

Did it pass in Desktop?
Did it pass in (emulated) Mobile?
Did it pass in (emulated) iPad?

changeset-bot · 2025-09-16T13:17:47Z

🦋 Changeset detected

Latest commit: 1c1fd16

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

This PR includes changesets to release 84 packages

Name	Type
@spectrum-web-components/picker	Patch
@spectrum-web-components/action-menu	Patch
@spectrum-web-components/custom-vars-viewer	Patch
example-project-rollup	Patch
example-project-webpack	Patch
@spectrum-web-components/story-decorator	Patch
@spectrum-web-components/bundle	Patch
@spectrum-web-components/breadcrumbs	Patch
documentation	Patch
@spectrum-web-components/eslint-plugin	Patch
@spectrum-web-components/accordion	Patch
@spectrum-web-components/action-bar	Patch
@spectrum-web-components/action-button	Patch
@spectrum-web-components/action-group	Patch
@spectrum-web-components/alert-banner	Patch
@spectrum-web-components/alert-dialog	Patch
@spectrum-web-components/asset	Patch
@spectrum-web-components/avatar	Patch
@spectrum-web-components/badge	Patch
@spectrum-web-components/button-group	Patch
@spectrum-web-components/button	Patch
@spectrum-web-components/card	Patch
@spectrum-web-components/checkbox	Patch
@spectrum-web-components/clear-button	Patch
@spectrum-web-components/close-button	Patch
@spectrum-web-components/coachmark	Patch
@spectrum-web-components/color-area	Patch
@spectrum-web-components/color-field	Patch
@spectrum-web-components/color-handle	Patch
@spectrum-web-components/color-loupe	Patch
@spectrum-web-components/color-slider	Patch
@spectrum-web-components/color-wheel	Patch
@spectrum-web-components/combobox	Patch
@spectrum-web-components/contextual-help	Patch
@spectrum-web-components/dialog	Patch
@spectrum-web-components/divider	Patch
@spectrum-web-components/dropzone	Patch
@spectrum-web-components/field-group	Patch
@spectrum-web-components/field-label	Patch
@spectrum-web-components/help-text	Patch
@spectrum-web-components/icon	Patch
@spectrum-web-components/icons-ui	Patch
@spectrum-web-components/icons-workflow	Patch
@spectrum-web-components/icons	Patch
@spectrum-web-components/iconset	Patch
@spectrum-web-components/illustrated-message	Patch
@spectrum-web-components/infield-button	Patch
@spectrum-web-components/link	Patch
@spectrum-web-components/menu	Patch
@spectrum-web-components/meter	Patch
@spectrum-web-components/modal	Patch
@spectrum-web-components/number-field	Patch
@spectrum-web-components/overlay	Patch
@spectrum-web-components/picker-button	Patch
@spectrum-web-components/popover	Patch
@spectrum-web-components/progress-bar	Patch
@spectrum-web-components/progress-circle	Patch
@spectrum-web-components/radio	Patch
@spectrum-web-components/search	Patch
@spectrum-web-components/sidenav	Patch
@spectrum-web-components/slider	Patch
@spectrum-web-components/split-view	Patch
@spectrum-web-components/status-light	Patch
@spectrum-web-components/swatch	Patch
@spectrum-web-components/switch	Patch
@spectrum-web-components/table	Patch
@spectrum-web-components/tabs	Patch
@spectrum-web-components/tags	Patch
@spectrum-web-components/textfield	Patch
@spectrum-web-components/thumbnail	Patch
@spectrum-web-components/toast	Patch
@spectrum-web-components/tooltip	Patch
@spectrum-web-components/top-nav	Patch
@spectrum-web-components/tray	Patch
@spectrum-web-components/underlay	Patch
@spectrum-web-components/vrt-compare	Patch
@spectrum-web-components/base	Patch
@spectrum-web-components/grid	Patch
@spectrum-web-components/opacity-checkerboard	Patch
@spectrum-web-components/reactive-controllers	Patch
@spectrum-web-components/shared	Patch
@spectrum-web-components/styles	Patch
@spectrum-web-components/theme	Patch
@spectrum-web-components/truncated	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 · 2025-09-16T13:19:55Z

📚 Branch Preview

🔍 Visual Regression Test Results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

Deployed to Azure Blob Storage: pr-5733

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

github-actions · 2025-09-16T13:24:08Z

Tachometer results

Chrome

picker permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	596 kB	448.58ms - 452.68ms	-	faster ✔ 13% - 14% 69.58ms - 75.49ms
branch	570 kB	521.03ms - 525.29ms	slower ❌ 15% - 17% 69.58ms - 75.49ms	-

Firefox

picker permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	596 kB	1003.33ms - 1029.75ms	-	faster ✔ 3% - 6% 36.56ms - 66.20ms
branch	570 kB	1061.20ms - 1074.64ms	slower ❌ 4% - 7% 36.56ms - 66.20ms	-

.circleci/config.yml

tools/styles/tokens-v2/system-theme-bridge.css

rise-erpelding

Thanks @iuliauta, this looks great!

One thing I want to call out that I think is out of scope for this work, is that there's a diff in the VRTs for the position of the text (just for L, it looks like), where there's more spacing above the text where we increased the border width, so it sits lower within the Picker.

I'm wondering if part of the issue is that we're not calculating the border-width out of the spacing like we do with button, for instance doing something like:

--spectrum-picker-spacing-edge-to-text: calc(var(--spectrum-component-edge-to-text-100) - var(--spectrum-picker-border-width));

This would help keep the spacing consistent even if the border width changes.

All of that said though, since the fix would need to happen outside of the theme files, I think it's out of scope, but I wanted to explain what I think might be happening here.

iuliauta · 2025-09-22T10:30:21Z

Spectrum | Light | Medium | LTR

Thanks @iuliauta, this looks great!

One thing I want to call out that I think is out of scope for this work, is that there's a diff in the VRTs for the position of the text (just for L, it looks like), where there's more spacing above the text where we increased the border width, so it sits lower within the Picker.
I'm wondering if part of the issue is that we're not calculating the border-width out of the spacing like we do with [button](https://github.com/adobe/spectrum-web-components/blob/main/packages/button/src/spectrum-button.css#L76), for instance doing something like:
--spectrum-picker-spacing-edge-to-text: calc(var(--spectrum-component-edge-to-text-100) - var(--spectrum-picker-border-width));
This would help keep the spacing consistent even if the border width changes.

All of that said though, since the fix would need to happen outside of the theme files, I think it's out of scope, but I wanted to explain what I think might be happening here.

Thank you for pointing it out! I tried to see how this change will influence the themes and it seems that it impacts spectrum-1 theme and I suggest to address this improvement in another PR, unrelated to CSS themes PRs (as you already mentioned in your message).

…the code review request

…ng because it breaks s1 theme

iuliauta self-assigned this Sep 16, 2025

iuliauta added the Component: Picker label Sep 16, 2025

iuliauta marked this pull request as ready for review September 17, 2025 07:37

iuliauta requested a review from a team as a code owner September 17, 2025 07:37

iuliauta added the ready-for-review label Sep 17, 2025

iuliauta mentioned this pull request Sep 17, 2025

fix(sp-textfield): update border width and color for s2/Express #5734

Merged

14 tasks

Rajdeepc requested changes Sep 17, 2025

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

marissahuysentruyt reviewed Sep 17, 2025

View reviewed changes

tools/styles/tokens-v2/system-theme-bridge.css Outdated Show resolved Hide resolved

rise-erpelding reviewed Sep 17, 2025

View reviewed changes

tools/styles/tokens-v2/system-theme-bridge.css Outdated Show resolved Hide resolved

rise-erpelding approved these changes Sep 18, 2025

View reviewed changes

Rajdeepc approved these changes Sep 22, 2025

View reviewed changes

rise-erpelding added ready-for-merge Will auto-update until merged and removed ready-for-review labels Sep 23, 2025

marissahuysentruyt changed the title ~~feat(sp-picker): s2 theme update - no borders for picker~~ fix(sp-picker): s2 theme update - no borders for picker Sep 23, 2025

marissahuysentruyt force-pushed the iuta/improve-picker-spectrum-two branch 3 times, most recently from 24de739 to e1824f2 Compare September 23, 2025 20:54

marissahuysentruyt added do not merge and removed ready-for-merge Will auto-update until merged labels Sep 23, 2025

marissahuysentruyt force-pushed the iuta/improve-picker-spectrum-two branch from 660b721 to 1d37e1f Compare September 24, 2025 18:43

castastrophe approved these changes Sep 24, 2025

View reviewed changes

castastrophe force-pushed the iuta/improve-picker-spectrum-two branch from 1d37e1f to 7d7f677 Compare September 24, 2025 20:49

castastrophe enabled auto-merge (squash) September 24, 2025 20:50

castastrophe disabled auto-merge September 24, 2025 20:50

iuliauta added 2 commits September 24, 2025 16:52

feat(sp-picker): s2 theme update - no borders for picker

e9ac78e

feat(sp-picker): add changeset

5cb18bd

iuliauta added 5 commits September 24, 2025 16:52

feat(sp-picker): change border width to 0px for s2

2d4e5fc

feat(sp-picker): revert the border width setting to 0px according to …

142bb54

…the code review request

feat(sp-picker): set border width to spectrum-border-width-200 for s2

e6089db

feat(sp-picker): calculating the border-width out of the spacing

3482ffe

feat(sp-picker): revert calculating the border-width out of the spaci…

100f01a

…ng because it breaks s1 theme

marissahuysentruyt force-pushed the iuta/improve-picker-spectrum-two branch from 7d7f677 to 100f01a Compare September 24, 2025 20:53

chore: update golden hash

1c1fd16

marissahuysentruyt added ready-for-merge Will auto-update until merged and removed do not merge labels Sep 24, 2025

graynorton merged commit dbba861 into main Sep 24, 2025
24 checks passed

graynorton deleted the iuta/improve-picker-spectrum-two branch September 24, 2025 22:54

castastrophe mentioned this pull request Sep 25, 2025

fix(sp-textfield): update border width and color for s2/Express #5756

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(sp-picker): s2 theme update - no borders for picker #5733

fix(sp-picker): s2 theme update - no borders for picker #5733

iuliauta commented Sep 16, 2025 •

edited by marissahuysentruyt

Loading

Uh oh!

changeset-bot bot commented Sep 16, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 16, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 16, 2025 •

edited

Loading

picker permalink

basic-test

picker permalink

basic-test

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rise-erpelding left a comment •

edited

Loading

Uh oh!

iuliauta commented Sep 22, 2025

Uh oh!

Uh oh!

Uh oh!

fix(sp-picker): s2 theme update - no borders for picker #5733

fix(sp-picker): s2 theme update - no borders for picker #5733

Conversation

iuliauta commented Sep 16, 2025 • edited by marissahuysentruyt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and context

Related issue(s)

Screenshots (if appropriate)

Author's checklist

Reviewer's checklist

Manual review test cases

Device review

Uh oh!

changeset-bot bot commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📚 Branch Preview

🔍 Visual Regression Test Results

Uh oh!

github-actions bot commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tachometer results

picker permalink

basic-test

picker permalink

basic-test

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rise-erpelding left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iuliauta commented Sep 22, 2025

Uh oh!

Uh oh!

Uh oh!

iuliauta commented Sep 16, 2025 •

edited by marissahuysentruyt

Loading

changeset-bot bot commented Sep 16, 2025 •

edited

Loading

github-actions bot commented Sep 16, 2025 •

edited

Loading

github-actions bot commented Sep 16, 2025 •

edited

Loading

rise-erpelding left a comment •

edited

Loading