Editor: Resolve Publish button busy state styling regression #16303
Conversation
aduth
added
[Type] Bug
aduth
requested review from
ajitbohra,
chrisvanpatten,
gziolo,
jorgefilipecosta,
talldan and
youknowriad
as code owners
| @@ -28,7 +28,7 @@ export function Button( props, ref ) { | |||
|
|
|||
| const classes = classnames( 'components-button', className, { | |||
| 'is-button': isDefault || isPrimary || isLarge || isSmall, | |||
| 'is-default': isDefault || isLarge || isSmall, | |||
| 'is-default': ! isPrimary && ( isDefault || isLarge || isSmall ), | |||
There was a problem hiding this comment.
Would the condition isDefault || ! isPrimary && ( isLarge || isSmall )also work? I guess when someone passes isDefault there is an expectation the is-default class is added for sure. Otherwise, maybe we can add a small note on https://github.com/WordPress/gutenberg//blob/dd9bf47ff07680d672efc5b31abac268e2cb69c0/packages/components/src/button/README.md#L128 saying the prop is ignored if isPrimary prop is also passed.
There was a problem hiding this comment.
Hm, well, the root of the problem is that nobody should ever apply both isPrimary and isDefault, so your expectation may be valid in considering isDefault alone, but not in combination with isPrimary.
That being said, your suggestion still seems like a slight improvement in sensibly mapping isDefault and isPrimary to their respective classes, irrespective the styling conflict which would likely ensue from this choice.
|
I think the button is supposed to be animated. I'm not seeing the animation? |
|
nevermind, I had the "disable animations" plugin enabled :) |
Fixes #15029
This pull request seeks to resolve an issue where the Publish button (or any busy, disabled button) would display with incorrect styling.
Implementation Notes:
There are two separate changes which independently resolve the issue, but which together help reduce redundancy and assure broader applicability:
is-defaultstyling if it is is a primary button, since the "default" styling is the gray background (which primary styling is intended to supersede)is-defaultwas applied) because all style properties ofis-large(source) are substituted in post editor header styles ([1], [2]). This also ensures consistency with the "Toggle" variant of the same button, which already doesn't applyisLargeand, aside from text of the button is intended to appear visually identical.Testing Instructions:
Verify that upon Publishing a post, the intermediate state of the save request displays as shown in the "After" screenshot above.