Skip to content

Commit

Permalink
Editor: Resolve Publish button busy state styling regression (#16303)
Browse files Browse the repository at this point in the history
* Components: Avoid assigning default styling if explicitly primary

* Editor: Remove redundant assignment of publish button as large

* Components: Add CHANGELOG entry for busy styling fix

* Components: Always apply is-default class if isDefault prop passed

* Components: Add test case to protect regression on isPrimary, isLarge/isSmall combination
  • Loading branch information
aduth authored Jul 3, 2019
1 parent eba2fc8 commit 9052d4c
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 6 deletions.
5 changes: 4 additions & 1 deletion packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
## Next release
## Master

### New Features

- Added a new `popoverProps` prop to the `Dropdown` component which allows users of the `Dropdown` component to pass props directly to the `PopOver` component.

### Bug Fixes

- The `Button` component will no longer assign default styling (`is-default` class) when explicitly assigned as primary (the `isPrimary` prop). This should resolve potential conflicts affecting a combination of `isPrimary`, `isDefault`, and `isLarge` / `isSmall`, where the busy animation would appear with incorrect coloring.

## 8.0.0 (2019-06-12)

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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': isDefault || ( ! isPrimary && ( isLarge || isSmall ) ),
'is-primary': isPrimary,
'is-large': isLarge,
'is-small': isSmall,
Expand Down
11 changes: 8 additions & 3 deletions packages/components/src/button/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,27 @@ describe( 'Button', () => {
expect( button.type() ).toBe( 'button' );
} );

it( 'should render a button element with button-primary class', () => {
it( 'should render a button element with is-primary class', () => {
const button = shallow( <Button isPrimary /> );
expect( button.hasClass( 'is-large' ) ).toBe( false );
expect( button.hasClass( 'is-primary' ) ).toBe( true );
expect( button.hasClass( 'is-button' ) ).toBe( true );
} );

it( 'should render a button element with button-large class', () => {
it( 'should render a button element with is-large class', () => {
const button = shallow( <Button isLarge /> );
expect( button.hasClass( 'is-large' ) ).toBe( true );
expect( button.hasClass( 'is-default' ) ).toBe( true );
expect( button.hasClass( 'is-button' ) ).toBe( true );
expect( button.hasClass( 'is-primary' ) ).toBe( false );
} );

it( 'should render a button element with button-small class', () => {
it( 'should render a button element without is-default if primary', () => {
const button = shallow( <Button isPrimary isLarge /> );
expect( button.hasClass( 'is-default' ) ).toBe( false );
} );

it( 'should render a button element with is-small class', () => {
const button = shallow( <Button isSmall /> );
expect( button.hasClass( 'is-default' ) ).toBe( true );
expect( button.hasClass( 'is-button' ) ).toBe( true );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ export class PostPublishButton extends Component {
'aria-disabled': isButtonDisabled,
className: 'editor-post-publish-button',
isBusy: isSaving && isPublished,
isLarge: true,
isPrimary: true,
onClick: onClickButton,
};
Expand Down

0 comments on commit 9052d4c

Please sign in to comment.