Skip to content

Commit

Permalink
Components: Allow to apply isPressed prop to Button and SVG component…
Browse files Browse the repository at this point in the history
…s to indicate the state of the button (#17748)

* Components: Try fixing the active state of toolbar SVG icons on native mobile

* Components: Start using isToggled prop for all Button components

* Fix the specificity issue with the focused state of color picker button

* Rename isToggled to isPressed for the Button component

* Bring back legacy components-tab-button class and its styles
  • Loading branch information
gziolo authored Dec 9, 2019
1 parent 38b4ca1 commit 383248a
Show file tree
Hide file tree
Showing 38 changed files with 155 additions and 169 deletions.
6 changes: 3 additions & 3 deletions packages/base-styles/_z-index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ $z-layers: (
// Needs to appear bellow other color circular picker related UI elements.
".components-circular-option-picker__option-wrapper::before": -1,

".components-circular-option-picker__option.is-active": 1,
// Needs to be higher than .components-circular-option-picker__option.is-active.
".components-circular-option-picker__option.is-active + .dashicons-saved": 2
".components-circular-option-picker__option.is-pressed": 1,
// Needs to be higher than .components-circular-option-picker__option.is-pressed.
".components-circular-option-picker__option.is-pressed + .dashicons-saved": 2
);

@function z-index( $key ) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Basic rendering should display with required props 1`] = `"<span><div tabindex=\\"-1\\"><div><div><div class=\\"components-popover block-editor-link-control\\"><div class=\\"components-popover__content\\" tabindex=\\"-1\\"><div class=\\"block-editor-link-control__popover-inner\\"><div class=\\"block-editor-link-control__search\\"><form><div class=\\"components-base-control editor-url-input block-editor-url-input block-editor-link-control__search-input\\"><div class=\\"components-base-control__field\\"><input type=\\"text\\" aria-label=\\"URL\\" required=\\"\\" placeholder=\\"Search or type url\\" role=\\"combobox\\" aria-expanded=\\"false\\" aria-autocomplete=\\"list\\" aria-owns=\\"block-editor-url-input-suggestions-0\\" value=\\"\\"></div></div><button type=\\"reset\\" disabled=\\"\\" aria-label=\\"Reset\\" class=\\"components-button components-icon-button block-editor-link-control__search-reset\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" class=\\"dashicon dashicons-no-alt\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\"><path d=\\"M14.95 6.46L11.41 10l3.54 3.54-1.41 1.41L10 11.42l-3.53 3.53-1.42-1.42L8.58 10 5.05 6.47l1.42-1.42L10 8.58l3.54-3.53z\\"></path></svg></button></form></div></div></div></div></div></div></div></span>"`;
exports[`Basic rendering should display with required props 1`] = `"<span><div tabindex=\\"-1\\"><div><div><div class=\\"components-popover block-editor-link-control\\"><div class=\\"components-popover__content\\" tabindex=\\"-1\\"><div class=\\"block-editor-link-control__popover-inner\\"><div class=\\"block-editor-link-control__search\\"><form><div class=\\"components-base-control editor-url-input block-editor-url-input block-editor-link-control__search-input\\"><div class=\\"components-base-control__field\\"><input type=\\"text\\" aria-label=\\"URL\\" required=\\"\\" placeholder=\\"Search or type url\\" role=\\"combobox\\" aria-expanded=\\"false\\" aria-autocomplete=\\"list\\" aria-owns=\\"block-editor-url-input-suggestions-0\\" value=\\"\\"></div></div><button type=\\"reset\\" disabled=\\"\\" aria-label=\\"Reset\\" class=\\"components-button components-icon-button block-editor-link-control__search-reset\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\" class=\\"dashicon dashicons-no-alt\\"><path d=\\"M14.95 6.46L11.41 10l3.54 3.54-1.41 1.41L10 11.42l-3.53 3.53-1.42-1.42L8.58 10 5.05 6.47l1.42-1.42L10 8.58l3.54-3.53z\\"></path></svg></button></form></div></div></div></div></div></div></div></span>"`;
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ export class MediaPlaceholder extends Component {
<Button
className="editor-media-placeholder__button block-editor-media-placeholder__button"
onClick={ this.openURLInput }
isToggled={ isURLInputVisible }
isPressed={ isURLInputVisible }
isLarge
>
{ __( 'Insert from URL' ) }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Basic rendering should render with required props 1`] = `"<fieldset class=\\"block-editor-responsive-block-control\\"><legend class=\\"block-editor-responsive-block-control__title\\">Padding</legend><div class=\\"block-editor-responsive-block-control__inner\\"><div class=\\"components-base-control components-toggle-control block-editor-responsive-block-control__toggle\\"><div class=\\"components-base-control__field\\"><span class=\\"components-form-toggle is-checked\\"><input class=\\"components-form-toggle__input\\" id=\\"inspector-toggle-control-0\\" type=\\"checkbox\\" aria-describedby=\\"inspector-toggle-control-0__help\\" checked=\\"\\"><span class=\\"components-form-toggle__track\\"></span><span class=\\"components-form-toggle__thumb\\"></span><svg class=\\"components-form-toggle__on\\" width=\\"2\\" height=\\"6\\" xmlns=\\"http://www.w3.org/2000/svg\\" viewBox=\\"0 0 2 6\\" role=\\"img\\" aria-hidden=\\"true\\" focusable=\\"false\\"><path d=\\"M0 0h2v6H0z\\"></path></svg></span><label for=\\"inspector-toggle-control-0\\" class=\\"components-toggle-control__label\\">Use the same padding on all screensizes.</label></div><p id=\\"inspector-toggle-control-0__help\\" class=\\"components-base-control__help\\">Toggle between using the same value for all screen sizes or using a unique value per screen size.</p></div><div class=\\"block-editor-responsive-block-control__group block-editor-responsive-block-control__group--default\\"><div class=\\"components-base-control\\"><div class=\\"components-base-control__field\\"><label class=\\"components-base-control__label\\" for=\\"inspector-select-control-0\\"><span aria-describedby=\\"rbc-desc-0\\">All</span><span class=\\"screen-reader-text\\" id=\\"rbc-desc-0\\">Controls the padding property for All viewports.</span></label><select id=\\"inspector-select-control-0\\" class=\\"components-select-control__input\\"><option value=\\"\\">Please select</option><option value=\\"small\\">Small</option><option value=\\"medium\\">Medium</option><option value=\\"large\\">Large</option></select></div></div><p id=\\"all\\">All is used here for testing purposes to ensure we have access to details about the device.</p></div></div></fieldset>"`;
exports[`Basic rendering should render with required props 1`] = `"<fieldset class=\\"block-editor-responsive-block-control\\"><legend class=\\"block-editor-responsive-block-control__title\\">Padding</legend><div class=\\"block-editor-responsive-block-control__inner\\"><div class=\\"components-base-control components-toggle-control block-editor-responsive-block-control__toggle\\"><div class=\\"components-base-control__field\\"><span class=\\"components-form-toggle is-checked\\"><input class=\\"components-form-toggle__input\\" id=\\"inspector-toggle-control-0\\" type=\\"checkbox\\" aria-describedby=\\"inspector-toggle-control-0__help\\" checked=\\"\\"><span class=\\"components-form-toggle__track\\"></span><span class=\\"components-form-toggle__thumb\\"></span><svg width=\\"2\\" height=\\"6\\" xmlns=\\"http://www.w3.org/2000/svg\\" viewBox=\\"0 0 2 6\\" class=\\"components-form-toggle__on\\" role=\\"img\\" aria-hidden=\\"true\\" focusable=\\"false\\"><path d=\\"M0 0h2v6H0z\\"></path></svg></span><label for=\\"inspector-toggle-control-0\\" class=\\"components-toggle-control__label\\">Use the same padding on all screensizes.</label></div><p id=\\"inspector-toggle-control-0__help\\" class=\\"components-base-control__help\\">Toggle between using the same value for all screen sizes or using a unique value per screen size.</p></div><div class=\\"block-editor-responsive-block-control__group block-editor-responsive-block-control__group--default\\"><div class=\\"components-base-control\\"><div class=\\"components-base-control__field\\"><label class=\\"components-base-control__label\\" for=\\"inspector-select-control-0\\"><span aria-describedby=\\"rbc-desc-0\\">All</span><span class=\\"screen-reader-text\\" id=\\"rbc-desc-0\\">Controls the padding property for All viewports.</span></label><select id=\\"inspector-select-control-0\\" class=\\"components-select-control__input\\"><option value=\\"\\">Please select</option><option value=\\"small\\">Small</option><option value=\\"medium\\">Medium</option><option value=\\"large\\">Large</option></select></div></div><p id=\\"all\\">All is used here for testing purposes to ensure we have access to details about the device.</p></div></div></fieldset>"`;
10 changes: 2 additions & 8 deletions packages/block-editor/src/components/url-input/button.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
Expand Down Expand Up @@ -45,9 +40,8 @@ class URLInputButton extends Component {
icon="admin-links"
label={ buttonLabel }
onClick={ this.toggle }
className={ classnames( 'components-toolbar__control', {
'is-active': url,
} ) }
className="components-toolbar__control"
isPressed={ !! url }
/>
{ expanded &&
<form
Expand Down
8 changes: 4 additions & 4 deletions packages/block-editor/src/components/url-input/test/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ describe( 'URLInputButton', () => {
const wrapper = shallow( <URLInputButton /> );
expect( wrapper.hasClass( 'block-editor-url-input__button' ) ).toBe( true );
} );
it( 'should not have is-active class when url prop not defined', () => {
it( 'should have isPressed props set to false when url prop not defined', () => {
const wrapper = shallow( <URLInputButton /> );
expect( wrapper.find( 'ForwardRef(IconButton)' ).hasClass( 'is-active' ) ).toBe( false );
expect( wrapper.find( 'ForwardRef(IconButton)' ).prop( 'isPressed' ) ).toBe( false );
} );
it( 'should have is-active class name if url prop defined', () => {
it( 'should have isPressed prop set to true if url prop defined', () => {
const wrapper = shallow( <URLInputButton url="https://example.com" /> );
expect( wrapper.find( 'ForwardRef(IconButton)' ).hasClass( 'is-active' ) ).toBe( true );
expect( wrapper.find( 'ForwardRef(IconButton)' ).prop( 'isPressed' ) ).toBe( true );
} );
it( 'should have hidden form by default', () => {
const wrapper = shallow( <URLInputButton /> );
Expand Down
54 changes: 0 additions & 54 deletions packages/block-library/src/code/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,57 +12,3 @@
box-shadow: none;
}
}

// We should extract this to a separate components-toolbar
.components-tab-button {
display: inline-flex;
align-items: flex-end;
margin: 0;
padding: 3px;
background: none;
outline: none;
color: $dark-gray-500;
cursor: pointer;
position: relative;
height: $icon-button-size;
font-family: $default-font;
font-size: $default-font-size;
font-weight: 500;
border: 0;

&.is-active,
&.is-active:hover {
color: $white;
}

&:disabled {
cursor: default;
}

& > span {
border: $border-width solid transparent;
padding: 0 6px;
box-sizing: content-box;
height: 28px;
line-height: 28px;
}

&:hover > span,
&:focus > span {
color: $dark-gray-500;
}

&:not(:disabled) {
&.is-active > span,
&:hover > span,
&:focus > span {
border: $border-width solid $dark-gray-500;
}
}

&.is-active > span,
&.is-active:hover > span {
background-color: $dark-gray-500;
color: $white;
}
}
4 changes: 2 additions & 2 deletions packages/block-library/src/heading/heading-level-icon.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import { Path, SVG } from '@wordpress/components';

export default function HeadingLevelIcon( { level, __unstableActive } ) {
export default function HeadingLevelIcon( { level, isPressed = false } ) {
const levelToPath = {
1: 'M9 5h2v10H9v-4H5v4H3V5h2v4h4V5zm6.6 0c-.6.9-1.5 1.7-2.6 2v1h2v7h2V5h-1.4z',
2: 'M7 5h2v10H7v-4H3v4H1V5h2v4h4V5zm8 8c.5-.4.6-.6 1.1-1.1.4-.4.8-.8 1.2-1.3.3-.4.6-.8.9-1.3.2-.4.3-.8.3-1.3 0-.4-.1-.9-.3-1.3-.2-.4-.4-.7-.8-1-.3-.3-.7-.5-1.2-.6-.5-.2-1-.2-1.5-.2-.4 0-.7 0-1.1.1-.3.1-.7.2-1 .3-.3.1-.6.3-.9.5-.3.2-.6.4-.8.7l1.2 1.2c.3-.3.6-.5 1-.7.4-.2.7-.3 1.2-.3s.9.1 1.3.4c.3.3.5.7.5 1.1 0 .4-.1.8-.4 1.1-.3.5-.6.9-1 1.2-.4.4-1 .9-1.6 1.4-.6.5-1.4 1.1-2.2 1.6V15h8v-2H15z',
Expand All @@ -17,7 +17,7 @@ export default function HeadingLevelIcon( { level, __unstableActive } ) {
}

return (
<SVG width="20" height="20" viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg" __unstableActive={ __unstableActive } >
<SVG width="20" height="20" viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg" isPressed={ isPressed } >
<Path d={ levelToPath[ level ] } />
</SVG>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/heading/heading-toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class HeadingToolbar extends Component {
createLevelControl( targetLevel, selectedLevel, onChange ) {
const isActive = targetLevel === selectedLevel;
return {
icon: <HeadingLevelIcon level={ targetLevel } __unstableActive={ isActive } />,
icon: <HeadingLevelIcon level={ targetLevel } isPressed={ isActive } />,
// translators: %s: heading level e.g: "1", "2", "3"
title: sprintf( __( 'Heading %d' ), targetLevel ),
isActive,
Expand Down
6 changes: 4 additions & 2 deletions packages/block-library/src/html/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,15 @@ class HTMLEdit extends Component {
<BlockControls>
<ToolbarGroup>
<Button
className={ `components-tab-button ${ ! isPreview ? 'is-active' : '' }` }
className="components-tab-button"
isPressed={ ! isPreview }
onClick={ this.switchToHTML }
>
<span>HTML</span>
</Button>
<Button
className={ `components-tab-button ${ isPreview ? 'is-active' : '' }` }
className="components-tab-button"
isPressed={ isPreview }
onClick={ this.switchToPreview }
>
<span>{ __( 'Preview' ) }</span>
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/image/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ export class ImageEdit extends Component {
key={ scale }
isSmall
isPrimary={ isCurrent }
aria-pressed={ isCurrent }
isPressed={ isCurrent }
onClick={ this.updateDimensions( scaledWidth, scaledHeight ) }
>
{ scale }%
Expand Down
6 changes: 4 additions & 2 deletions packages/block-library/src/legacy-widget/edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,15 @@ class LegacyWidgetEdit extends Component {
{ hasEditForm && (
<>
<Button
className={ `components-tab-button ${ ! isPreview ? 'is-active' : '' }` }
className="components-tab-button"
isPressed={ ! isPreview }
onClick={ this.switchToEdit }
>
<span>{ __( 'Edit' ) }</span>
</Button>
<Button
className={ `components-tab-button ${ isPreview ? 'is-active' : '' }` }
className="components-tab-button"
isPressed={ isPreview }
onClick={ this.switchToPreview }
>
<span>{ __( 'Preview' ) }</span>
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/button/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ Name | Type | Default | Description
`isDestructive` | `bool` | `false` | Renders a red text-based button style to indicate destructive behavior.
`isLarge` | `bool` | `false` | Increases the size of the button.
`isSmall` | `bool` | `false` | Decreases the size of the button.
`isToggled` | `bool` | `false` | Renders a toggled button style.
`isPressed` | `bool` | `false` | Renders a pressed button style.
`isBusy` | `bool` | `false` | Indicates activity while a action is being performed.
`isLink` | `bool` | `false` | Renders a button with an anchor style.
`focus` | `bool` | `false` | Whether the button is focused.
Expand Down
8 changes: 5 additions & 3 deletions packages/components/src/button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function Button( props, ref ) {
isLarge,
isSmall,
isTertiary,
isToggled,
isPressed,
isBusy,
isDefault,
isLink,
Expand All @@ -33,14 +33,16 @@ export function Button( props, ref ) {
'is-large': isLarge,
'is-small': isSmall,
'is-tertiary': isTertiary,
'is-toggled': isToggled,
'is-pressed': isPressed,
'is-busy': isBusy,
'is-link': isLink,
'is-destructive': isDestructive,
} );

const tag = href !== undefined && ! disabled ? 'a' : 'button';
const tagProps = tag === 'a' ? { href, target } : { type: 'button', disabled };
const tagProps = tag === 'a' ?
{ href, target } :
{ type: 'button', disabled, 'aria-pressed': isPressed };

return createElement( tag, {
...tagProps,
Expand Down
10 changes: 5 additions & 5 deletions packages/components/src/button/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ export function Button( props ) {
hint,
fixedRatio = true,
getStylesFromColorScheme,
isPressed,
'aria-disabled': ariaDisabled,
'aria-label': ariaLabel,
'aria-pressed': ariaPressed,
'data-subscript': subscript,
} = props;

Expand All @@ -78,11 +78,11 @@ export function Button( props ) {
const buttonViewStyle = {
opacity: isDisabled ? 0.3 : 1,
...( fixedRatio && styles.fixedRatio ),
...( ariaPressed ? styles.buttonActive : styles.buttonInactive ),
...( isPressed ? styles.buttonActive : styles.buttonInactive ),
};

const states = [];
if ( ariaPressed ) {
if ( isPressed ) {
states.push( 'selected' );
}

Expand All @@ -93,7 +93,7 @@ export function Button( props ) {
const subscriptInactive = getStylesFromColorScheme( styles.subscriptInactive, styles.subscriptInactiveDark );

const newChildren = Children.map( children, ( child ) => {
return child ? cloneElement( child, { colorScheme: props.preferredColorScheme, __unstableActive: ariaPressed } ) : child;
return child ? cloneElement( child, { colorScheme: props.preferredColorScheme, isPressed } ) : child;
} );

return (
Expand All @@ -111,7 +111,7 @@ export function Button( props ) {
<View style={ buttonViewStyle }>
<View style={ { flexDirection: 'row' } }>
{ newChildren }
{ subscript && ( <Text style={ ariaPressed ? styles.subscriptActive : subscriptInactive }>{ subscript }</Text> ) }
{ subscript && ( <Text style={ isPressed ? styles.subscriptActive : subscriptInactive }>{ subscript }</Text> ) }
</View>
</View>
</TouchableOpacity>
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/button/stories/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ export const small = () => {
);
};

export const toggled = () => {
const label = text( 'Label', 'Toggled Button' );
export const pressed = () => {
const label = text( 'Label', 'Pressed Button' );

return (
<Button isToggled>{ label }</Button>
<Button isPressed>{ label }</Button>
);
};

Expand Down
8 changes: 4 additions & 4 deletions packages/components/src/button/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe( 'Button', () => {
expect( button.hasClass( 'components-button' ) ).toBe( true );
expect( button.hasClass( 'is-large' ) ).toBe( false );
expect( button.hasClass( 'is-primary' ) ).toBe( false );
expect( button.hasClass( 'is-toggled' ) ).toBe( false );
expect( button.hasClass( 'is-pressed' ) ).toBe( false );
expect( button.prop( 'disabled' ) ).toBeUndefined();
expect( button.prop( 'type' ) ).toBe( 'button' );
expect( button.type() ).toBe( 'button' );
Expand Down Expand Up @@ -56,10 +56,10 @@ describe( 'Button', () => {
expect( button.hasClass( 'is-primary' ) ).toBe( false );
} );

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

it( 'should add a disabled prop to the button', () => {
Expand Down
3 changes: 1 addition & 2 deletions packages/components/src/circular-option-picker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@ function Option( {
} ) {
const optionButton = (
<Button
aria-pressed={ isSelected }
isPressed={ isSelected }
className={ classnames(
className,
'components-circular-option-picker__option',
{ 'is-active': isSelected }
) }
{ ...additionalProps }
/>
Expand Down
13 changes: 9 additions & 4 deletions packages/components/src/circular-option-picker/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,17 @@ $color-palette-circle-spacing: 12px;
@include reduce-motion("transition");
cursor: pointer;

&.is-active {
&.is-pressed {
box-shadow: inset 0 0 0 4px;
position: relative;
z-index: z-index(".components-circular-option-picker__option.is-active");
z-index: z-index(".components-circular-option-picker__option.is-pressed");

& + .dashicons-saved {
position: absolute;
left: 4px;
top: 4px;
border-radius: 50%;
z-index: z-index(".components-circular-option-picker__option.is-active + .dashicons-saved");
z-index: z-index(".components-circular-option-picker__option.is-pressed + .dashicons-saved");
background: $white;
pointer-events: none;
}
Expand All @@ -96,7 +96,6 @@ $color-palette-circle-spacing: 12px;
}

&:focus {
outline: none;
&::after {
content: "";
border: #{ $border-width * 2 } solid $dark-gray-400;
Expand All @@ -109,6 +108,12 @@ $color-palette-circle-spacing: 12px;
box-shadow: inset 0 0 0 2px $white;
}
}

&.components-button:focus {
background-color: transparent;
box-shadow: inset 0 0 0 ($color-palette-circle-size / 2);
outline: none;
}
}

.components-circular-option-picker__button-action .components-circular-option-picker__option {
Expand Down
Loading

0 comments on commit 383248a

Please sign in to comment.