Skip to content

feat(picker): refactoring template markup, testing token passthroughs #3792

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

Open
wants to merge 7 commits into
base: spectrum-two
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/open-squids-refuse.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@spectrum-css/picker": major
---

### S2 Picker Component Refactor

- Refactors Picker component to use proper custom property naming conventions
- Adds size-specific animation distances for Popover component
- Improves component structure with proper class by renaming `spectrum-Picker` to `spectrum-Picker-button`
- `spectrum-Picker` now encapsulates help text, label, and popover components
- Adds `flex-shrink` to progress circle for better layout control when truncation and loading is visible
- Updates Popover animation distance to use `spectrum-Picker` custom properties

#### New mods

`--mod-picker-popover-animation-distance`
67 changes: 35 additions & 32 deletions components/picker/dist/metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
"sourceFile": "index.css",
"selectors": [
".spectrum-Picker",
".spectrum-Picker + .spectrum-Popover--bottom-start.is-open",
".spectrum-Picker .spectrum-Picker-icon",
".spectrum-Picker .spectrum-Picker-label",
".spectrum-Picker .spectrum-ProgressCircle",
Expand All @@ -13,60 +12,64 @@
".spectrum-Picker--quiet:focus-visible:after",
".spectrum-Picker--sideLabel",
".spectrum-Picker--sizeL",
".spectrum-Picker--sizeL + .spectrum-Popover--bottom-start.is-open",
".spectrum-Picker--sizeS",
".spectrum-Picker--sizeS + .spectrum-Popover--bottom-start.is-open",
".spectrum-Picker--sizeXL",
".spectrum-Picker--sizeXL + .spectrum-Popover--bottom-start.is-open",
".spectrum-Picker-button",
".spectrum-Picker-button.is-disabled",
".spectrum-Picker-button.is-invalid.is-keyboardFocused:not(:disabled, .is-disabled)",
".spectrum-Picker-button.is-invalid.is-open:not(:disabled, .is-disabled)",
".spectrum-Picker-button.is-invalid.is-open:not(:disabled, .is-disabled):hover",
".spectrum-Picker-button.is-invalid:not(:disabled, .is-disabled)",
".spectrum-Picker-button.is-invalid:not(:disabled, .is-disabled) .spectrum-Picker-validationIcon",
".spectrum-Picker-button.is-invalid:not(:disabled, .is-disabled):active",
".spectrum-Picker-button.is-invalid:not(:disabled, .is-disabled):focus-visible",
".spectrum-Picker-button.is-invalid:not(:disabled, .is-disabled):hover",
".spectrum-Picker-button.is-keyboardFocused",
".spectrum-Picker-button.is-keyboardFocused .spectrum-Picker-label.is-placeholder",
".spectrum-Picker-button.is-keyboardFocused .spectrum-Picker-menuIcon",
".spectrum-Picker-button.is-keyboardFocused:after",
".spectrum-Picker-button.is-open:not(.spectrum-Picker--quiet, :disabled, .is-disabled, .is-loading)",
".spectrum-Picker-button.is-open:not(.spectrum-Picker--quiet, :disabled, .is-disabled, .is-loading) .spectrum-Picker-menuIcon",
".spectrum-Picker-button.is-open:not(.spectrum-Picker--quiet, :disabled, .is-disabled, .is-loading):hover",
".spectrum-Picker-button.is-open:not(.spectrum-Picker--quiet, :disabled, .is-disabled, .is-loading):hover .spectrum-Picker-menuIcon",
".spectrum-Picker-button::-moz-focus-inner",
".spectrum-Picker-button:active",
".spectrum-Picker-button:active .spectrum-Picker-label.is-placeholder",
".spectrum-Picker-button:active:after",
".spectrum-Picker-button:after",
".spectrum-Picker-button:disabled",
".spectrum-Picker-button:focus",
".spectrum-Picker-button:focus-visible",
".spectrum-Picker-button:focus-visible .spectrum-Picker-label.is-placeholder",
".spectrum-Picker-button:focus-visible .spectrum-Picker-menuIcon",
".spectrum-Picker-button:focus-visible:after",
".spectrum-Picker-button:hover",
".spectrum-Picker-button:hover .spectrum-Picker-menuIcon",
".spectrum-Picker-label",
".spectrum-Picker-label.is-placeholder",
".spectrum-Picker-label.is-placeholder:active",
".spectrum-Picker-label.is-placeholder:hover",
".spectrum-Picker-menuIcon",
".spectrum-Picker-menuIcon:active",
".spectrum-Picker-popover",
".spectrum-Picker-validationIcon",
".spectrum-Picker.is-disabled",
".spectrum-Picker.is-disabled .spectrum-Picker-icon",
".spectrum-Picker.is-disabled .spectrum-Picker-label.is-placeholder",
".spectrum-Picker.is-disabled .spectrum-Picker-menuIcon",
".spectrum-Picker.is-disabled .spectrum-Picker-validationIcon",
".spectrum-Picker.is-disabled:not(.spectrum-Picker--quiet)",
".spectrum-Picker.is-invalid.is-keyboardFocused:not(:disabled, .is-disabled)",
".spectrum-Picker.is-invalid.is-open:not(:disabled, .is-disabled)",
".spectrum-Picker.is-invalid.is-open:not(:disabled, .is-disabled):hover",
".spectrum-Picker.is-invalid:not(:disabled, .is-disabled)",
".spectrum-Picker.is-invalid:not(:disabled, .is-disabled) .spectrum-Picker-validationIcon",
".spectrum-Picker.is-invalid:not(:disabled, .is-disabled):active",
".spectrum-Picker.is-invalid:not(:disabled, .is-disabled):focus-visible",
".spectrum-Picker.is-invalid:not(:disabled, .is-disabled):hover",
".spectrum-Picker.is-keyboardFocused",
".spectrum-Picker.is-keyboardFocused .spectrum-Picker-label.is-placeholder",
".spectrum-Picker.is-keyboardFocused .spectrum-Picker-menuIcon",
".spectrum-Picker.is-keyboardFocused:after",
".spectrum-Picker.is-loading",
".spectrum-Picker.is-loading .spectrum-Picker-menuIcon",
".spectrum-Picker.is-open:not(.spectrum-Picker--quiet, :disabled, .is-disabled, .is-loading)",
".spectrum-Picker.is-open:not(.spectrum-Picker--quiet, :disabled, .is-disabled, .is-loading) .spectrum-Picker-menuIcon",
".spectrum-Picker.is-open:not(.spectrum-Picker--quiet, :disabled, .is-disabled, .is-loading):hover",
".spectrum-Picker.is-open:not(.spectrum-Picker--quiet, :disabled, .is-disabled, .is-loading):hover .spectrum-Picker-menuIcon",
".spectrum-Picker::-moz-focus-inner",
".spectrum-Picker:active",
".spectrum-Picker:active .spectrum-Picker-label.is-placeholder",
".spectrum-Picker:active:after",
".spectrum-Picker:after",
".spectrum-Picker:disabled",
".spectrum-Picker:disabled .spectrum-Picker-icon",
".spectrum-Picker:disabled .spectrum-Picker-label.is-placeholder",
".spectrum-Picker:disabled .spectrum-Picker-menuIcon",
".spectrum-Picker:disabled .spectrum-Picker-validationIcon",
".spectrum-Picker:disabled:not(.spectrum-Picker--quiet)",
".spectrum-Picker:focus",
".spectrum-Picker:focus-visible",
".spectrum-Picker:focus-visible .spectrum-Picker-label.is-placeholder",
".spectrum-Picker:focus-visible .spectrum-Picker-menuIcon",
".spectrum-Picker:focus-visible:after",
".spectrum-Picker:hover",
".spectrum-Picker:hover .spectrum-Picker-menuIcon",
".spectrum-Picker:lang(ja) .spectrum-Picker-label",
".spectrum-Picker:lang(ko) .spectrum-Picker-label",
".spectrum-Picker:lang(zh) .spectrum-Picker-label"
Expand Down Expand Up @@ -123,6 +126,7 @@
"--mod-picker-min-inline-size-quiet",
"--mod-picker-placeholder-font-style",
"--mod-picker-placeholder-font-weight",
"--mod-picker-popover-animation-distance",
"--mod-picker-spacing-bottom-to-text",
"--mod-picker-spacing-edge-to-disclosure-icon",
"--mod-picker-spacing-edge-to-disclosure-icon-quiet",
Expand All @@ -131,7 +135,6 @@
"--mod-picker-spacing-icon-to-disclosure-icon",
"--mod-picker-spacing-label-to-picker",
"--mod-picker-spacing-label-to-picker-quiet",
"--mod-picker-spacing-picker-to-popover",
"--mod-picker-spacing-starting-icon-to-text",
"--mod-picker-spacing-text-to-icon-inline-end",
"--mod-picker-spacing-top-to-alert-icon",
Expand Down Expand Up @@ -196,7 +199,6 @@
"--spectrum-picker-spacing-icon-to-disclosure-icon",
"--spectrum-picker-spacing-label-to-picker",
"--spectrum-picker-spacing-label-to-picker-quiet",
"--spectrum-picker-spacing-picker-to-popover",
"--spectrum-picker-spacing-starting-icon-to-text",
"--spectrum-picker-spacing-text-to-icon-inline-end",
"--spectrum-picker-spacing-top-to-alert-icon",
Expand Down Expand Up @@ -302,7 +304,8 @@
"passthroughs": [
"--mod-button-animation-duration",
"--mod-button-font-family",
"--mod-button-line-height"
"--mod-button-line-height",
"--mod-popover-animation-distance"
],
"high-contrast": [
"--highcontrast-picker-background-color",
Expand Down
26 changes: 8 additions & 18 deletions components/picker/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,7 @@
--spectrum-picker-focus-indicator-thickness: var(--spectrum-focus-indicator-thickness);
--spectrum-picker-focus-indicator-color: var(--spectrum-focus-indicator-color);

& + .spectrum-Popover--bottom-start.is-open {
--spectrum-picker-spacing-picker-to-popover: var(--spectrum-component-to-menu-medium);
}
--mod-picker-popover-animation-distance: var(--spectrum-component-to-menu-medium);
}

.spectrum-Picker--sizeS {
Expand All @@ -124,10 +122,7 @@
--spectrum-picker-spacing-top-to-progress-circle: var(--spectrum-field-top-to-progress-circle-small);
--spectrum-picker-spacing-top-to-disclosure-icon: var(--spectrum-field-top-to-disclosure-icon-75);
--spectrum-picker-spacing-edge-to-disclosure-icon: var(--spectrum-field-end-edge-to-disclosure-icon-75);

& + .spectrum-Popover--bottom-start.is-open {
--spectrum-picker-spacing-picker-to-popover: var(--spectrum-component-to-menu-small);
}
--mod-picker-popover-animation-distance: var(--spectrum-component-to-menu-small);
}

.spectrum-Picker--sizeL {
Expand All @@ -146,10 +141,7 @@
--spectrum-picker-spacing-top-to-progress-circle: var(--spectrum-field-top-to-progress-circle-large);
--spectrum-picker-spacing-top-to-disclosure-icon: var(--spectrum-field-top-to-disclosure-icon-200);
--spectrum-picker-spacing-edge-to-disclosure-icon: var(--spectrum-field-end-edge-to-disclosure-icon-200);

& + .spectrum-Popover--bottom-start.is-open {
--spectrum-picker-spacing-picker-to-popover: var(--spectrum-component-to-menu-large);
}
--mod-picker-popover-animation-distance: var(--spectrum-component-to-menu-large);
}

.spectrum-Picker--sizeXL {
Expand All @@ -168,10 +160,7 @@
--spectrum-picker-spacing-top-to-progress-circle: var(--spectrum-field-top-to-progress-circle-extra-large);
--spectrum-picker-spacing-top-to-disclosure-icon: var(--spectrum-field-top-to-disclosure-icon-300);
--spectrum-picker-spacing-edge-to-disclosure-icon: var(--spectrum-field-end-edge-to-disclosure-icon-300);

& + .spectrum-Popover--bottom-start.is-open {
--spectrum-picker-spacing-picker-to-popover: var(--spectrum-component-to-menu-extra-large);
}
--mod-picker-popover-animation-distance: var(--spectrum-component-to-menu-extra-large);
}

.spectrum-Picker--quiet {
Expand All @@ -190,7 +179,7 @@
--spectrum-picker-spacing-label-to-picker: var(--mod-picker-spacing-label-to-picker-quiet, var(--spectrum-picker-spacing-label-to-picker-quiet));
}

.spectrum-Picker {
.spectrum-Picker-button {
@extend %spectrum-BaseButton;

display: flex;
Expand Down Expand Up @@ -408,10 +397,11 @@
margin-block-start: calc(var(--mod-picker-spacing-top-to-progress-circle, var(--spectrum-picker-spacing-top-to-progress-circle)) - var(--mod-picker-border-width, var(--spectrum-picker-border-width)));
margin-block-end: calc(var(--mod-picker-spacing-top-to-progress-circle, var(--spectrum-picker-spacing-top-to-progress-circle)) - var(--mod-picker-border-width, var(--spectrum-picker-border-width)));
margin-inline-end: var(--mod-picker-spacing-icon-to-disclosure-icon, var(--spectrum-picker-spacing-icon-to-disclosure-icon));
flex-shrink: 0;
}

.spectrum-Picker + .spectrum-Popover--bottom-start.is-open {
transform: translateY(var(--mod-picker-spacing-picker-to-popover, var(--spectrum-picker-spacing-picker-to-popover)));
.spectrum-Picker-popover {
--mod-popover-animation-distance: var(--mod-picker-popover-animation-distance, var(--spectrum-component-to-menu-medium));
}

.spectrum-Picker--quiet {
Expand Down
3 changes: 2 additions & 1 deletion components/picker/stories/picker.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export default {
...(IconStories?.argTypes?.iconName ?? {}),
name: "Icon",
description: "Optional workflow icon that appears before the value/placeholder text within the picker.",
if: false,
if: { arg: "showWorkflowIcon", eq: true },
},
isQuiet: {
...isQuiet,
Expand Down Expand Up @@ -117,6 +117,7 @@ export default {
placeholder: "Select a country",
helpText: "",
currentValue: "",
contentIconName: "Image",
showWorkflowIcon: false,
isQuiet: false,
isKeyboardFocused: false,
Expand Down
32 changes: 15 additions & 17 deletions components/picker/stories/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@ import "../index.css";
*/
export const Picker = ({
rootClass = "spectrum-Picker",
id = getRandomId("picker"),
id = getRandomId("picker-button"),
size = "m",
labelPosition = "top",
placeholder,
currentValue,
contentIconName,
isQuiet = false,
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt May 23, 2025

Choose a reason for hiding this comment

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

I'm not totally sure where to put this comment, but I think it's related to the removal of this isQuiet arg.

I noticed on the docs page that the Quiet Invalid story has borders. I'm not certain where that code lives however...

If I put the .spectrum-Picker--quiet on the button itself, the borders are removed as expected.

Preview:
Screenshot 2025-05-23 at 11 30 21 AM

Other recent PR preview (from the table migration):
Screenshot 2025-05-23 at 11 30 29 AM

isKeyboardFocused = false,
showWorkflowIcon = false,
isOpen = false,
Expand All @@ -42,11 +40,7 @@ export const Picker = ({
return html`
<button
class=${classMap({
[rootClass]: true,
[`${rootClass}--size${size?.toUpperCase()}`]:
typeof size !== "undefined",
[`${rootClass}--quiet`]: isQuiet,
[`${rootClass}--sideLabel`]: labelPosition == "side",
[`${rootClass}-button`]: true,
["is-invalid"]: isInvalid,
["is-open"]: isOpen,
["is-loading"]: isLoading,
Expand All @@ -65,18 +59,11 @@ export const Picker = ({
}}
aria-labelledby=${ifDefined(ariaLabeledBy)}
>
${when(contentIconName, () =>
Icon({
iconName: contentIconName,
size,
customClasses: ["spectrum-Picker-icon"],
}, context))
}
${when(showWorkflowIcon, () =>
Icon({
size,
setName: "workflow",
iconName: "Image",
iconName: contentIconName,
customClasses: [`${rootClass}-icon`],
}, context)
)}
Expand Down Expand Up @@ -133,6 +120,7 @@ export const Template = ({
isInvalid = false,
isDisabled = false,
showWorkflowIcon = false,
contentIconName,
isHovered = false,
isActive = false,
isKeyboardFocused = false,
Expand All @@ -155,6 +143,7 @@ export const Template = ({
isQuiet,
currentValue,
showWorkflowIcon,
contentIconName,
isOpen,
isInvalid,
isDisabled,
Expand All @@ -176,6 +165,7 @@ export const Template = ({
content: popoverContent,
size,
customStyles: customPopoverStyles,
customClasses: [`${rootClass}-popover`],
popoverWrapperStyles: {
"display": "block",
},
Expand All @@ -189,15 +179,23 @@ export const Template = ({
isDisabled,
}, context) : "";


const markup = html`
<div
class=${classMap({
[rootClass]: true,
[`${rootClass}--size${size?.toUpperCase()}`]:
typeof size !== "undefined",
[`${rootClass}--quiet`]: isQuiet,
[`${rootClass}--sideLabel`]: labelPosition == "side",
})}
style=${styleMap({
position: "relative",
display: "inline-block",
...(labelPosition == "side") && {
display: "flex",
flexWrap: "nowrap",
}
},
})}
>
${when(label, () =>
Expand Down
Loading