Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ShaneK
left a comment
There was a problem hiding this comment.
Looks really good, every md value I checked lines up with the PR description! Just a few things, mainly the readonly datetime buttons going gray and the toolbar icon buttons growing to 64px.
| background: 'transparent', | ||
|
|
||
| opacity: '0.5', | ||
| color: rgba('var(--ion-text-color-rgb, 0, 0, 0)', 0.38), |
There was a problem hiding this comment.
A readonly datetime disables its Cancel, Clear and Done buttons and sets opacity: 1 so they don't look disabled, but now that md shows disabled through the color they come out at rgba(0, 0, 0, 0.38) instead of primary. Could the readonly rule reset the disabled color too? The same thing happens in ios in #31645, so a fix in datetime would cover both.
| | Disabled | the whole button at `0.5` opacity | `38%` text color, a `12%` container and no elevation | `IonButton.hue.bold.<fill>.disabled.opacity`, `.color`, `.background` and `.box.shadow` | | ||
| | Pressed ripple | `16%` | `24%` on solid, `12%` on outline and clear | `IonButton.hue.bold.<fill>.rippleEffect.opacity` | | ||
|
|
||
| Buttons in toolbars keep their previous colors when disabled and have no elevation on hover or focus. |
There was a problem hiding this comment.
This reads like every toolbar button loses the hover and focus elevation, but the none overrides are only under IonToolbar.IonButtons. A solid button placed straight in ion-toolbar still goes from elevation 2 to elevation 4 when focused. Should this say buttons in ion-buttons, or should the overrides move up to IonToolbar.IonButton?
| }; | ||
| }; | ||
|
|
||
| /** Unset colors keep the default ones */ |
There was a problem hiding this comment.
Only color and border.color fall back to the default here, background doesn't. Maybe something like "An unset color or border.color keeps the default one"?
| }, | ||
| }, | ||
|
|
||
| spinner: { |
There was a problem hiding this comment.
The comment on the spinner rules in button.scss still says only the ionic theme sizes and spaces a slotted spinner, but md sizes it now too. Could we update it to something like "The ionic and md themes size a slotted spinner, and only ionic spaces it"?
| | Spinner in a button | the spinner's own size (`28px` by default) | `1.2857em` by default (`18px` at the default size), with the other spinner sizes in their usual ratio to it | `IonButton.size.<size>.spinner.size.<spinner size>.width` and `.height` | | ||
| | Outline border | `2px`, the button's color | `1px`, the text color at `12%` | `IonButton.hue.bold.outline.default.border.width` and `.color` (`.semantic.default.border.color` with a `color`) | | ||
| | Solid elevation on hover and focus | the resting elevation | elevation 4 | `IonButton.hue.bold.solid.hover.box.shadow` and `.focus.box.shadow` | | ||
| | Disabled | the whole button at `0.5` opacity | `38%` text color, a `12%` container and no elevation | `IonButton.hue.bold.<fill>.disabled.opacity`, `.color`, `.background` and `.box.shadow` | |
There was a problem hiding this comment.
The outline row mentions the .semantic path for buttons with a color, but this row and the pressed ripple row don't, and both changed under semantic too. Could they mention it as well?
| medium: { | ||
| min: { | ||
| height: 'var(--ion-scaling-900)', | ||
| width: 'var(--ion-scaling-xxxxl)', |
There was a problem hiding this comment.
With this min width, a toolbar icon button whose icon isn't in slot="icon-only" grows from 35px to 64px, while one with slot="icon-only" stays at 48px. You can see it in the nested popover snapshots in #31637, where the popover shifts too. Leaving out the slot is pretty common in apps, so could IonToolbar.IonButtons.IonButton reset the min width, or should this go in BREAKING.md?
Issue number: resolves internal
What is the current behavior?
md buttons use values carried over from v9 instead of Material Design 2: 15.4px padding,
0.06emtracking, a 2px outline in the button's color, a 0.5 fade when disabled, and the same elevation on hover and focus as at rest.What is the new behavior?
md buttons follow Material Design 2, using the values from Material Components Web 14, Google's MD2 implementation:
0.0893emletter spacingTo support this, the recipe gains
hover.box.shadow,focus.box.shadowanddisabled.{color, border.color, box.shadow}. A theme that doesn't set the disabled colors keeps its default colors, so ios and ionic don't change.Toolbar and datetime buttons keep their current look: no elevation on hover or focus, and disabled toolbar buttons still fade their own colors.
Snapshots are in #31637.
Does this introduce a breaking change?
The
mdbutton follows Material Design 2 for its spacing, outline, elevation and disabled state. To keep a previous value, set the token for global styles, or the matching--ion-button-*CSS variable for component-specific styles. Replace<fill>withsolid,outlineorclear, and<size>withsmall,mediumorlarge.1.1em16pxIonButton.size.medium.padding.startand.end64pxIonButton.size.medium.min.width0.06em0.0893emIonButton.letter.spacing1.35em1.2857em(18pxat the default size)IonButton.size.<size>.icon.font.size0.3em0.4444em(8pxat the default size)IonButton.size.<size>.icon.leading.margin.endand.trailing.margin.start28pxby default)1.2857emby default (18pxat the default size), with the other spinner sizes in their usual ratio to itIonButton.size.<size>.spinner.size.<spinner size>.widthand.height2px, the button's color1px, the text color at12%IonButton.hue.bold.outline.default.border.widthand.color(.semantic.default.border.colorwith acolor)IonButton.hue.bold.solid.hover.box.shadowand.focus.box.shadow0.5opacity38%text color, a12%container and no elevationIonButton.hue.bold.<fill>.disabled.opacity,.color,.backgroundand.box.shadow16%24%on solid,12%on outline and clearIonButton.hue.bold.<fill>.rippleEffect.opacityButtons in toolbars keep their previous colors when disabled and have no elevation on hover or focus.
Other information
Not matched to MD2 on purpose
rounddefault from feat(button): set default shape to soft for ios and round for md and ionic themes #29404 staysKnown gaps
TODO(FW-6417)). The lighter ripple helps, but the text still falls short of WCAG's 4.5:1. The fix belongs to the dark paletteHow to test