Skip to content

fix(stepper): clarify mod definitions #3670

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

fix(stepper): clarify mod definitions #3670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8aa2a4
fix(stepper): remove transparent border color
marissahuysentruyt Apr 15, 2025
ed0a922
fix(stepper): ensure mod passthroughs can still be surfaced
marissahuysentruyt Apr 15, 2025
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
9 changes: 5 additions & 4 deletions components/stepper/dist/metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@
".spectrum-Stepper.spectrum-Stepper--sizeS",
".spectrum-Stepper.spectrum-Stepper--sizeXL",
".spectrum-Stepper:before",
".spectrum-Stepper:hover:not(.is-invalid, .is-disabled, .is-focused)",
".spectrum-Stepper:not(.is-disabled) .is-focused",
".spectrum-Stepper:not(.is-disabled) .is-focused:hover",
".spectrum-Stepper:hover:not(.is-invalid, .is-disabled, .is-focused, .is-keyboardFocused)",
".spectrum-Stepper:not(.is-disabled):focus",
".spectrum-Stepper:not(.is-disabled):focus-visible",
".spectrum-Stepper:not(.is-disabled):focus:hover",
Expand Down Expand Up @@ -73,6 +71,7 @@
"--mod-stepper-focus-indicator-color",
"--mod-stepper-focus-indicator-gap",
"--mod-stepper-focus-indicator-visibility",
"--mod-stepper-focus-indicator-visibility-keyboard-focus",
"--mod-stepper-focus-indicator-width",
"--mod-stepper-height",
"--mod-stepper-min-width-multiplier",
Expand Down Expand Up @@ -101,7 +100,6 @@
"--spectrum-stepper-buttons-background-color-disabled",
"--spectrum-stepper-buttons-border-color",
"--spectrum-stepper-buttons-border-color-focus",
"--spectrum-stepper-buttons-border-color-focus-hover",
"--spectrum-stepper-buttons-border-color-hover",
"--spectrum-stepper-buttons-border-color-keyboard-focus",
"--spectrum-stepper-buttons-border-style",
Expand Down Expand Up @@ -184,7 +182,10 @@
"--mod-textfield-background-color",
"--mod-textfield-border-color",
"--mod-textfield-border-color-disabled",
"--mod-textfield-border-color-focus",
"--mod-textfield-border-color-hover",
"--mod-textfield-border-color-invalid-hover",
"--mod-textfield-border-color-keyboard-focus",
"--mod-textfield-border-width",
"--mod-textfield-focus-indicator-color",
"--mod-textfield-focus-indicator-width",
Expand Down
27 changes: 16 additions & 11 deletions components/stepper/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@

&.is-keyboardFocused {
/* quiet focus indicator only on bottom border */
--mod-stepper-focus-indicator-visibility: visible;
--mod-stepper-focus-indicator-visibility: var(--mod-stepper-focus-indicator-visibility-keyboard-focus, visible);
}
}

Expand All @@ -170,29 +170,28 @@
--mod-textfield-border-color-disabled: var(--spectrum-stepper-border-color-disabled);
}

&:hover:not(.is-invalid, .is-disabled, .is-focused) {
&:hover:not(.is-invalid, .is-disabled, .is-focused, .is-keyboardFocused) {
--mod-infield-button-border-color: var(--mod-stepper-buttons-border-color-hover, var(--spectrum-stepper-buttons-border-color-hover));
}

&:not(.is-disabled) {
.is-focused,
&.is-focused,
Comment on lines +173 to +178
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Since number field has been tricky as of late, can we update these selectors?

I'm pretty sure both of these selector changes were updated in this PR (we deprioritized a couple of months ago, that's all).

&:focus {
--mod-infield-button-border-color: var(--highcontrast-stepper-border-color-focus, var(--mod-stepper-buttons-border-color-focus, var(--spectrum-stepper-buttons-border-color-focus)));
--mod-textfield-focus-indicator-width: 0;
--mod-textfield-focus-indicator-width: var(--mod-stepper-focus-indicator-width, 0);

&:hover {
/* stylelint-disable-next-line spectrum-tools/no-unknown-custom-properties -- allows for --spectrum-stepper-buttons-border-color-focus-hover to be defined outside of current context */
--mod-infield-button-border-color: var(--mod-stepper-buttons-border-color-focus-hover, var(--spectrum-stepper-buttons-border-color-focus-hover));
--mod-textfield-focus-indicator-width: 0;
--mod-textfield-border-color: var(--highcontrast-stepper-border-color-focus-hover, var(--mod-stepper-border-color-focus-hover, var(--spectrum-stepper-border-color-focus-hover)));
--mod-infield-button-border-color: var(--mod-stepper-buttons-border-color-focus-hover, var(--spectrum-stepper-border-color-focus-hover));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The simplest solution to this border color issue was using --spectrum-stepper-border-color-focus-hover, instead of --spectrum-stepper-buttons-border-color-focus-hover. That kept coming up undefined, even though it was in the spectrum-two.css file.

--mod-textfield-focus-indicator-width: var(--mod-stepper-focus-indicator-width, 0);
--mod-textfield-border-color-focus: var(--mod-stepper-border-color-focus, var(--spectrum-stepper-border-color-focus-hover));
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The mod for just the textfield border wasn't working, but changing it to the focus border mod did. 🤷‍♀️

}
}

&.is-keyboardFocused,
&:focus-visible {
--mod-infield-button-border-color: var(--highcontrast-stepper-border-color-keyboard-focus, var(--mod-stepper-buttons-border-color-keyboard-focus, var(--spectrum-stepper-buttons-border-color-keyboard-focus)));
--mod-textfield-focus-indicator-width: 0;
--mod-textfield-border-color: var(--highcontrast-stepper-border-color-keyboard-focus, var(--mod-stepper-border-color-keyboard-focus, var(--spectrum-stepper-border-color-keyboard-focus)));
--mod-textfield-focus-indicator-width: var(--mod-stepper-focus-indicator-width, 0);
--mod-textfield-border-color-hover: var(--highcontrast-stepper-border-color-keyboard-focus, var(--mod-stepper-border-color-keyboard-focus, var(--spectrum-stepper-border-color-keyboard-focus)));

/* keyboard focus indicator is visible */
outline: var(--spectrum-stepper-focus-indicator-width) solid;
Expand All @@ -207,14 +206,17 @@

&:hover {
--mod-infield-button-border-color: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-hover-invalid, var(--spectrum-negative-border-color-hover)));
--mod-textfield-border-color-hover: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-focus-hover-invalid, var(--spectrum-stepper-border-color-focus-hover-invalid)));
}

&.is-focused,
&:focus {
--mod-infield-button-border-color: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-focus-invalid, var(--spectrum-stepper-border-color-focus-invalid)));
--mod-textfield-border-color-focus: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-focus-invalid, var(--spectrum-stepper-border-color-focus-invalid)));

&:hover {
--mod-infield-button-border-color: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-focus-hover-invalid, var(--spectrum-stepper-border-color-focus-hover-invalid)));
--mod-textfield-border-color-invalid-hover: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-focus-hover-invalid, var(--spectrum-stepper-border-color-focus-hover-invalid)));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Another similar finding, that we were missing a textfield mod in the invalid state.

}
}

Expand Down Expand Up @@ -254,15 +256,18 @@
/* stylelint-disable-next-line max-nesting-depth -- @todo reduce complexity of selectors */
&:hover {
--mod-infield-button-border-color: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-focus-hover, var(--spectrum-stepper-border-color-focus-hover)));
--mod-textfield-border-color-focus: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-focus-hover, var(--spectrum-stepper-border-color-focus-hover)));
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

A similar situation here as described above, where this textfield mod was actually the mod we needed for the focus+hover. It was undefined when I looked through dev tools.

}
}

&.is-keyboardFocused {
--mod-infield-button-border-color: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-keyboard-focus, var(--spectrum-stepper-border-color-keyboard-focus)));
--mod-textfield-border-color-keyboard-focus: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-keyboard-focus, var(--spectrum-stepper-border-color-keyboard-focus)));
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If I'm remembering correctly, following the variables in dev tools, I noticed that this textfield mod was undefined. Adding it here fixed what seemed like a styling bug, where the buttons and textfields were behaving differently between the default & invalid variants.

Production:

Screen.Recording.2025-04-15.at.2.00.03.PM.mov

This branch:

Screen.Recording.2025-04-15.at.2.01.56.PM.mov


/* stylelint-disable-next-line max-nesting-depth -- @todo reduce complexity of selectors */
&:hover {
--mod-infield-button-border-color: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-hover, var(--spectrum-stepper-border-color-hover)));
--mod-infield-button-border-color: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-hover, var(--spectrum-stepper-border-color-keyboard-focus)));
--mod-textfield-border-color-hover: var(--highcontrast-stepper-border-color, var(--mod-stepper-border-color-hover, var(--spectrum-stepper-border-color-keyboard-focus)));
Comment on lines +269 to +270
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

These changes correct some keyboard focus hover border colors.

For a keyboard focused stepper, we were relying on the regular hover border color (which is gray-600, I believe):

Screenshot 2025-04-15 at 1 45 04 PM

It's now changed to gray-800 (just keeping the keyboard focus color even on hover), but is that correct? It felt like a bug, but that is how textfield, combobox, and picker all work. However, with this update, the default stepper now behaves like the invalid default stepper, where the keyboard focus border color does not respond to hovering or not.

Screenshot 2025-04-15 at 1 46 07 PM

}
}
}
Expand Down
1 change: 0 additions & 1 deletion components/stepper/themes/express.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
/** Invalid **/
--spectrum-stepper-border-color-invalid: transparent;
--spectrum-stepper-border-color-focus-invalid: transparent;
--spectrum-stepper-border-color-focus-hover-invalid: transparent;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TODO: this was looking like a bug on focus+hover. double check if this change is necessary any more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2025-04-15 at 2 21 37 PM

I don't have a more defined grasp on why that's showing up now, except that it's now part of the textfield mod stack, so it makes sense that it's rendering now.

--spectrum-stepper-border-color-keyboard-focus-invalid: transparent;

/** Disabled **/
Expand Down
Loading