From 635e0cbb37542f8ecc574b449e4250bd2442116b Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 22 Mar 2021 18:10:41 -0500 Subject: [PATCH] fix(Dropdown): prevent unnecessary duplicate VO readings (#7768) * fix(ListBoxMenuItem): add title tooltip only when content overflows * docs(ListBoxMenu): update children proptype * docs(Dropdown): reduce item width * chore: update snapshots * fix(ListBoxMenuItem): add title tooltip only when content overflows * refactor(ComboBox): convert to functional component to allow hooks * fix(ListBoxMenuItem): forward ref for class based listbox components * test(ListBox): update assertions for forwarded ref in listbox menu items * chore: update snapshots Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../src/components/ComboBox/ComboBox-test.js | 21 +- .../react/src/components/ComboBox/ComboBox.js | 900 +++++++++--------- .../src/components/Dropdown/Dropdown-story.js | 3 +- .../src/components/Dropdown/Dropdown-test.js | 5 +- .../react/src/components/Dropdown/Dropdown.js | 12 +- .../__snapshots__/Dropdown-test.js.snap | 32 +- .../src/components/ListBox/ListBoxMenu.js | 1 + .../src/components/ListBox/ListBoxMenuItem.js | 42 +- .../__snapshots__/ListBoxMenu-test.js.snap | 4 +- .../ListBoxMenuItem-test.js.snap | 12 +- .../__tests__/FilterableMultiSelect-test.js | 4 +- .../MultiSelect/__tests__/MultiSelect-test.js | 2 +- 12 files changed, 525 insertions(+), 513 deletions(-) diff --git a/packages/react/src/components/ComboBox/ComboBox-test.js b/packages/react/src/components/ComboBox/ComboBox-test.js index bacc66c3fb87..06b5ee247864 100644 --- a/packages/react/src/components/ComboBox/ComboBox-test.js +++ b/packages/react/src/components/ComboBox/ComboBox-test.js @@ -10,7 +10,6 @@ import { mount } from 'enzyme'; import { findListBoxNode, findMenuNode, - findMenuItemNode, assertMenuOpen, assertMenuClosed, generateItems, @@ -20,13 +19,7 @@ import ComboBox from '../ComboBox'; import { settings } from 'carbon-components'; const { prefix } = settings; - const findInputNode = (wrapper) => wrapper.find(`.${prefix}--text-input`); -const downshiftActions = { - setHighlightedIndex: jest.fn(), -}; -const clearInput = (wrapper) => - wrapper.instance().handleOnStateChange({ inputValue: '' }, downshiftActions); const openMenu = (wrapper) => { wrapper.find(`[role="combobox"]`).simulate('click'); }; @@ -64,9 +57,8 @@ describe('ComboBox', () => { expect(mockProps.onChange).not.toHaveBeenCalled(); for (let i = 0; i < mockProps.items.length; i++) { - clearInput(wrapper); openMenu(wrapper); - findMenuItemNode(wrapper, i).simulate('click'); + wrapper.find('ForwardRef(ListBoxMenuItem)').at(i).simulate('click'); expect(mockProps.onChange).toHaveBeenCalledTimes(i + 1); expect(mockProps.onChange).toHaveBeenCalledWith({ selectedItem: mockProps.items[i], @@ -100,7 +92,7 @@ describe('ComboBox', () => { it('should let the user select an option by clicking on the option node', () => { const wrapper = mount(); openMenu(wrapper); - findMenuItemNode(wrapper, 0).simulate('click'); + wrapper.find('ForwardRef(ListBoxMenuItem)').at(0).simulate('click'); expect(mockProps.onChange).toHaveBeenCalledTimes(1); expect(mockProps.onChange).toHaveBeenCalledWith({ selectedItem: mockProps.items[0], @@ -110,7 +102,7 @@ describe('ComboBox', () => { mockProps.onChange.mockClear(); openMenu(wrapper); - findMenuItemNode(wrapper, 1).simulate('click'); + wrapper.find('ForwardRef(ListBoxMenuItem)').at(1).simulate('click'); expect(mockProps.onChange).toHaveBeenCalledTimes(1); expect(mockProps.onChange).toHaveBeenCalledWith({ selectedItem: mockProps.items[1], @@ -207,12 +199,7 @@ describe('ComboBox', () => { it('should set `inputValue` to an empty string if a falsey-y value is given', () => { const wrapper = mount(); - - wrapper.instance().handleOnInputValueChange('foo', downshiftActions); - expect(wrapper.state('inputValue')).toBe('foo'); - - wrapper.instance().handleOnInputValueChange(null, downshiftActions); - expect(wrapper.state('inputValue')).toBe(''); + expect(wrapper.find('input').instance().value).toBe(''); }); }); }); diff --git a/packages/react/src/components/ComboBox/ComboBox.js b/packages/react/src/components/ComboBox/ComboBox.js index e3ba0b2dd378..ab8a584a3600 100644 --- a/packages/react/src/components/ComboBox/ComboBox.js +++ b/packages/react/src/components/ComboBox/ComboBox.js @@ -8,7 +8,7 @@ import cx from 'classnames'; import Downshift from 'downshift'; import PropTypes from 'prop-types'; -import React from 'react'; +import React, { useEffect, useState, useRef } from 'react'; import { settings } from 'carbon-components'; import { Checkmark16, @@ -34,16 +34,21 @@ const defaultItemToString = (item) => { const defaultShouldFilterItem = () => true; -const getInputValue = (props, state) => { - if (props.selectedItem) { - return props.itemToString(props.selectedItem); +const getInputValue = ({ + initialSelectedItem, + inputValue, + itemToString, + selectedItem, +}) => { + if (selectedItem) { + return itemToString(selectedItem); } // TODO: consistent `initialSelectedItem` behavior with other listbox components in v11 - if (props.initialSelectedItem) { - return props.itemToString(props.initialSelectedItem); + if (initialSelectedItem) { + return itemToString(initialSelectedItem); } - return state.inputValue || ''; + return inputValue || ''; }; const findHighlightedIndex = ({ items, itemToString }, inputValue) => { @@ -65,242 +70,103 @@ const findHighlightedIndex = ({ items, itemToString }, inputValue) => { const getInstanceId = setupGetInstanceId(); -export default class ComboBox extends React.Component { - static propTypes = { - /** - * 'aria-label' of the ListBox component. - */ - ariaLabel: PropTypes.string, - - /** - * An optional className to add to the container node - */ - className: PropTypes.string, - - /** - * Specify the direction of the combobox dropdown. Can be either top or bottom. - */ - direction: PropTypes.oneOf(['top', 'bottom']), - - /** - * Specify if the control should be disabled, or not - */ - disabled: PropTypes.bool, - - /** - * Additional props passed to Downshift - */ - downshiftProps: PropTypes.shape(Downshift.propTypes), - - /** - * Provide helper text that is used alongside the control label for - * additional help - */ - helperText: PropTypes.string, - - /** - * Specify a custom `id` for the input - */ - id: PropTypes.string.isRequired, - - /** - * Allow users to pass in an arbitrary item or a string (in case their items are an array of strings) - * from their collection that are pre-selected - */ - initialSelectedItem: PropTypes.oneOfType([ - PropTypes.object, - PropTypes.string, - ]), - - /** - * Specify if the currently selected value is invalid. - */ - invalid: PropTypes.bool, - - /** - * Message which is displayed if the value is invalid. - */ - invalidText: PropTypes.node, - - /** - * Optional function to render items as custom components instead of strings. - * Defaults to null and is overriden by a getter - */ - itemToElement: PropTypes.func, - - /** - * Helper function passed to downshift that allows the library to render a - * given item to a string label. By default, it extracts the `label` field - * from a given item to serve as the item label in the list - */ - itemToString: PropTypes.func, - - /** - * We try to stay as generic as possible here to allow individuals to pass - * in a collection of whatever kind of data structure they prefer - */ - items: PropTypes.array.isRequired, - - /** - * should use "light theme" (white background)? - */ - light: PropTypes.bool, - - /** - * `onChange` is a utility for this controlled component to communicate to a - * consuming component when a specific dropdown item is selected. - * @param {{ selectedItem }} - */ - onChange: PropTypes.func.isRequired, - - /** - * Callback function to notify consumer when the text input changes. - * This provides support to change available items based on the text. - * @param {string} inputText - */ - onInputChange: PropTypes.func, - - /** - * Callback function that fires when the combobox menu toggle is clicked - * @param {MouseEvent} event - */ - onToggleClick: PropTypes.func, - - /** - * Used to provide a placeholder text node before a user enters any input. - * This is only present if the control has no items selected - */ - placeholder: PropTypes.string.isRequired, - - /** - * For full control of the selection - */ - selectedItem: PropTypes.oneOfType([PropTypes.object, PropTypes.string]), - - /** - * Specify your own filtering logic by passing in a `shouldFilterItem` - * function that takes in the current input and an item and passes back - * whether or not the item should be filtered. - */ - shouldFilterItem: PropTypes.func, - - /** - * Specify the size of the ListBox. Currently supports either `sm`, `lg` or `xl` as an option. - */ - size: ListBoxPropTypes.ListBoxSize, - - /** - * Provide text to be used in a `