From d64ee5d423e150185a773123138811df76ea71e8 Mon Sep 17 00:00:00 2001 From: Peter Kulko <93188219+PKulkoRaccoonGang@users.noreply.github.com> Date: Fri, 8 Dec 2023 15:50:23 +0200 Subject: [PATCH] feat!: Chip component redesign (#2836) --- src/Chip/Chip.test.jsx | 98 ++++++++++-- src/Chip/ChipIcon.tsx | 54 +++++++ src/Chip/README.md | 127 ++++++++++++++-- src/Chip/__snapshots__/Chip.test.jsx.snap | 174 +++++++++++----------- src/Chip/_variables.scss | 47 +++--- src/Chip/constants.js | 5 + src/Chip/index.scss | 141 ++++++++++++------ src/Chip/index.tsx | 121 +++++++++------ src/Chip/mixins.scss | 42 ++++++ src/ChipCarousel/_variables.scss | 4 +- src/ChipCarousel/index.scss | 1 + src/utils/propTypes/utils.js | 19 ++- 12 files changed, 601 insertions(+), 232 deletions(-) create mode 100644 src/Chip/ChipIcon.tsx create mode 100644 src/Chip/constants.js create mode 100644 src/Chip/mixins.scss diff --git a/src/Chip/Chip.test.jsx b/src/Chip/Chip.test.jsx index fd933621fb..f5e5367d18 100644 --- a/src/Chip/Chip.test.jsx +++ b/src/Chip/Chip.test.jsx @@ -4,6 +4,7 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { Close } from '../../icons'; +import { STYLE_VARIANTS } from './constants'; import Chip from '.'; function TestChip(props) { @@ -24,58 +25,123 @@ describe('', () => { }); it('renders with props iconBefore', () => { const tree = renderer.create(( - + )).toJSON(); expect(tree).toMatchSnapshot(); }); it('renders with props iconAfter', () => { const tree = renderer.create(( - + )).toJSON(); expect(tree).toMatchSnapshot(); }); it('renders with props iconBefore and iconAfter', () => { const tree = renderer.create(( - Chip + + Chip + + )).toJSON(); + expect(tree).toMatchSnapshot(); + }); + it('renders div with "button" role when onClick is provided', () => { + const tree = renderer.create(( + Chip )).toJSON(); expect(tree).toMatchSnapshot(); }); }); describe('correct rendering', () => { + it('render a non-interactive element if onClick handlers are not provided', () => { + render(); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); + it('render an interactive element if onClick handler is provided', () => { + render(); + expect(screen.queryByRole('button')).toBeInTheDocument(); + }); it('renders with correct class when variant is added', () => { - render(); - const chip = screen.getByTestId('chip'); + render(); + const chip = screen.getByRole('button'); expect(chip).toHaveClass('pgn__chip pgn__chip-dark'); }); it('renders with active class when disabled prop is added', () => { - render(); - const chip = screen.getByTestId('chip'); + render(); + const chip = screen.getByRole('button'); expect(chip).toHaveClass('disabled'); }); it('renders with the client\'s className', () => { const className = 'testClassName'; - render(); - const chip = screen.getByTestId('chip'); + render(); + const chip = screen.getByRole('button'); expect(chip).toHaveClass(className); }); it('onIconAfterClick is triggered', async () => { const func = jest.fn(); render( - , + , ); - const iconAfter = screen.getByTestId('icon-after'); + const iconAfter = screen.getByLabelText('icon-after'); await userEvent.click(iconAfter); - expect(func).toHaveBeenCalled(); + expect(func).toHaveBeenCalledTimes(1); }); it('onIconAfterKeyDown is triggered', async () => { const func = jest.fn(); render( - , + , + ); + const iconAfter = screen.getByLabelText('icon-after'); + await userEvent.click(iconAfter, '{enter}', { skipClick: true }); + expect(func).toHaveBeenCalledTimes(1); + }); + it('onIconBeforeClick is triggered', async () => { + const func = jest.fn(); + render( + , + ); + const iconBefore = screen.getByLabelText('icon-before'); + await userEvent.click(iconBefore); + expect(func).toHaveBeenCalledTimes(1); + }); + it('onIconBeforeKeyDown is triggered', async () => { + const func = jest.fn(); + render( + , ); - const iconAfter = screen.getByTestId('icon-after'); - await userEvent.type(iconAfter, '{enter}'); - expect(func).toHaveBeenCalled(); + const iconBefore = screen.getByLabelText('icon-before'); + await userEvent.click(iconBefore, '{enter}', { skipClick: true }); + expect(func).toHaveBeenCalledTimes(1); + }); + it('checks the absence of the `selected` class in the chip', async () => { + render(); + const chip = screen.getByRole('button'); + expect(chip).not.toHaveClass('selected'); + }); + it('checks the presence of the `selected` class in the chip', async () => { + render(); + const chip = screen.getByRole('button'); + expect(chip).toHaveClass('selected'); }); }); }); diff --git a/src/Chip/ChipIcon.tsx b/src/Chip/ChipIcon.tsx new file mode 100644 index 0000000000..a32692c5ce --- /dev/null +++ b/src/Chip/ChipIcon.tsx @@ -0,0 +1,54 @@ +import React, { KeyboardEventHandler, MouseEventHandler } from 'react'; +import PropTypes from 'prop-types'; +import Icon from '../Icon'; +// @ts-ignore +import IconButton from '../IconButton'; +// @ts-ignore +import { STYLE_VARIANTS } from './constants'; + +export interface ChipIconProps { + className: string, + src: React.ReactElement | Function, + onClick?: KeyboardEventHandler & MouseEventHandler, + alt?: string, + variant: string, + disabled?: boolean, +} + +function ChipIcon({ + className, src, onClick, alt, variant, disabled, +}: ChipIconProps) { + if (onClick) { + return ( + + ); + } + + return ; +} + +ChipIcon.propTypes = { + className: PropTypes.string.isRequired, + src: PropTypes.oneOfType([PropTypes.element, PropTypes.func]).isRequired, + onClick: PropTypes.func, + alt: PropTypes.string, + variant: PropTypes.string, + disabled: PropTypes.bool, +}; + +ChipIcon.defaultProps = { + onClick: undefined, + alt: undefined, + variant: STYLE_VARIANTS.LIGHT, + disabled: false, +}; + +export default ChipIcon; diff --git a/src/Chip/README.md b/src/Chip/README.md index 6497f9e7d3..3915513327 100644 --- a/src/Chip/README.md +++ b/src/Chip/README.md @@ -16,34 +16,139 @@ notes: | ## Basic Usage ```jsx live -
+ New New - New -
+ +``` + +## Clickable Variant + +Use `onClick` prop to make the whole `Chip` clickable, this will also add appropriate styles to make `Chip` interactive. + +```jsx live + console.log('Click!')}>Click Me +``` + +## With isSelected prop + +```jsx live +New ``` ## With Icon Before and After +### Basic Usage + +Use `iconBefore` and `iconAfter` props to provide icons for the `Chip`, note that you also can provide +accessible names for these icons for screen reader support via `iconBeforeAlt` and `iconAfterAlt` respectively. ```jsx live -
- New + + Person + Close console.log('Remove Chip')} + iconAfterAlt="icon-after" + iconBeforeAlt="icon-before" > - New + Both + + +``` + +### Clickable icon variant + +Provide click handlers for icons via `onIconAfterClick` and `onIconBeforeClick` props. + +```jsx live + + console.log('onIconBeforeClick')} + > + Person + + console.log('onIconAfterClick')} + iconAfterAlt="icon-after" + > + Close console.log('Remove Chip')} + onIconAfterClick={() => console.log('onIconAfterClick')} + onIconBeforeClick={() => console.log('onIconBeforeClick')} + iconAfterAlt="icon-after" + iconBeforeAlt="icon-before" + > + Both + + console.log('onIconAfterClick')} + onIconBeforeClick={() => console.log('onIconBeforeClick')} + iconAfterAlt="icon-after" + iconBeforeAlt="icon-before" + disabled + > + Both + + +``` + +**Note**: both `Chip` and its icons cannot be made interactive at the same time, e.g. if you provide both `onClick` and `onIconAfterClick` props, +`onClick` will be ignored and only the icon will get interactive behaviour, see example below (this is done to avoid usability issues where users might click on the `Chip` itself by mistake when they meant to click the icon instead). + +```jsx live + console.log('onIconBeforeClick')} + onClick={() => console.log('onClick')} +> + Person + +``` + +### Inverse Pallete + +```jsx live + + New + console.log('onIconAfterClick')} + iconAfterAlt="icon-after" + > + New 1 + + console.log('onIconAfterClick')} + iconAfterAlt="icon-after" disabled > New -
+ ``` diff --git a/src/Chip/__snapshots__/Chip.test.jsx.snap b/src/Chip/__snapshots__/Chip.test.jsx.snap index 4b706e50db..ce2f964cc2 100644 --- a/src/Chip/__snapshots__/Chip.test.jsx.snap +++ b/src/Chip/__snapshots__/Chip.test.jsx.snap @@ -1,5 +1,21 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[` snapshots renders div with "button" role when onClick is provided 1`] = ` +
+
+ Test +
+
+`; + exports[` snapshots renders with props iconAfter 1`] = `
snapshots renders with props iconAfter 1`] = ` > Test
-
- - - - - -
+ + + `; @@ -42,29 +51,25 @@ exports[` snapshots renders with props iconBefore 1`] = `
-
- - - - - -
+ + +
@@ -77,60 +82,49 @@ exports[` snapshots renders with props iconBefore and iconAfter 1`] = `
-
- - - - - -
+ + +
Test
-
- - - - - -
+ + +
`; diff --git a/src/Chip/_variables.scss b/src/Chip/_variables.scss index 90c2878e07..33a80ac466 100644 --- a/src/Chip/_variables.scss +++ b/src/Chip/_variables.scss @@ -1,19 +1,28 @@ -$chip-padding-x: .5rem !default; -$chip-padding-y: .125rem !default; -$chip-padding-to-icon: 3px !default; -$chip-icon-padding: .25rem !default; -$chip-margin: .125rem !default; -$chip-border-radius: .25rem !default; -$chip-disable-opacity: .3 !default; -$chip-icon-size: 1.25rem !default; - -$chip-theme-variants: ( - "light": ( - "background": $light-500, - "color": $black, - ), - "dark": ( - "background": $dark-200, - "color": $white, - ) -) !default; +$chip-padding-x: .5rem !default; +$chip-padding-y: 1px !default; +$chip-icon-margin: .25rem !default; +$chip-margin: .125rem !default; +$chip-border-radius: .375rem !default; +$chip-disable-opacity: .3 !default; +$chip-icon-size: 1.5rem !default; +$chip-label-color: $primary-700 !default; +$chip-border-color: $light-800 !default; +$chip-outline-width: 3px !default; +$chip-light-bg-color: $white !default; +$chip-light-outline-color: $chip-label-color !default; +$chip-light-selected-outline-distance: 3px !default; +$chip-light-selected-focus-border-color: $dark-500 !default; +$chip-light-hover-bg: $dark-500 !default; +$chip-light-hover-border-color: $chip-light-hover-bg !default; +$chip-light-hover-label-color: $chip-light-bg-color !default; +$chip-light-hover-icon-color: $chip-light-hover-label-color !default; +$chip-light-focus-outline-distance: .313rem !default; +$chip-dark-bg: $primary-300 !default; +$chip-dark-outline-color: $white !default; +$chip-dark-selected-outline-distance: 3px !default; +$chip-dark-selected-focus-border-color: $chip-dark-outline-color !default; +$chip-dark-label-color: $chip-dark-outline-color !default; +$chip-dark-hover-bg: $white !default; +$chip-dark-hover-border-color: $chip-dark-hover-bg !default; +$chip-dark-hover-label-color: $primary-500 !default; +$chip-dark-focus-outline-distance: .313rem !default; diff --git a/src/Chip/constants.js b/src/Chip/constants.js new file mode 100644 index 0000000000..6259d0c8dd --- /dev/null +++ b/src/Chip/constants.js @@ -0,0 +1,5 @@ +// eslint-disable-next-line import/prefer-default-export +export const STYLE_VARIANTS = { + DARK: 'dark', + LIGHT: 'light', +}; diff --git a/src/Chip/index.scss b/src/Chip/index.scss index d809b022fe..abfa54040d 100644 --- a/src/Chip/index.scss +++ b/src/Chip/index.scss @@ -1,98 +1,141 @@ @import "variables"; +@import "mixins"; .pgn__chip { - background: $light-500; border-radius: $chip-border-radius; display: inline-flex; + justify-content: space-between; + align-items: center; margin: $chip-margin; - box-sizing: border-box; + border: 1px solid $chip-border-color; + padding: $chip-padding-y $chip-padding-x; + position: relative; + outline: none; + transition: all .3s; .pgn__chip__label { - font-size: $font-size-sm; - padding: $chip-padding-y $chip-padding-x; + font-size: $font-size-xs; + line-height: 1.5rem; + font-weight: $font-weight-bold; + color: $chip-label-color; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; - box-sizing: border-box; - cursor: default; - &.p-before { - padding-left: $chip-padding-to-icon; + [dir="rtl"] & { + margin-left: $chip-icon-margin; + } + } - [dir="rtl"] & { - padding-left: $chip-padding-x; - padding-right: $chip-padding-to-icon; - } + .pgn__chip__icon-before { + margin-right: $chip-icon-margin; + + [dir="rtl"] & { + margin-right: 0; + margin-left: .25rem; } + } - &.p-after { - padding-right: $chip-padding-to-icon; + .pgn__chip__icon-after { + margin-left: $chip-icon-margin; - [dir="rtl"] & { - padding-right: $chip-padding-x; - padding-left: $chip-padding-to-icon; - } + [dir="rtl"] & { + margin-left: 0; } } .pgn__chip__icon-before, .pgn__chip__icon-after { - align-items: center; - display: flex; - padding-left: $chip-icon-padding; - padding-right: $chip-icon-padding; - box-sizing: border-box; - cursor: default; - - .pgn__icon { + &.btn-icon { width: $chip-icon-size; height: $chip-icon-size; } + } + + &.pgn__chip-light { + background-color: $chip-light-bg-color; + + &.selected { + @include chip-outline( + $chip-light-outline-color, + calc($chip-light-selected-outline-distance * -1), + calc($chip-border-radius + $chip-outline-width), + $chip-light-selected-outline-distance + ); + + &:focus { + border: 1px solid $chip-light-selected-focus-border-color; + } + } - &.active:hover, - &.active:focus { + .pgn__chip__icon-before, + .pgn__chip__icon-after { + &.pgn__icon { + color: $chip-label-color; + } + } + + &.interactive { cursor: pointer; - background: $black; - * { - color: $white; - fill: $white; + @include chip-hover($dark-500, $white); + + &:focus { + @include chip-outline( + $chip-light-selected-focus-border-color, + calc($chip-light-focus-outline-distance * -1), + calc($chip-border-radius + $chip-outline-width) + ); } } } - .pgn__chip__icon-before { - border-radius: $chip-border-radius 0 0 $chip-border-radius; + &.pgn__chip-dark { + background-color: $chip-dark-bg; - [dir="rtl"] & { - border-radius: 0 $chip-border-radius $chip-border-radius 0; + &.selected { + @include chip-outline($chip-dark-outline-color, + calc($chip-dark-selected-outline-distance * -1), + calc($chip-border-radius + $chip-outline-width), + $chip-dark-selected-outline-distance + ); + + &:focus { + border: 1px solid $chip-dark-selected-focus-border-color; + } } - } - .pgn__chip__icon-after { - border-radius: 0 $chip-border-radius $chip-border-radius 0; + .pgn__chip__label { + color: $chip-dark-label-color; + } - [dir="rtl"] & { - border-radius: $chip-border-radius 0 0 $chip-border-radius; + .pgn__chip__icon-before, + .pgn__chip__icon-after { + &.pgn__icon { + color: $chip-dark-outline-color; + } } - } - @each $color, $styles in $chip-theme-variants { - &.pgn__chip-#{$color} { - background: map-get($styles, "background"); + &.interactive { + cursor: pointer; + + @include chip-hover($white, $primary-500); - * { - color: map-get($styles, "color"); - fill: map-get($styles, "color"); + &:focus { + @include chip-outline( + $chip-dark-outline-color, + calc($chip-dark-focus-outline-distance * -1), + calc($chip-border-radius + $chip-outline-width) + ); } } } &.disabled, &:disabled { - cursor: default; opacity: $chip-disable-opacity; pointer-events: none; + user-select: none; &::before { display: none; diff --git a/src/Chip/index.tsx b/src/Chip/index.tsx index 189053d5d0..23abfde5c7 100644 --- a/src/Chip/index.tsx +++ b/src/Chip/index.tsx @@ -2,76 +2,97 @@ import React, { ForwardedRef, KeyboardEventHandler, MouseEventHandler } from 're import PropTypes from 'prop-types'; import classNames from 'classnames'; // @ts-ignore -import Icon from '../Icon'; +import { requiredWhen } from '../utils/propTypes'; +// @ts-ignore +import { STYLE_VARIANTS } from './constants'; +// @ts-ignore +import ChipIcon from './ChipIcon'; -const STYLE_VARIANTS = [ - 'light', - 'dark', -]; +export const CHIP_PGN_CLASS = 'pgn__chip'; export interface IChip { children: React.ReactNode, + onClick?: KeyboardEventHandler & MouseEventHandler, className?: string, variant?: string, iconBefore?: React.ReactElement | Function, + iconBeforeAlt?: string, iconAfter?: React.ReactElement | Function, + iconAfterAlt?: string, onIconBeforeClick?: KeyboardEventHandler & MouseEventHandler, onIconAfterClick?: KeyboardEventHandler & MouseEventHandler, disabled?: boolean, + isSelected?: boolean, } -export const CHIP_PGN_CLASS = 'pgn__chip'; - const Chip = React.forwardRef(({ children, className, variant, iconBefore, + iconBeforeAlt, iconAfter, + iconAfterAlt, onIconBeforeClick, onIconAfterClick, disabled, + isSelected, + onClick, ...props -}: IChip, ref: ForwardedRef) => ( -
- {iconBefore && ( -
- -
- )} +}: IChip, ref: ForwardedRef) => { + const hasInteractiveIcons = !!(onIconBeforeClick || onIconAfterClick); + const isChipInteractive = !hasInteractiveIcons && !!onClick; + + const interactionProps = isChipInteractive ? { + onClick, + onKeyPress: onClick, + tabIndex: 0, + role: 'button', + } : {}; + + return (
- {children} -
- {iconAfter && ( + {iconBefore && ( + + )}
- + {children}
- )} -
-)); + {iconAfter && ( + + )} +
+ ); +}); Chip.propTypes = { /** Specifies the content of the `Chip`. */ @@ -79,9 +100,11 @@ Chip.propTypes = { /** Specifies an additional `className` to add to the base element. */ className: PropTypes.string, /** The `Chip` style variant to use. */ - variant: PropTypes.oneOf(STYLE_VARIANTS), + variant: PropTypes.oneOf(['light', 'dark']), /** Disables the `Chip`. */ disabled: PropTypes.bool, + /** Click handler for the whole Chip, has effect only when Chip does not have any interactive icons. */ + onClick: PropTypes.func, /** * An icon component to render before the content. * Example import of a Paragon icon component: @@ -89,6 +112,8 @@ Chip.propTypes = { * `import { Check } from '@openedx/paragon/icons';` */ iconBefore: PropTypes.oneOfType([PropTypes.element, PropTypes.func]), + /** Specifies icon alt text. */ + iconBeforeAlt: requiredWhen(PropTypes.string, ['iconBefore', 'onIconBeforeClick']), /** A click handler for the `Chip` icon before. */ onIconBeforeClick: PropTypes.func, /** @@ -98,18 +123,26 @@ Chip.propTypes = { * `import { Check } from '@openedx/paragon/icons';` */ iconAfter: PropTypes.oneOfType([PropTypes.element, PropTypes.func]), + /** Specifies icon alt text. */ + iconAfterAlt: requiredWhen(PropTypes.string, ['iconAfter', 'onIconAfterClick']), /** A click handler for the `Chip` icon after. */ onIconAfterClick: PropTypes.func, + /** Indicates if `Chip` has been selected. */ + isSelected: PropTypes.bool, }; Chip.defaultProps = { className: undefined, - variant: 'light', + variant: STYLE_VARIANTS.LIGHT, disabled: false, + onClick: undefined, iconBefore: undefined, iconAfter: undefined, onIconBeforeClick: undefined, onIconAfterClick: undefined, + isSelected: false, + iconAfterAlt: undefined, + iconBeforeAlt: undefined, }; export default Chip; diff --git a/src/Chip/mixins.scss b/src/Chip/mixins.scss new file mode 100644 index 0000000000..a5f850aa8b --- /dev/null +++ b/src/Chip/mixins.scss @@ -0,0 +1,42 @@ +@mixin chip-outline($outline-color: $white, $distance-to-border: 0, $border-radius: 50%, $border-width: .125rem) { + &::before { + content: ""; + position: absolute; + top: $distance-to-border; + right: $distance-to-border; + bottom: $distance-to-border; + left: $distance-to-border; + border: solid $border-width $outline-color; + border-radius: $border-radius; + } +} + +@mixin chip-hover($base-color, $secondary-color) { + &:hover { + background-color: $base-color; + border-color: $base-color; + + .pgn__chip__label { + color: $secondary-color; + } + + .pgn__chip__icon-before, + .pgn__chip__icon-after { + &.pgn__icon, + &.btn-icon { + color: $secondary-color; + } + + &.btn-icon:hover { + background-color: $secondary-color; + color: $base-color; + } + + &.btn-icon:focus { + color: $secondary-color; + border: 2px solid $secondary-color; + background-color: $base-color; + } + } + } +} diff --git a/src/ChipCarousel/_variables.scss b/src/ChipCarousel/_variables.scss index e033dc2fcd..ef4ec9c747 100644 --- a/src/ChipCarousel/_variables.scss +++ b/src/ChipCarousel/_variables.scss @@ -1 +1,3 @@ -$chip-carousel-controls-top-offset: -3px !default; +$chip-carousel-controls-top-offset: .375rem !default; +$chip-carousel-container-padding-x: .625rem !default; +$chip-carousel-container-padding-y: .313rem !default; diff --git a/src/ChipCarousel/index.scss b/src/ChipCarousel/index.scss index 744acf9dea..f36ae6303a 100644 --- a/src/ChipCarousel/index.scss +++ b/src/ChipCarousel/index.scss @@ -11,6 +11,7 @@ &.pgn__chip-carousel-gap__#{$level} { .pgn__overflow-scroll-overflow-container { column-gap: $space; + padding: $chip-carousel-container-padding-x $chip-carousel-container-padding-y; } } } diff --git a/src/utils/propTypes/utils.js b/src/utils/propTypes/utils.js index 3310653236..f6a9f262ad 100644 --- a/src/utils/propTypes/utils.js +++ b/src/utils/propTypes/utils.js @@ -22,6 +22,16 @@ export const customPropTypeRequirement = (targetType, conditionFn, filterString) } ); +/** + * Checks if all specified properties are defined in the `props` object. + * + * @param {Object} props - The object in which the properties are checked. + * @param {string[]} otherPropNames - An array of strings representing the property names to be checked. + * @returns {boolean} `true` if all properties are defined and not equal to `undefined`, `false` otherwise. + */ +export const isEveryPropDefined = (props, otherPropNames) => otherPropNames + .every(propName => props[propName] !== undefined); + /** * Returns a PropType entry with the given propType that is required if otherPropName * is truthy. @@ -34,8 +44,13 @@ export const customPropTypeRequirement = (targetType, conditionFn, filterString) export const requiredWhen = (propType, otherPropName) => ( customPropTypeRequirement( propType, - (props) => props[otherPropName] === true, - `${otherPropName} is truthy`, + (props) => { + if (Array.isArray(otherPropName)) { + return isEveryPropDefined(props, otherPropName); + } + return props[otherPropName] === true; + }, + `${otherPropName} ${Array.isArray(otherPropName) ? 'are defined' : 'is truthy'}`, ) );