Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
# Conflicts: # core/src/components/button/button.scss
ShaneK
left a comment
There was a problem hiding this comment.
Looks good to me, thanks for the fixes, just a couple of small follow-ups on the tests!
# Conflicts: # BREAKING.md # core/src/components/button/button.scss # core/src/themes/md/default.tokens.ts
| :host(.button-hue-bold.button-fill-#{$fill}.ion-activated) { | ||
| color: var(#{$token}-activated-color, var(#{$token}-default-color)); | ||
| } | ||
| :host(.button-hue-#{$hue}.button-fill-#{$fill}.ion-activated) { |
There was a problem hiding this comment.
In the ionic theme, the text on a hovered subtle outline or clear button darkens while it's pressed and goes back on release, for example rgb(0, 84, 233) to rgb(0, 74, 205) for primary. The merge in 392620e removed their activated tokens, so the color here falls back to the default color instead of the hover one. Bold doesn't hit this because its hover color is already the default. Which would you prefer?
- Make the subtle outline and clear hover color match the default, like bold
- Restore the activated color as the contrast color for ionic subtle outline and clear
There was a problem hiding this comment.
Went with the first option in 844c5ad. Ionic subtle outline and clear were the only buttons in any theme whose hover changed the text color, so their hover now keeps foreground, like bold. Restoring the activated color would have brought back a text color change on touch, where a press goes straight from the resting color to contrast. I also added a test that hovers each hue and fill, presses, and checks the text color stays the same. It fails on the old tokens with the colors you found.
# Conflicts: # core/src/components/button/test/a11y/button.e2e.ts
Issue number: resolves internal
What is the current behavior?
Buttons only have bold colors. Unlike chip and badge, which also have a
colorprop, there's nohueprop for muted colors.What is the new behavior?
hueprop toion-button. It reads the prop first, thenIonButton.huein the theme config, and defaults tobold, like chiphue.subtleto the recipe in every theme, with the same fills, states,semanticcolors, ripple and disabled styles asboldcontrasttextforegroundtext and a subtleshadeborder. md keeps its neutral 12% borderforegroundtextforeground, and text on an opaque fill of the color usescontrast, matchingion-textand FW-6422foregroundfor their text too, like clear. In ios, clear buttons with acoloralso keepforegroundon hover and focusion-datetimeandion-input-password-togglestaybold, since those components only style the bold tokens, so a hue from the config doesn't restyle themDoes this introduce a breaking change?
The text of
outlinebuttons inmdandiosuses the color's darkerforegroundinstead ofbase. Inios, clear buttons with acoloralso keepforegroundon hover and focus. To revert, setIonButton.hue.bold.outline.<state>.color(.semantic.<state>.colorwith acolor) for global styles, or--ion-button-hue-bold-outline-<state>-colorfor component-specific styles, to the color'sbase. Foriosclearbuttons with acolor, setIonButton.hue.bold.clear.semantic.<state>.coloror--ion-button-hue-bold-clear-semantic-<state>-colorforhoverandfocusto the color'sbaseas well.Other information
Why these choices
#D9E5FCfor primary), measured from the kit and from a nativeUIButtonin its tinted configuration. The shared palette (#EBF1FD) matches ios chips and badges, works for everycolorand follows the palette in dark mode. iOS 17's value would make subtle buttons darker than the subtle chips beside themforegroundin md and ios: the text sits on the page, not on a fill of the color, so it follows the same rule as clear. Usingbasemade outline labels lighter than clear labels in the same colorKnown gaps
light,warningandsuccesstext fails WCAG's 4.5:1 on white, the same as in subtle chips. The colors come from the shared palette, so the fix belongs there (FW-6417). Subtlelightalso fails in dark mode. The axe check leaves these colors out until FW-6417 fixes themion-buttonsand toolbar migrations can add their ownHow to test