Skip to content
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

Button Component: Update styling to avoid relying on Core Styles making it reusable #6562

Merged
merged 3 commits into from
May 21, 2018
Merged

Button Component: Update styling to avoid relying on Core Styles making it reusable #6562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d577a3
Button Component: Update styling to avoid relying on Core Styles maki…
youknowriad May 3, 2018
31527d6
Fix admin schemes by using theme colors
youknowriad May 15, 2018
2465dc4
Fix issues with hover, active, focus states.
May 16, 2018
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
12 changes: 8 additions & 4 deletions components/button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,23 @@ export function Button( props, ref ) {
isSmall,
isToggled,
isBusy,
isDefault,
isLink,
className,
disabled,
focus,
...additionalProps
} = props;

const classes = classnames( 'components-button', className, {
button: ( isPrimary || isLarge || isSmall ),
'button-primary': isPrimary,
'button-large': isLarge,
'button-small': isSmall,
'is-button': isDefault || isPrimary || isLarge || isSmall,
'is-default': isDefault || isLarge || isSmall,
'is-primary': isPrimary,
'is-large': isLarge,
'is-small': isSmall,
'is-toggled': isToggled,
'is-busy': isBusy,
'is-link': isLink,
} );

const tag = href !== undefined && ! disabled ? 'a' : 'button';
Expand Down
158 changes: 146 additions & 12 deletions components/button/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,143 @@
.components-button {
background: none;
border: none;
outline: none;
display: inline-flex;
text-decoration: none;
font-size: $default-font-size;
margin: 0;
border-radius: 0;
border: 0;
cursor: pointer;
-webkit-appearance: none;

&.is-button {
padding: 0 10px 1px;
line-height: 26px;
height: 28px;
border-radius: 3px;
white-space: nowrap;
border-width: 1px;
border-style: solid;
}

// Default button style
&.is-default {
color: #555;
border-color: #cccccc;
background: #f7f7f7;
box-shadow: inset 0 -1px 0 #cccccc;
vertical-align: top;

&:hover {
background: #fafafa;
border-color: #999;
box-shadow: inset 0 -1px 0 #999;
color: #23282d;
}

&:focus:not(:disabled):not([aria-disabled="true"]) {
background: #fafafa;
color: #23282d;
border-color: #999;
box-shadow: inset 0 -1px 0 #999,
0 0 0 2px $blue-medium-200;
}

&:active:not(:disabled):not([aria-disabled="true"]) {
background: #eee;
border-color: #999;
box-shadow: inset 0 1px 0 #999,
}

&:disabled,
&[disabled] {
color: #a0a5aa !important;
border-color: #ddd !important;
background: #f7f7f7 !important;
box-shadow: none !important;
text-shadow: 0 1px 0 #fff !important;
cursor: default;
-webkit-transform: none !important;
transform: none !important;
}
}

&.is-primary {
background: color( theme( button ) shade( 10% ) );
border-color: color( theme( button ) shade( 20% ) ) color( theme( button ) shade( 30% ) ) color( theme( primary ) shade( 30% ) );
box-shadow: inset 0 -1px 0 color( theme( button ) shade( 30% ) );
color: #fff;
text-decoration: none;
text-shadow: 0 -1px 1px color( theme( button ) shade( 30% ) ),
1px 0 1px color( theme( button ) shade( 30% ) ),
0 1px 1px color( theme( button ) shade( 30% ) ),
-1px 0 1px color( theme( button ) shade( 30% ) );

&:hover,
&:focus:not(:disabled):not([aria-disabled="true"]) {
background: color( theme( button ) shade( 15% ) );
border-color: color( theme( button ) shade( 50% ) );
box-shadow: inset 0 -1px 0 color( theme( button ) shade( 50% ) );
color: $white;
}

&:focus:not(:disabled):not([aria-disabled="true"]) {
box-shadow: inset 0 -1px 0 color( theme( button ) shade( 50% ) ),
0 0 0 2px $blue-medium-200;
}

&:active:not(:disabled):not([aria-disabled="true"]) {
background: color( theme( button ) shade( 20% ) );
border-color: color( theme( button ) shade( 50% ) );
box-shadow: inset 0 1px 0 color( theme( button ) shade( 50% ) );
vertical-align: top;
}

&:disabled,
&[disabled] {
color: color( theme( button ) tint( 30% ) ) !important;
background: color( theme( button ) shade( 30% ) ) !important;
border-color: color( theme( button ) shade( 20% ) ) !important;
box-shadow: none !important;
text-shadow: 0 -1px 0 rgba( 0, 0, 0, 0.1 ) !important;
cursor: default;
}

&.is-busy,
&.is-primary.is-busy[disabled] {
color: $white !important;
background-size: 100px 100% !important;
background-image: repeating-linear-gradient( -45deg, theme( primary ), color( theme( primary ) shade( 50% ) ) 11px, color( theme( primary ) shade( 50% ) ) 10px, theme( primary ) 20px) !important;
border-color: color( theme( primary ) shade( 50% ) ) !important;
}
}

/* Buttons that look like links, for a cross of good semantics with the visual */
&.is-link {
margin: 0;
Copy link
Member

@aduth aduth Jun 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Regression: This selector is more specific than (well, equal specific but out of order from) and thus overrides the margin applied from .components-color-palette_clear:

gutenberg/components/color-palette/style.scss

Line 9 in 94889af

margin-right: 20px;

v4.9.2 #6562
v4.9.2 #6562

Copy link
Member

Choose a reason for hiding this comment

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

Honestly, we shouldn't need margin here for the button. The color palette panel shouldn't be doing a negative margin. It should have a container and spread the options via display: flex & align-items: space-between

padding: 0;
box-shadow: none;
border: 0;
border-radius: 0;
background: none;
outline: none;
cursor: pointer;
text-align: left;
/* Mimics the default link style in common.css */
color: #0073aa;
text-decoration: underline;
transition-property: border, background, color;
transition-duration: .05s;
transition-timing-function: ease-in-out;

&:hover, &:active {
color: #00a0d2;
}

&:focus {
color: #124964;
box-shadow:
0 0 0 1px #5b9dd9,
0 0 2px 1px rgba(30, 140, 190, .8);
}
}

&:active {
color: currentColor;
Expand All @@ -30,16 +163,17 @@
opacity: 1;
}

&.button-primary.is-busy,
&.button-primary.is-busy[disabled] {
color: $white !important;
background-size: 100px 100% !important;
background-image: repeating-linear-gradient( -45deg, theme( primary ), color( theme( primary ) shade( 50% ) ) 11px, color( theme( primary ) shade( 50% ) ) 10px, theme( primary ) 20px) !important;
border-color: color( theme( primary ) shade( 50% ) ) !important;
&.is-large {
height: 30px;
line-height: 28px;
padding: 0 12px 2px;
}

.wp-core-ui.gutenberg-editor-page & {
font-size: $default-font-size;
&.is-small {
height: 24px;
line-height: 22px;
padding: 0 8px 1px;
font-size: 11px;
}
}

Expand Down
29 changes: 15 additions & 14 deletions components/button/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ describe( 'Button', () => {
it( 'should render a button element with only one class', () => {
const button = shallow( <Button /> );
expect( button.hasClass( 'components-button' ) ).toBe( true );
expect( button.hasClass( 'button' ) ).toBe( false );
expect( button.hasClass( 'button-large' ) ).toBe( false );
expect( button.hasClass( 'button-primary' ) ).toBe( false );
expect( button.hasClass( 'is-large' ) ).toBe( false );
expect( button.hasClass( 'is-primary' ) ).toBe( false );
expect( button.hasClass( 'is-toggled' ) ).toBe( false );
expect( button.prop( 'disabled' ) ).toBeUndefined();
expect( button.prop( 'type' ) ).toBe( 'button' );
Expand All @@ -32,29 +31,31 @@ describe( 'Button', () => {

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

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

Expand Down
8 changes: 5 additions & 3 deletions components/color-palette/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { __, sprintf } from '@wordpress/i18n';
* Internal dependencies
*/
import './style.scss';
import Button from '../button';

export default function ColorPalette( { colors, disableCustomColors = false, value, onChange } ) {
function applyOrUnset( color ) {
Expand Down Expand Up @@ -72,13 +73,14 @@ export default function ColorPalette( { colors, disableCustomColors = false, val
/>
}

<button
className="button-link components-color-palette__clear"
<Button
className="components-color-palette__clear"
type="button"
onClick={ () => onChange( undefined ) }
isLink
>
{ __( 'Clear' ) }
</button>
</Button>
</div>
);
}
14 changes: 8 additions & 6 deletions components/color-palette/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,14 @@ exports[`ColorPalette should allow disabling custom color picker 1`] = `
/>
</Tooltip>
</div>
<button
className="button-link components-color-palette__clear"
<Button
className="components-color-palette__clear"
isLink={true}
onClick={[Function]}
type="button"
>
Clear
</button>
</Button>
</div>
`;

Expand Down Expand Up @@ -213,12 +214,13 @@ exports[`ColorPalette should render a dynamic toolbar of colors 1`] = `
renderContent={[Function]}
renderToggle={[Function]}
/>
<button
className="button-link components-color-palette__clear"
<Button
className="components-color-palette__clear"
isLink={true}
onClick={[Function]}
type="button"
>
Clear
</button>
</Button>
</div>
`;
2 changes: 1 addition & 1 deletion components/color-palette/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe( 'ColorPalette', () => {
} );

test( 'should call onClick with undefined, when the clearButton onClick is triggered', () => {
const clearButton = wrapper.find( '.button-link.components-color-palette__clear' );
const clearButton = wrapper.find( '.components-color-palette__clear' );

expect( clearButton ).toHaveLength( 1 );

Expand Down
6 changes: 4 additions & 2 deletions components/date-time/time.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,16 @@ class TimePicker extends Component {
/>
{ is12Hour && <div>
<Button
className="button components-time-picker__am-button"
isDefault
className="components-time-picker__am-button"
isToggled={ am === 'AM' }
onClick={ this.updateAmPm( 'AM' ) }
>
{ __( 'AM' ) }
</Button>
<Button
className="button components-time-picker__pm-button"
isDefault
className="components-time-picker__pm-button"
isToggled={ am === 'PM' }
onClick={ this.updateAmPm( 'PM' ) }
>
Expand Down
4 changes: 2 additions & 2 deletions components/form-file-upload/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
.wp-core-ui .components-form-file-upload {
.button.button-large {
.components-form-file-upload {
.components-button.is-large {
padding: 6px 12px 2px 3px;

@include break-medium() {
Expand Down
2 changes: 1 addition & 1 deletion core-blocks/audio/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
margin-top: 0.5em;
}

.wp-block-audio .button.button-large {
.wp-block-audio .components-button.is-large {
margin-top: 0.5em;
}

Expand Down
4 changes: 2 additions & 2 deletions core-blocks/audio/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ exports[`core/audio block edit matches snapshot 1`] = `
value=""
/>
<button
class="components-button button button-large"
class="components-button is-button is-default is-large"
type="submit"
>
Use URL
Expand All @@ -49,7 +49,7 @@ exports[`core/audio block edit matches snapshot 1`] = `
class="components-form-file-upload"
>
<button
class="components-button components-icon-button wp-block-audio__upload-button button button-large"
class="components-button components-icon-button wp-block-audio__upload-button is-button is-default is-large"
type="button"
>
<svg
Expand Down
5 changes: 2 additions & 3 deletions core-blocks/block/edit-panel/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
margin: #{ $item-spacing / 2 } 0 $item-spacing;
}

// Needs specificity to override the margin-bottom set by .button
.wp-core-ui & .shared-block-edit-panel__button {
.components-button.shared-block-edit-panel__button {
margin: 0 5px 0 0;
}

Expand All @@ -44,7 +43,7 @@
margin: 0;
}

.wp-core-ui & .shared-block-edit-panel__button {
.components-button.shared-block-edit-panel__button {
margin: 0 0 0 5px;
}
}
Expand Down
Loading