From 4bbe7f3f0d803a5f8b6778b8ee9661362115f607 Mon Sep 17 00:00:00 2001 From: kamilmateusiak Date: Thu, 23 May 2019 14:38:13 +0200 Subject: [PATCH] Dropdown improvements (#70) * adding dropdown content resize observer * refactoring Dropdown and Switch prop types and default props * custom key codes for Dropdown close and DropdownList select events * prevent default on item select with keydown event * adding autofocus first item to DropdownList component --- package.json | 3 +- src/components/Dropdown/Dropdown.js | 154 +++++++++++++----- src/components/Dropdown/DropdownList.js | 70 +++++++- src/components/Dropdown/DropdownList.test.js | 4 +- src/components/Dropdown/DropdownListItem.js | 1 + .../__snapshots__/DropdownList.test.js.snap | 4 +- src/components/Switch/Switch.js | 55 ++++--- .../Switch/__snapshots__/Switch.test.js.snap | 2 +- src/interfaces/dropdowns.d.ts | 4 + 9 files changed, 220 insertions(+), 77 deletions(-) diff --git a/package.json b/package.json index 34733a022..0382ae558 100644 --- a/package.json +++ b/package.json @@ -141,7 +141,8 @@ "react-day-picker": "^7.2.4", "react-material-icon-svg": "1.7.0", "react-popper": "^1.3.3", - "react-transition-group": "^2.4.0" + "react-transition-group": "^2.4.0", + "resize-observer-polyfill": "^1.5.1" }, "postcss": {}, "jest": { diff --git a/src/components/Dropdown/Dropdown.js b/src/components/Dropdown/Dropdown.js index 12dde1816..be1781cbf 100644 --- a/src/components/Dropdown/Dropdown.js +++ b/src/components/Dropdown/Dropdown.js @@ -3,6 +3,7 @@ import * as PropTypes from 'prop-types'; import memoizeOne from 'memoize-one'; import cssClassNames from 'classnames/bind'; import { Manager, Reference, Popper } from 'react-popper'; +import ResizeObserver from 'resize-observer-polyfill'; import styles from './style.scss'; import getMergedClassNames from '../../utils/getMergedClassNames'; import { KeyCodes } from '../../constants/keyCodes'; @@ -10,13 +11,6 @@ import { KeyCodes } from '../../constants/keyCodes'; const cx = cssClassNames.bind(styles); class Dropdown extends React.PureComponent { - static defaultProps = { - modifiers: {}, - zIndex: 20, - closeOnEscPress: true, - closeOnEnterPress: false - }; - static buildPopperModifiers(modifiers) { const { offset, flip, hide, preventOverflow, arrow, ...rest } = modifiers; return { @@ -40,6 +34,7 @@ class Dropdown extends React.PureComponent { componentDidMount() { if (this.props.isVisible) { this.addEventHandlers(); + this.attachResizeObserver(); } } @@ -49,6 +44,7 @@ class Dropdown extends React.PureComponent { if (isShown) { this.addEventHandlers(); + this.attachResizeObserver(); if (this.popupRef) { this.popupRef.focus({ preventScroll: true }); } @@ -56,11 +52,13 @@ class Dropdown extends React.PureComponent { if (isHidden) { this.removeEventHandlers(); + this.detachResizeObserver(); } } componentWillUnmount() { this.removeEventHandlers(); + this.detachResizeObserver(); } getModifiers = memoizeOne(Dropdown.buildPopperModifiers); @@ -88,15 +86,26 @@ class Dropdown extends React.PureComponent { }; handleKeyDown = event => { - if (this.props.onClose) { - const isEscKeyPressed = event.keyCode === KeyCodes.esc; - const isEnterKeyPressed = event.keyCode === KeyCodes.enter; + const { keyCode } = event; + const { + closeKeyCodes, + closeOnEnterPress, + closeOnEscPress, + onClose + } = this.props; + + if (onClose) { + const isEscKeyPressed = keyCode === KeyCodes.esc; + const isEnterKeyPressed = keyCode === KeyCodes.enter; + const isCustomCloseKeyPressed = + closeKeyCodes && closeKeyCodes.includes(keyCode); if ( - (this.props.closeOnEscPress && isEscKeyPressed) || - (this.props.closeOnEnterPress && isEnterKeyPressed) + (closeOnEscPress && isEscKeyPressed) || + (closeOnEnterPress && isEnterKeyPressed) || + isCustomCloseKeyPressed ) { - this.props.onClose(); + onClose(); if (this.triggerRef) { this.triggerRef.focus(); } @@ -104,6 +113,24 @@ class Dropdown extends React.PureComponent { } }; + attachResizeObserver = () => { + // to boost component performance resize observer should be optional + if (this.props.shouldUpdateOnResize && this.popupRef) { + this.observer = new ResizeObserver(() => { + if (this.popperScheduleUpdate) { + this.popperScheduleUpdate(); + } + }); + this.observer.observe(this.popupRef); + } + }; + + detachResizeObserver = () => { + if (this.observer) { + this.observer.disconnect(); + } + }; + addEventHandlers = () => { document.addEventListener('keydown', this.handleKeyDown, true); document.addEventListener('click', this.handleDocumentClick); @@ -114,9 +141,14 @@ class Dropdown extends React.PureComponent { document.removeEventListener('click', this.handleDocumentClick); }; - render() { - const { children, className, triggerRenderer, isVisible } = this.props; - + renderDropdownContent = ({ + ref, + style, + placement, + arrowProps, + scheduleUpdate + }) => { + const { className, isVisible, zIndex, children, modifiers } = this.props; const mergedClassNames = getMergedClassNames( cx({ dropdown: true, @@ -125,41 +157,59 @@ class Dropdown extends React.PureComponent { className ); - const modifiers = this.getModifiers(this.props.modifiers); + const computedModifiers = this.getModifiers(modifiers); + + // updating `popperScheduleUpdate` reference used in resize observer + this.popperScheduleUpdate = scheduleUpdate; + + return ( +
+ {children} + {computedModifiers.arrow.enabled && ( +
+ )} +
+ ); + }; + + render() { + const { + placement, + triggerRenderer, + eventsEnabled, + positionFixed, + referenceElement, + isVisible + } = this.props; + + const computedModifiers = this.getModifiers(this.props.modifiers); return ( {triggerRenderer && ( {triggerRenderer} )} - {this.props.isVisible && ( + {isVisible && ( - {({ ref, style, placement, arrowProps }) => ( -
- {children} - {modifiers.arrow.enabled && ( -
- )} -
- )} + {this.renderDropdownContent} )} @@ -168,10 +218,14 @@ class Dropdown extends React.PureComponent { } Dropdown.propTypes = { - children: PropTypes.node.isRequired, + children: PropTypes.node, className: PropTypes.string, closeOnEscPress: PropTypes.bool, closeOnEnterPress: PropTypes.bool, + /** + * you can specify which key press should trigger Dropdown close + */ + closeKeyCodes: PropTypes.arrayOf(PropTypes.number), eventsEnabled: PropTypes.bool, isVisible: PropTypes.bool.isRequired, modifiers: PropTypes.object, @@ -197,9 +251,23 @@ Dropdown.propTypes = { clientWidth: PropTypes.number.isRequired, clientHeight: PropTypes.number.isRequired }), + /** + * Pass `true` when it's possible that content of your dropdown will resize + * (e.g removing list items on select) + */ + shouldUpdateOnResize: PropTypes.bool, triggerRenderer: PropTypes.func, zIndex: PropTypes.number, onClose: PropTypes.func }; +Dropdown.defaultProps = { + modifiers: {}, + zIndex: 20, + closeOnEscPress: true, + closeOnEnterPress: false, + placement: 'bottom-start', + shouldUpdateOnResize: false +}; + export default Dropdown; diff --git a/src/components/Dropdown/DropdownList.js b/src/components/Dropdown/DropdownList.js index e7a7681aa..f597cd0ac 100644 --- a/src/components/Dropdown/DropdownList.js +++ b/src/components/Dropdown/DropdownList.js @@ -9,9 +9,27 @@ import findNextFocusableItem from '../../helpers/find-next-focusable-item'; const baseClass = 'dropdown'; class DropdownList extends React.PureComponent { - state = { - focusedElement: null - }; + constructor(props) { + super(props); + + this.state = { + focusedElement: this.getFirstFocusableItemId(), + itemsCount: props.items.length + }; + } + + static getDerivedStateFromProps(props, state) { + if ( + props.autoFocusOnItemsCountChange && + props.items.length !== state.itemsCount + ) { + return { + focusedElement: this.getFirstFocusableItemId(), + itemsCount: props.items.length + }; + } + return null; + } componentDidMount() { document.addEventListener('keydown', this.onKeydown); @@ -31,8 +49,9 @@ class DropdownList extends React.PureComponent { this.handleArrowKeyUse(event); } - if (keyCode === KeyCodes.enter) { - this.handleEnterKeyUse(); + if (this.isItemSelectKeyCode(keyCode)) { + event.preventDefault(); + this.handleSelectKeyUse(); } }; @@ -48,7 +67,16 @@ class DropdownList extends React.PureComponent { return this.hoverCallbacks[itemKey]; }; - handleEnterKeyUse = () => { + getFirstFocusableItemId = () => { + const focusableItem = this.props.items.find(item => !item.isDisabled); + + if (!focusableItem) { + return null; + } + return focusableItem.itemId; + }; + + handleSelectKeyUse = () => { const { focusedElement } = this.state; if (focusedElement !== null) { @@ -104,6 +132,16 @@ class DropdownList extends React.PureComponent { ); }; + isItemSelectKeyCode = keyCode => { + const { itemSelectKeyCodes } = this.props; + + if (itemSelectKeyCodes && itemSelectKeyCodes.includes(keyCode)) { + return true; + } + + return false; + }; + scrollItems = () => { if (!this.listRef.current) { return; @@ -142,7 +180,14 @@ class DropdownList extends React.PureComponent { listRef = React.createRef(); render() { - const { className, items, getItemBody, ...restProps } = this.props; + const { + className, + items, + getItemBody, + itemSelectKeyCodes, + autoFocusOnItemsCountChange, + ...restProps + } = this.props; const mergedClassNames = getMergedClassNames( styles[`${baseClass}__list`], @@ -188,6 +233,7 @@ class DropdownList extends React.PureComponent { } DropdownList.propTypes = { + autoFocusOnItemsCountChange: PropTypes.bool, className: PropTypes.string, items: PropTypes.arrayOf( PropTypes.shape({ @@ -205,7 +251,15 @@ DropdownList.propTypes = { }) ).isRequired, getItemBody: PropTypes.func, - onScroll: PropTypes.func + onScroll: PropTypes.func, + /** + * you can specify which key press should trigger list item select + */ + itemSelectKeyCodes: PropTypes.arrayOf(PropTypes.number) +}; + +DropdownList.defaultProps = { + itemSelectKeyCodes: [KeyCodes.enter] }; export default DropdownList; diff --git a/src/components/Dropdown/DropdownList.test.js b/src/components/Dropdown/DropdownList.test.js index 24961e34c..bf0bfb078 100644 --- a/src/components/Dropdown/DropdownList.test.js +++ b/src/components/Dropdown/DropdownList.test.js @@ -14,14 +14,14 @@ const generateItems = (length = 10) => onItemSelect: jest.fn() })); -describe('Archives | Components | FiltersMenu', () => { +describe('Components | DropdownList', () => { let items; beforeEach(() => { items = generateItems(4); }); - it('renders correctly', () => { + it('renders correctly four items with autofocused second item', () => { const renderer = new ShallowRenderer(); const component = renderer.render(); diff --git a/src/components/Dropdown/DropdownListItem.js b/src/components/Dropdown/DropdownListItem.js index 45e70788c..e4f775fd3 100644 --- a/src/components/Dropdown/DropdownListItem.js +++ b/src/components/Dropdown/DropdownListItem.js @@ -12,6 +12,7 @@ const baseClass = 'dropdown__list-item'; class DropdownListItem extends React.PureComponent { handleClick = e => { if (!this.props.isDisabled && this.props.onItemSelect) { + e.nativeEvent.stopImmediatePropagation(); this.props.onItemSelect(this.props.itemId); } if (this.props.onClick) { diff --git a/src/components/Dropdown/__snapshots__/DropdownList.test.js.snap b/src/components/Dropdown/__snapshots__/DropdownList.test.js.snap index e92642332..4bbb8e4b1 100644 --- a/src/components/Dropdown/__snapshots__/DropdownList.test.js.snap +++ b/src/components/Dropdown/__snapshots__/DropdownList.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Archives | Components | FiltersMenu renders correctly 1`] = ` +exports[`Components | DropdownList renders correctly four items with autofocused second item 1`] = `
    } isDisabled={false} - isFocused={false} + isFocused={true} isSelected={false} itemId={2} onItemSelect={[MockFunction]} diff --git a/src/components/Switch/Switch.js b/src/components/Switch/Switch.js index 752da3a68..d21d584c3 100644 --- a/src/components/Switch/Switch.js +++ b/src/components/Switch/Switch.js @@ -9,24 +9,7 @@ const baseClass = 'switch'; const cx = classNames.bind(styles); const noop = () => {}; -class Switch extends React.PureComponent { - static propTypes = { - className: PropTypes.string, - defaultOn: PropTypes.bool, - innerRef: PropTypes.instanceOf(Element), - name: PropTypes.string, - on: PropTypes.bool, - onChange: PropTypes.func, - size: PropTypes.oneOf(acceptedSizes) - }; - - static defaultProps = { - defaultOn: false, - onChange: noop, - size: 'basic', - name: baseClass - }; - +class SwitchComponent extends React.PureComponent { state = { enabled: this.isControlledByProps() ? this.props.on : this.props.defaultOn, prevPropsOn: this.props.on // eslint-disable-line react/no-unused-state @@ -111,6 +94,38 @@ class Switch extends React.PureComponent { } } -export default React.forwardRef((props, ref) => ( - +const basePropTypes = { + className: PropTypes.string, + defaultOn: PropTypes.bool, + name: PropTypes.string, + on: PropTypes.bool, + onChange: PropTypes.func, + size: PropTypes.oneOf(acceptedSizes) +}; + +/* eslint-disable react/default-props-match-prop-types */ +const baseDefaultProps = { + defaultOn: false, + onChange: noop, + size: 'basic', + name: baseClass +}; + +SwitchComponent.propTypes = { + ...basePropTypes, + innerRef: PropTypes.instanceOf( + typeof Element === 'undefined' ? () => {} : Element + ) +}; + +SwitchComponent.defaultProps = baseDefaultProps; + +const Switch = React.forwardRef((props, ref) => ( + )); + +Switch.propTypes = basePropTypes; + +Switch.defaultProps = baseDefaultProps; + +export default Switch; diff --git a/src/components/Switch/__snapshots__/Switch.test.js.snap b/src/components/Switch/__snapshots__/Switch.test.js.snap index 26530dfa9..b49a77f3c 100644 --- a/src/components/Switch/__snapshots__/Switch.test.js.snap +++ b/src/components/Switch/__snapshots__/Switch.test.js.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`Switch should render basic Switch 1`] = ` - }) => void; onClose: () => void; @@ -49,8 +51,10 @@ export interface IDropdownItem extends IDropdownItemBase { export interface IDropdownListProps extends React.HTMLAttributes { + autoFocusOnItemsCountChange?: boolean; className?: string; items: IDropdownItem[]; + itemSelectKeyCodes?: number[]; getItemBody?(payload: IGetItemBodyPayload): React.ReactNode; }