From 8c06276b00c75ba34c94e93d896212f02b5b8ac0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Juh=C3=A9=20Lluveras?= Date: Fri, 10 Jul 2020 11:09:49 +0200 Subject: [PATCH] Unify Chip styles (https://github.com/woocommerce/woocommerce-blocks/pull/2765) * Improve cross character used in dropdown selector * Add is-open and has-checked classes to dropdown selector so we stop relying on complex pseudoselector combinations * Update Chip component with new props * Update Chip styles * Update dropdown selector selected chip with the Chip component * Update active filters chip with the Chip component * Style fixes * Update snapshots and add tests * Change onRemove prop signature * Fix wrong bottom margin * Fix misaligned remove icon * Update assets/js/base/components/chip/index.js Co-authored-by: Darren Ethier * Update assets/js/base/components/chip/index.js Co-authored-by: Darren Ethier * Add missing default prop * Prettier * Ensure showScreenReaderText is a bool * Rename removeOnClick with removeOnAnyClick * Update snapshots * Fix Chip aria-label logic * Split Chip and RemovableChip * Replace chip close character with an icon * Fix outdated comment Co-authored-by: Darren Ethier --- .../totals/totals-discount-item/index.js | 12 +- .../assets/js/base/components/chip/chip.js | 63 ++++ .../assets/js/base/components/chip/index.js | 72 +---- .../js/base/components/chip/removable-chip.js | 91 ++++++ .../assets/js/base/components/chip/style.scss | 59 ++-- .../chip/test/__snapshots__/index.js.snap | 281 +++++++++++++++--- .../js/base/components/chip/test/index.js | 100 +++++-- .../components/dropdown-selector/index.js | 2 + .../dropdown-selector/selected-chip.js | 24 +- .../dropdown-selector/selected-value.js | 3 +- .../components/dropdown-selector/style.scss | 98 +++--- .../active-attribute-filters.js | 2 + .../assets/js/blocks/active-filters/block.js | 4 + .../js/blocks/active-filters/style.scss | 139 ++++----- .../assets/js/blocks/active-filters/utils.js | 59 ++-- 15 files changed, 693 insertions(+), 316 deletions(-) create mode 100644 plugins/woocommerce-blocks/assets/js/base/components/chip/chip.js create mode 100644 plugins/woocommerce-blocks/assets/js/base/components/chip/removable-chip.js diff --git a/plugins/woocommerce-blocks/assets/js/base/components/cart-checkout/totals/totals-discount-item/index.js b/plugins/woocommerce-blocks/assets/js/base/components/cart-checkout/totals/totals-discount-item/index.js index c672a0b80bf..9fa6303b62d 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/cart-checkout/totals/totals-discount-item/index.js +++ b/plugins/woocommerce-blocks/assets/js/base/components/cart-checkout/totals/totals-discount-item/index.js @@ -4,7 +4,7 @@ import { __, sprintf } from '@wordpress/i18n'; import { DISPLAY_CART_PRICES_INCLUDING_TAX } from '@woocommerce/block-settings'; import LoadingMask from '@woocommerce/base-components/loading-mask'; -import Chip from '@woocommerce/base-components/chip'; +import { RemovableChip } from '@woocommerce/base-components/chip'; import PropTypes from 'prop-types'; /** @@ -51,7 +51,7 @@ const TotalsDiscountItem = ( { >
    { cartCoupons.map( ( cartCoupon ) => ( - ) ) }
diff --git a/plugins/woocommerce-blocks/assets/js/base/components/chip/chip.js b/plugins/woocommerce-blocks/assets/js/base/components/chip/chip.js new file mode 100644 index 00000000000..229cf27fbdc --- /dev/null +++ b/plugins/woocommerce-blocks/assets/js/base/components/chip/chip.js @@ -0,0 +1,63 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; +import classNames from 'classnames'; + +/** + * Internal dependencies + */ +import './style.scss'; + +/** + * Component used to render a "chip" -- a list item containing some text. + * + * Each chip defaults to a list element but this can be customized by providing + * a wrapperElement. + */ +const Chip = ( { + text, + screenReaderText = '', + element = 'li', + className = '', + radius = 'small', + children = null, + ...props +} ) => { + const Wrapper = element; + const wrapperClassName = classNames( + className, + 'wc-block-components-chip', + 'wc-block-components-chip--radius-' + radius + ); + + const showScreenReaderText = Boolean( + screenReaderText && screenReaderText !== text + ); + + return ( + // @ts-ignore + + + { text } + + { showScreenReaderText && ( + { screenReaderText } + ) } + { children } + + ); +}; + +Chip.propTypes = { + text: PropTypes.node.isRequired, + screenReaderText: PropTypes.string, + element: PropTypes.elementType, + className: PropTypes.string, + radius: PropTypes.oneOf( [ 'none', 'small', 'medium', 'large' ] ), +}; + +export default Chip; diff --git a/plugins/woocommerce-blocks/assets/js/base/components/chip/index.js b/plugins/woocommerce-blocks/assets/js/base/components/chip/index.js index 4b38ab38796..7e83fec240d 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/chip/index.js +++ b/plugins/woocommerce-blocks/assets/js/base/components/chip/index.js @@ -1,70 +1,2 @@ -/** - * External dependencies - */ -import PropTypes from 'prop-types'; -import classNames from 'classnames'; -import { __, sprintf } from '@wordpress/i18n'; - -/** - * Internal dependencies - */ -import './style.scss'; - -/** - * Component used to render a "chip" -- a list item containing some text with - * an X button to remove/dismiss each chip. - * - * Each chip defaults to a list element but this can be customized by providing - * a wrapperElement. - */ -const Chip = ( { - text, - screenReaderText, - element = 'li', - className = '', - onRemove = () => {}, - disabled = false, - radius = 'small', -} ) => { - const Wrapper = element; - const wrapperClassName = classNames( - className, - 'wc-block-components-chip', - 'wc-block-components-chip--radius-' + radius - ); - - return ( - // @ts-ignore - - - - { screenReaderText ? screenReaderText : text } - - - - ); -}; - -Chip.propTypes = { - text: PropTypes.string.isRequired, - screenReaderText: PropTypes.string, - element: PropTypes.elementType, - className: PropTypes.string, - onRemove: PropTypes.func, - radius: PropTypes.oneOf( [ 'none', 'small', 'medium', 'large' ] ), -}; - -export default Chip; +export { default as Chip } from './chip'; +export { default as RemovableChip } from './removable-chip'; diff --git a/plugins/woocommerce-blocks/assets/js/base/components/chip/removable-chip.js b/plugins/woocommerce-blocks/assets/js/base/components/chip/removable-chip.js new file mode 100644 index 00000000000..33eaec93d0b --- /dev/null +++ b/plugins/woocommerce-blocks/assets/js/base/components/chip/removable-chip.js @@ -0,0 +1,91 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; +import classNames from 'classnames'; +import { __, sprintf } from '@wordpress/i18n'; +import { Icon, noAlt } from '@woocommerce/icons'; + +/** + * Internal dependencies + */ +import { Chip } from './index.js'; + +/** + * Component used to render a "chip" -- an item containing some text with + * an X button to remove/dismiss each chip. + */ +const RemovableChip = ( { + ariaLabel = '', + className = '', + disabled = false, + onRemove = () => void null, + removeOnAnyClick = false, + text, + screenReaderText = '', + ...props +} ) => { + const RemoveElement = removeOnAnyClick ? 'span' : 'button'; + + if ( ! ariaLabel ) { + const ariaLabelText = + screenReaderText && typeof screenReaderText === 'string' + ? screenReaderText + : text; + ariaLabel = + typeof ariaLabelText !== 'string' + ? /* translators: Remove chip. */ + __( 'Remove', 'woo-gutenberg-products-block' ) + : sprintf( + /* translators: %s text of the chip to remove. */ + __( 'Remove "%s"', 'woo-gutenberg-products-block' ), + ariaLabelText + ); + } + + const clickableElementProps = { + 'aria-label': ariaLabel, + disabled, + onClick: onRemove, + onKeyDown: ( e ) => { + if ( e.key === 'Backspace' || e.key === 'Delete' ) { + onRemove(); + } + }, + }; + + const chipProps = removeOnAnyClick ? clickableElementProps : {}; + const removeProps = removeOnAnyClick + ? { 'aria-hidden': true } + : clickableElementProps; + + return ( + + + + + + ); +}; + +RemovableChip.propTypes = { + text: PropTypes.node.isRequired, + ariaLabel: PropTypes.string, + className: PropTypes.string, + disabled: PropTypes.bool, + onRemove: PropTypes.func, + removeOnAnyClick: PropTypes.bool, + screenReaderText: PropTypes.string, +}; + +export default RemovableChip; diff --git a/plugins/woocommerce-blocks/assets/js/base/components/chip/style.scss b/plugins/woocommerce-blocks/assets/js/base/components/chip/style.scss index 747f575467b..e4d1881092d 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/chip/style.scss +++ b/plugins/woocommerce-blocks/assets/js/base/components/chip/style.scss @@ -1,13 +1,23 @@ .wc-block-components-chip { - display: inline-block; - background: $core-grey-light-500; - padding: 0.365em 0.5em; + @include reset-typography(); + align-items: center; + border: 0; + display: inline-flex; + padding: em($gap-smallest / 2) 0.5em em($gap-smallest); margin: 0 0.365em 0.365em 0; - color: $core-grey-dark-800; border-radius: 0; - line-height: 1em; + line-height: 1; max-width: 100%; + // Chip might be a button, so we need to override theme styles. + &, + &:hover, + &:focus, + &:active { + background: $core-grey-light-500; + color: $core-grey-dark-800; + } + &.wc-block-components-chip--radius-small { border-radius: 3px; } @@ -15,30 +25,41 @@ border-radius: 0.433em; } &.wc-block-components-chip--radius-large { - border-radius: 0.865em; - padding: 0.365em 0.75em; + border-radius: 2em; + padding-left: 0.75em; + padding-right: 0.75em; } .wc-block-components-chip__text { + flex-grow: 1; + } + &.is-removable { padding-right: 0.5em; } + &.is-removable .wc-block-components-chip__text { + padding-right: 0.25em; + } .wc-block-components-chip__remove { @include font-size(smaller); background: transparent; border: 0; appearance: none; - float: none; - vertical-align: middle; - line-height: 1.33em; - padding: 0.66em; // Should equate to ~8px; chip has ~6px padding, and font size difference/2 is 2px. - margin: -0.66em; + padding: 0; - &:hover, - &:focus { - color: #d94f4f; - } - &:disabled { - color: $core-grey-dark-100; - cursor: not-allowed; + > .dashicon { + vertical-align: middle; } } } + +button.wc-block-components-chip:hover > .wc-block-components-chip__remove, +button.wc-block-components-chip:focus > .wc-block-components-chip__remove, +.wc-block-components-chip__remove:hover, +.wc-block-components-chip__remove:focus { + fill: #d94f4f; +} + +button.wc-block-components-chip:disabled > .wc-block-components-chip__remove, +.wc-block-components-chip__remove:disabled { + fill: $core-grey-dark-100; + cursor: not-allowed; +} diff --git a/plugins/woocommerce-blocks/assets/js/base/components/chip/test/__snapshots__/index.js.snap b/plugins/woocommerce-blocks/assets/js/base/components/chip/test/__snapshots__/index.js.snap index 015ba2446cc..2a1097e6313 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/chip/test/__snapshots__/index.js.snap +++ b/plugins/woocommerce-blocks/assets/js/base/components/chip/test/__snapshots__/index.js.snap @@ -1,105 +1,312 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Chip should render children nodes 1`] = ` +
  • + + Test + + Lorem Ipsum +
    • +`; + +exports[`Chip should render defined radius 1`] = ` +
  • + + Test + +
    • +`; + +exports[`Chip should render nodes as the text 1`] = ` +
  • + +

    + Test +

    +     +
    • +`; + +exports[`Chip should render screen reader text 1`] = ` +
  • + + Test + + + Test 2 + +
    • +`; + +exports[`Chip should render text 1`] = ` +
  • + + Test + +
    • +`; + exports[`Chip with custom wrapper should render a chip made up of a div instead of a li 1`] = `
    - - Test - -
    `; -exports[`Chip without custom wrapper should render defined radius 1`] = ` +exports[`RemovableChip should render custom aria label 1`] = `
  • - - Test +

    + Test +

    • `; -exports[`Chip without custom wrapper should render text and the remove button 1`] = ` +exports[`RemovableChip should render default aria label if text is a node 1`] = `
  • - Test + Test 2
    • `; -exports[`Chip without custom wrapper should render with disabled remove button 1`] = ` +exports[`RemovableChip should render screen reader text aria label 1`] = `
  • + Test 2 + + +
    • +`; + +exports[`RemovableChip should render text and the remove button 1`] = ` +
  • + Test +
    • +`; + +exports[`RemovableChip should render with disabled remove button 1`] = ` +
  • + + Test + +
    • `; + +exports[`RemovableChip with removeOnAnyClick should be a button when removeOnAnyClick is set to true 1`] = ` + +`; diff --git a/plugins/woocommerce-blocks/assets/js/base/components/chip/test/index.js b/plugins/woocommerce-blocks/assets/js/base/components/chip/test/index.js index e8ec6d4fd08..f666b1ab1ff 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/chip/test/index.js +++ b/plugins/woocommerce-blocks/assets/js/base/components/chip/test/index.js @@ -6,31 +6,45 @@ import TestRenderer from 'react-test-renderer'; /** * Internal dependencies */ -import Chip from '../'; +import { Chip, RemovableChip } from '..'; describe( 'Chip', () => { - describe( 'without custom wrapper', () => { - test( 'should render text and the remove button', () => { - const component = TestRenderer.create( ); + test( 'should render text', () => { + const component = TestRenderer.create( ); - expect( component.toJSON() ).toMatchSnapshot(); - } ); + expect( component.toJSON() ).toMatchSnapshot(); + } ); - test( 'should render defined radius', () => { - const component = TestRenderer.create( - - ); + test( 'should render nodes as the text', () => { + const component = TestRenderer.create( + Test } /> + ); - expect( component.toJSON() ).toMatchSnapshot(); - } ); + expect( component.toJSON() ).toMatchSnapshot(); + } ); - test( 'should render with disabled remove button', () => { - const component = TestRenderer.create( - - ); + test( 'should render defined radius', () => { + const component = TestRenderer.create( + + ); - expect( component.toJSON() ).toMatchSnapshot(); - } ); + expect( component.toJSON() ).toMatchSnapshot(); + } ); + + test( 'should render screen reader text', () => { + const component = TestRenderer.create( + + ); + + expect( component.toJSON() ).toMatchSnapshot(); + } ); + + test( 'should render children nodes', () => { + const component = TestRenderer.create( + Lorem Ipsum + ); + + expect( component.toJSON() ).toMatchSnapshot(); } ); describe( 'with custom wrapper', () => { @@ -43,3 +57,53 @@ describe( 'Chip', () => { } ); } ); } ); + +describe( 'RemovableChip', () => { + test( 'should render text and the remove button', () => { + const component = TestRenderer.create( ); + + expect( component.toJSON() ).toMatchSnapshot(); + } ); + + test( 'should render with disabled remove button', () => { + const component = TestRenderer.create( + + ); + + expect( component.toJSON() ).toMatchSnapshot(); + } ); + + test( 'should render custom aria label', () => { + const component = TestRenderer.create( + Test } ariaLabel="Aria test" /> + ); + + expect( component.toJSON() ).toMatchSnapshot(); + } ); + + test( 'should render default aria label if text is a node', () => { + const component = TestRenderer.create( + Test } screenReaderText="Test 2" /> + ); + + expect( component.toJSON() ).toMatchSnapshot(); + } ); + + test( 'should render screen reader text aria label', () => { + const component = TestRenderer.create( + + ); + + expect( component.toJSON() ).toMatchSnapshot(); + } ); + + describe( 'with removeOnAnyClick', () => { + test( 'should be a button when removeOnAnyClick is set to true', () => { + const component = TestRenderer.create( + + ); + + expect( component.toJSON() ).toMatchSnapshot(); + } ); + } ); +} ); diff --git a/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/index.js b/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/index.js index 054238a9d33..f3973a3efa4 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/index.js +++ b/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/index.js @@ -91,6 +91,8 @@ const DropdownSelector = ( { className={ classNames( classes, { 'is-multiple': multiple, 'is-single': ! multiple, + 'has-checked': checked.length > 0, + 'is-open': isOpen, } ) } > { /* eslint-disable-next-line jsx-a11y/label-has-for */ } diff --git a/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/selected-chip.js b/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/selected-chip.js index fe1c6fc42a7..0d5b241fed7 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/selected-chip.js +++ b/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/selected-chip.js @@ -2,31 +2,23 @@ * External dependencies */ import { __, sprintf } from '@wordpress/i18n'; +import { RemovableChip } from '@woocommerce/base-components/chip'; const DropdownSelectorSelectedChip = ( { onRemoveItem, option } ) => { return ( - + text={ option.label } + radius="large" + /> ); }; diff --git a/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/selected-value.js b/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/selected-value.js index fddff3a8273..919527cd712 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/selected-value.js +++ b/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/selected-value.js @@ -3,6 +3,7 @@ */ import { __, sprintf } from '@wordpress/i18n'; import { useEffect, useRef } from '@wordpress/element'; +import { Icon, noAlt } from '@woocommerce/icons'; const DropdownSelectorSelectedValue = ( { onClick, onRemoveItem, option } ) => { const labelRef = useRef( null ); @@ -47,7 +48,7 @@ const DropdownSelectorSelectedValue = ( { onClick, onRemoveItem, option } ) => { option.name ) } > - 𝘅 + ); diff --git a/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/style.scss b/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/style.scss index dd112f42e18..c71b2624091 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/style.scss +++ b/plugins/woocommerce-blocks/assets/js/base/components/dropdown-selector/style.scss @@ -1,3 +1,7 @@ +// 18px is the minimum input field line-height and 14px is the font-size of +// the drop down selector elements. +$dropdown-selector-line-height: 18/14; + .wc-block-components-dropdown-selector { max-width: 300px; position: relative; @@ -5,34 +9,37 @@ } .wc-block-components-dropdown-selector__input-wrapper { + background: #fff; + border: 1px solid $input-border-gray; + color: $input-text-active; align-items: center; - border: 1px solid #9f9f9f; border-radius: 4px; cursor: text; display: flex; flex-wrap: wrap; - padding: 2px; + padding: 2px $gap-smaller; .is-disabled & { background-color: $core-grey-light-500; } -} -.wc-block-components-dropdown-selector__placeholder { - @include font-size(small); - height: 1.8em; - margin: 0 $gap-smallest; - white-space: nowrap; + .is-multiple.has-checked > & { + padding: 2px $gap-smallest; + } + + .is-open > & { + border-radius: 4px 4px 0 0; + } } .wc-block-components-dropdown-selector__input { @include font-size(small); - height: 1.8em; + line-height: $dropdown-selector-line-height; + margin: em($gap-small/4) 0; min-width: 0; + padding: em($gap-smallest * 0.75) 0 em($gap-smallest * 0.75); .is-single & { - margin: 0 4px; - padding: 0; width: 100%; &:hover, @@ -40,24 +47,23 @@ &:active { outline: 0; } + } - &:not(:first-child):focus { - margin-bottom: 1.5px; - margin-top: 1.5px; - } + .is-single.has-checked.is-open & { + margin-bottom: 1.5px; + margin-top: 1.5px; + } - &:not(:first-child):not(:focus) { - @include visually-hidden(); - // Fixes an issue in Firefox that `flex: wrap` in the container was making - // this element to still occupy one line. - position: absolute; - } + .is-single.has-checked:not(.is-open) & { + @include visually-hidden(); + // Fixes an issue in Firefox that `flex: wrap` in the container was making + // this element to still occupy one line. + position: absolute; } .is-multiple & { flex: 1; min-width: 0; - margin: 1.5px; } } @@ -76,7 +82,6 @@ .wc-block-components-dropdown-selector { // Reset - + { displayStyle === 'chips' ? ( + + ) : ( + + { prefixedName } + + + ) } ); };