From 8d0dd6cee93f4823e012c137cf145cce0765a839 Mon Sep 17 00:00:00 2001 From: emyarod Date: Thu, 1 Apr 2021 13:57:59 -0500 Subject: [PATCH 1/7] docs(Tile): update deprecation warning --- packages/react/src/components/Tile/Tile.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/components/Tile/Tile.js b/packages/react/src/components/Tile/Tile.js index 83c39b07494d..78779b48f6fe 100644 --- a/packages/react/src/components/Tile/Tile.js +++ b/packages/react/src/components/Tile/Tile.js @@ -217,8 +217,8 @@ export class SelectableTile extends Component { */ iconDescription: deprecate( PropTypes.string, - 'The `iconDescription` prop for `RadioTile` is no longer needed and has ' + - 'been deprecated. It will be moved in the next major release.' + 'The `iconDescription` prop for `SelectableTile` is no longer needed and has ' + + 'been deprecated. It will be removed in the next major release.' ), /** From c260d0000d12f639fe17cc1cefadc0d32dbe5ade Mon Sep 17 00:00:00 2001 From: emyarod Date: Thu, 1 Apr 2021 15:12:58 -0500 Subject: [PATCH 2/7] fix(Tile): sync props and state when uncontrolled --- packages/react/src/components/Tile/Tile.js | 390 ++++++++++----------- 1 file changed, 192 insertions(+), 198 deletions(-) diff --git a/packages/react/src/components/Tile/Tile.js b/packages/react/src/components/Tile/Tile.js index 78779b48f6fe..324499d68f49 100644 --- a/packages/react/src/components/Tile/Tile.js +++ b/packages/react/src/components/Tile/Tile.js @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import React, { Component } from 'react'; +import React, { Component, useEffect, useRef, useState } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import { settings } from 'carbon-components'; @@ -181,216 +181,210 @@ export class ClickableTile extends Component { } } -export class SelectableTile extends Component { - state = { - selected: this.props.selected, - }; - - static propTypes = { - /** - * The child nodes. - */ - children: PropTypes.node, - - /** - * The CSS class names. - */ - className: PropTypes.string, - - /** - * Specify whether the SelectableTile should be disabled - */ - disabled: PropTypes.bool, - - /** - * Specify the function to run when the SelectableTile is clicked - */ - handleClick: PropTypes.func, - - /** - * Specify the function to run when the SelectableTile is interacted with via a keyboard - */ - handleKeyDown: PropTypes.func, - - /** - * The description of the checkmark icon. - */ - iconDescription: deprecate( - PropTypes.string, - 'The `iconDescription` prop for `SelectableTile` is no longer needed and has ' + - 'been deprecated. It will be removed in the next major release.' - ), - - /** - * The ID of the ``. - */ - id: PropTypes.string, - - /** - * `true` to use the light version. For use on $ui-01 backgrounds only. - * Don't use this to make tile background color same as container background color. - */ - light: PropTypes.bool, - - /** - * The `name` of the ``. - */ - name: PropTypes.string, - - /** - * The empty handler of the ``. - */ - onChange: PropTypes.func, - - /** - * `true` to select this tile. - */ - selected: PropTypes.bool, - - /** - * Specify the tab index of the wrapper element - */ - tabIndex: PropTypes.number, - - /** - * The `title` of the ``. - */ - title: PropTypes.string, - - /** - * The value of the ``. - */ - value: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, - }; - - static defaultProps = { - value: 'value', - title: 'title', - selected: false, - handleClick: () => {}, - handleKeyDown: () => {}, - onChange: () => {}, - tabIndex: 0, - light: false, - }; - - static getDerivedStateFromProps({ selected }, state) { - const { prevSelected } = state; - return prevSelected === selected - ? null - : { - selected, - prevSelected: selected, - }; - } - - handleClick = (evt) => { +export function SelectableTile(props) { + const { + children, + id, + tabIndex = 0, + value, + name, + title, + // eslint-disable-next-line no-unused-vars + iconDescription, + className, + handleClick, + handleKeyDown, + onClick = () => {}, + onChange = () => {}, + onKeyDown = () => {}, + light, + disabled, + selected, + ...other + } = props; + + // TODO: replace with onClick when handleClick prop is deprecated + const clickHandler = handleClick || onClick; + + // TODO: replace with onClick when handleClick prop is deprecated + const keyDownHandler = handleKeyDown || onKeyDown; + + const [isSelected, setIsSelected] = useState(selected); + const input = useRef(null); + const classes = classNames( + `${prefix}--tile`, + `${prefix}--tile--selectable`, + { + [`${prefix}--tile--is-selected`]: isSelected, + [`${prefix}--tile--light`]: light, + [`${prefix}--tile--disabled`]: disabled, + }, + className + ); + const inputClasses = classNames(`${prefix}--tile-input`, { + [`${prefix}--tile-input--checked`]: isSelected, + }); + + // TODO: rename to handleClick when handleClick prop is deprecated + const handleOnClick = (evt) => { evt.preventDefault(); evt.persist(); - this.setState( - { - selected: !this.state.selected, - }, - () => { - this.props.handleClick(evt); - this.props.onChange(evt); - } - ); + setIsSelected(!isSelected); + clickHandler(evt); + onChange(evt); }; - handleKeyDown = (evt) => { + // TODO: rename to handleKeyDown when handleKeyDown prop is deprecated + const handleOnKeyDown = (evt) => { evt.persist(); if (matches(evt, [keys.Enter, keys.Space])) { evt.preventDefault(); - this.setState( - { - selected: !this.state.selected, - }, - () => { - this.props.handleKeyDown(evt); - this.props.onChange(evt); - } - ); - } else { - this.props.handleKeyDown(evt); + setIsSelected(!isSelected); + onChange(evt); } + keyDownHandler(evt); }; - handleOnChange = (event) => { - this.setState({ selected: event.target.checked }); - this.props.onChange(event); + const handleChange = (event) => { + setIsSelected(event.target.checked); + onChange(event); }; - render() { - const { - children, - id, - tabIndex, - value, - name, - title, - // eslint-disable-next-line no-unused-vars - iconDescription, - className, - handleClick, // eslint-disable-line - handleKeyDown, // eslint-disable-line - // eslint-disable-next-line no-unused-vars - onChange, - light, - disabled, - ...other - } = this.props; - - const inputClasses = classNames(`${prefix}--tile-input`, { - [`${prefix}--tile-input--checked`]: this.state.selected, - }); - - const classes = classNames( - `${prefix}--tile`, - `${prefix}--tile--selectable`, - { - [`${prefix}--tile--is-selected`]: this.state.selected, - [`${prefix}--tile--light`]: light, - [`${prefix}--tile--disabled`]: disabled, - }, - className - ); - - return ( - <> - { - this.input = input; - }} - tabIndex={-1} - id={id} - className={inputClasses} - value={value} - onChange={!disabled ? this.handleOnChange : null} - type="checkbox" - disabled={disabled} - name={name} - title={title} - checked={this.state.selected} - /> - {/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */} - - - ); - } + useEffect(() => { + setIsSelected(selected); + }, [selected]); + + return ( + <> + + {/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */} + + + ); } +SelectableTile.defaultProps = { + value: 'value', + title: 'title', + selected: false, + tabIndex: 0, + light: false, +}; +SelectableTile.propTypes = { + /** + * The child nodes. + */ + children: PropTypes.node, + + /** + * The CSS class names. + */ + className: PropTypes.string, + + /** + * Specify whether the SelectableTile should be disabled + */ + disabled: PropTypes.bool, + + /** + * Specify the function to run when the SelectableTile is clicked + */ + handleClick: deprecate( + PropTypes.func, + 'The `handleClick` prop for `SelectableTile` has been deprecated in favor of `onClick`. It will be removed in the next major release.' + ), + + /** + * Specify the function to run when the SelectableTile is interacted with via a keyboard + */ + handleKeyDown: deprecate( + PropTypes.func, + 'The `handleKeyDown` prop for `SelectableTile` has been deprecated in favor of `onKeyDown`. It will be removed in the next major release.' + ), + + /** + * The description of the checkmark icon. + */ + iconDescription: deprecate( + PropTypes.string, + 'The `iconDescription` prop for `SelectableTile` is no longer needed and has ' + + 'been deprecated. It will be removed in the next major release.' + ), + + /** + * The ID of the ``. + */ + id: PropTypes.string, + + /** + * `true` to use the light version. For use on $ui-01 backgrounds only. + * Don't use this to make tile background color same as container background color. + */ + light: PropTypes.bool, + + /** + * The `name` of the ``. + */ + name: PropTypes.string, + + /** + * The empty handler of the ``. + */ + onChange: PropTypes.func, + + /** + * Specify the function to run when the SelectableTile is clicked + */ + onClick: PropTypes.func, + + /** + * Specify the function to run when the SelectableTile is interacted with via a keyboard + */ + onKeyDown: PropTypes.func, + + /** + * `true` to select this tile. + */ + selected: PropTypes.bool, + + /** + * Specify the tab index of the wrapper element + */ + tabIndex: PropTypes.number, + + /** + * The `title` of the ``. + */ + title: PropTypes.string, + + /** + * The value of the ``. + */ + value: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, +}; export class ExpandableTile extends Component { state = {}; From 943568b44ad556817e173545875e953065b9363b Mon Sep 17 00:00:00 2001 From: emyarod Date: Thu, 1 Apr 2021 15:15:08 -0500 Subject: [PATCH 3/7] test(Tile): update SelectableTile tests --- .../react/src/components/Tile/Tile-test.js | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/packages/react/src/components/Tile/Tile-test.js b/packages/react/src/components/Tile/Tile-test.js index 8133f16117aa..73327ccefac7 100644 --- a/packages/react/src/components/Tile/Tile-test.js +++ b/packages/react/src/components/Tile/Tile-test.js @@ -134,11 +134,10 @@ describe('Tile', () => { beforeEach(() => { wrapper = mount( - +
Test
 ); - wrapper.state().selected = false; label = wrapper.find('label'); }); @@ -155,42 +154,51 @@ describe('Tile', () => { }); it('toggles the selectable state on click', () => { - expect(wrapper.state().selected).toEqual(false); + expect(wrapper.hasClass(`${prefix}--tile--is-selected`)).toEqual(false); label.simulate('click'); - expect(wrapper.state().selected).toEqual(true); + expect(wrapper.props().onClick).toHaveBeenCalledTimes(1); + expect(wrapper.render().hasClass(`${prefix}--tile--is-selected`)).toEqual( + true + ); }); it('toggles the selectable state when using enter or space', () => { - expect(wrapper.state().selected).toEqual(false); + expect(wrapper.hasClass(`${prefix}--tile--is-selected`)).toEqual(false); label.simulate('keydown', { which: 32 }); - expect(wrapper.state().selected).toEqual(true); + expect(wrapper.render().hasClass(`${prefix}--tile--is-selected`)).toEqual( + true + ); label.simulate('keydown', { which: 13 }); - expect(wrapper.state().selected).toEqual(false); + expect(wrapper.render().hasClass(`${prefix}--tile--is-selected`)).toEqual( + false + ); }); it('the input should be checked when state is selected', () => { - wrapper.setState({ selected: true }); + label.simulate('click'); expect(wrapper.find('input').props().checked).toEqual(true); }); it('supports setting initial selected state from props', () => { - expect(shallow().state().selected).toEqual( - true - ); + expect( + shallow() + .render() + .hasClass(`${prefix}--tile--is-selected`) + ).toEqual(true); }); it('supports setting selected state from props', () => { wrapper.setProps({ selected: true }); - wrapper.setState({ selected: true }); - wrapper.setProps({ selected: false }); - expect(wrapper.state().selected).toEqual(false); + expect(wrapper.render().hasClass(`${prefix}--tile--is-selected`)).toEqual( + true + ); }); it('avoids changing selected state upon setting props, unless actual value change is detected', () => { wrapper.setProps({ selected: true }); - wrapper.setState({ selected: false }); + label.simulate('click'); wrapper.setProps({ selected: true }); - expect(wrapper.state().selected).toEqual(false); + expect(wrapper.hasClass(`${prefix}--tile--is-selected`)).toEqual(false); }); it('supports light version', () => { From 812e5237f3bae8095efc3413088435c462ab68a4 Mon Sep 17 00:00:00 2001 From: emyarod Date: Thu, 1 Apr 2021 15:17:10 -0500 Subject: [PATCH 4/7] chore: update snapshots --- .../__snapshots__/PublicAPI-test.js.snap | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index fcc6ac5b20e7..9b46f5ddb8ff 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -5836,10 +5836,7 @@ Map { }, "SelectableTile" => Object { "defaultProps": Object { - "handleClick": [Function], - "handleKeyDown": [Function], "light": false, - "onChange": [Function], "selected": false, "tabIndex": 0, "title": "title", @@ -5855,12 +5852,8 @@ Map { "disabled": Object { "type": "bool", }, - "handleClick": Object { - "type": "func", - }, - "handleKeyDown": Object { - "type": "func", - }, + "handleClick": [Function], + "handleKeyDown": [Function], "iconDescription": [Function], "id": Object { "type": "string", @@ -5874,6 +5867,12 @@ Map { "onChange": Object { "type": "func", }, + "onClick": Object { + "type": "func", + }, + "onKeyDown": Object { + "type": "func", + }, "selected": Object { "type": "bool", }, From 8ea3b215ff1fe513a83d79e0bc02461577a50282 Mon Sep 17 00:00:00 2001 From: emyarod Date: Thu, 1 Apr 2021 15:19:49 -0500 Subject: [PATCH 5/7] docs(Tile): add temp example story --- .../react/src/components/Tile/Tile-story.js | 81 ++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/Tile/Tile-story.js b/packages/react/src/components/Tile/Tile-story.js index 690e918b11cd..50a5e0c734c9 100644 --- a/packages/react/src/components/Tile/Tile-story.js +++ b/packages/react/src/components/Tile/Tile-story.js @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import React from 'react'; +import React, { useEffect, useState } from 'react'; import { action } from '@storybook/addon-actions'; import { @@ -224,3 +224,82 @@ Expandable.parameters = { `, }, }; + +export const Test = () => { + const [selectedTiles, setSelectedTiles] = useState({ + tile1: true, + tile2: false, + tile3: false, + tile4: true, + }); + useEffect(() => { + console.log(selectedTiles); + }); + return ( +
+ + setSelectedTiles({ ...selectedTiles, tile1: !selectedTiles.tile1 }) + }> + Tile 1 toggle + + { + setSelectedTiles({ + ...selectedTiles, + tile1: true, + tile2: !selectedTiles.tile2, + }); + }}> + Tile 1 true, Tile 2 toggle + + + setSelectedTiles({ + ...selectedTiles, + tile1: true, + tile2: true, + tile3: !selectedTiles.tile3, + }) + }> + Tile 1 true, Tile 2 true, Tile 3 toggle + + { + setSelectedTiles({ + ...selectedTiles, + tile1: true, + tile2: true, + tile3: true, + tile4: true, + }); + }}> + All tiles true + +
+ ); +}; From d52f7d29040971369bfa4d4d6276869612cd33a3 Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 5 Apr 2021 10:38:49 -0500 Subject: [PATCH 6/7] refactor(Tile): use hoisted functions instead of expressions --- packages/react/src/components/Tile/Tile.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react/src/components/Tile/Tile.js b/packages/react/src/components/Tile/Tile.js index 324499d68f49..ebd7134cc9a5 100644 --- a/packages/react/src/components/Tile/Tile.js +++ b/packages/react/src/components/Tile/Tile.js @@ -226,16 +226,16 @@ export function SelectableTile(props) { }); // TODO: rename to handleClick when handleClick prop is deprecated - const handleOnClick = (evt) => { + function handleOnClick(evt) { evt.preventDefault(); evt.persist(); setIsSelected(!isSelected); clickHandler(evt); onChange(evt); - }; + } // TODO: rename to handleKeyDown when handleKeyDown prop is deprecated - const handleOnKeyDown = (evt) => { + function handleOnKeyDown(evt) { evt.persist(); if (matches(evt, [keys.Enter, keys.Space])) { evt.preventDefault(); @@ -243,12 +243,12 @@ export function SelectableTile(props) { onChange(evt); } keyDownHandler(evt); - }; + } - const handleChange = (event) => { + function handleChange(event) { setIsSelected(event.target.checked); onChange(event); - }; + } useEffect(() => { setIsSelected(selected); From 479fb55cc1c72b5c201e9598866671ce35ecc006 Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 12 Apr 2021 10:58:55 -0500 Subject: [PATCH 7/7] docs(Tile): remove test story --- .../react/src/components/Tile/Tile-story.js | 81 +------------------ 1 file changed, 1 insertion(+), 80 deletions(-) diff --git a/packages/react/src/components/Tile/Tile-story.js b/packages/react/src/components/Tile/Tile-story.js index 50a5e0c734c9..690e918b11cd 100644 --- a/packages/react/src/components/Tile/Tile-story.js +++ b/packages/react/src/components/Tile/Tile-story.js @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import React, { useEffect, useState } from 'react'; +import React from 'react'; import { action } from '@storybook/addon-actions'; import { @@ -224,82 +224,3 @@ Expandable.parameters = { `, }, }; - -export const Test = () => { - const [selectedTiles, setSelectedTiles] = useState({ - tile1: true, - tile2: false, - tile3: false, - tile4: true, - }); - useEffect(() => { - console.log(selectedTiles); - }); - return ( -
- - setSelectedTiles({ ...selectedTiles, tile1: !selectedTiles.tile1 }) - }> - Tile 1 toggle - - { - setSelectedTiles({ - ...selectedTiles, - tile1: true, - tile2: !selectedTiles.tile2, - }); - }}> - Tile 1 true, Tile 2 toggle - - - setSelectedTiles({ - ...selectedTiles, - tile1: true, - tile2: true, - tile3: !selectedTiles.tile3, - }) - }> - Tile 1 true, Tile 2 true, Tile 3 toggle - - { - setSelectedTiles({ - ...selectedTiles, - tile1: true, - tile2: true, - tile3: true, - tile4: true, - }); - }}> - All tiles true - -
- ); -};