From 4b77a2ad325600e11ab19700d01b412e59ce88ac Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Wed, 6 Nov 2024 13:59:44 -0600 Subject: [PATCH 1/2] fix(dropdown): stabilize internal references to reduce re-renders (#17952) * fix(dropdown): stabilize internal references to reduce re-renders * fix(dropdown): further optimize renders by stabilizing references and preventing state updates * fix(dropdown): ensure aria-expanded is set * chore: remove test story --- .../src/components/Dropdown/Dropdown.tsx | 215 ++++++++++++------ 1 file changed, 141 insertions(+), 74 deletions(-) diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index f6c41c1d8c88..ac6d2f691f4e 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/packages/react/src/components/Dropdown/Dropdown.tsx @@ -6,6 +6,7 @@ */ import React, { + useCallback, useContext, useState, FocusEvent, @@ -20,6 +21,7 @@ import { UseSelectInterface, UseSelectProps, UseSelectState, + UseSelectStateChange, UseSelectStateChangeTypes, } from 'downshift'; import cx from 'classnames'; @@ -240,6 +242,41 @@ export interface DropdownProps export type DropdownTranslationKey = ListBoxMenuIconTranslationKey; +/** + * Custom state reducer for `useSelect` in Downshift, providing control over + * state changes. + * + * This function is called each time `useSelect` updates its internal state or + * triggers `onStateChange`. It allows for fine-grained control of state + * updates by modifying or overriding the default changes from Downshift's + * reducer. + * https://github.com/downshift-js/downshift/tree/master/src/hooks/useSelect#statereducer + * + * @param {Object} state - The current full state of the Downshift component. + * @param {Object} actionAndChanges - Contains the action type and proposed + * changes from the default Downshift reducer. + * @param {Object} actionAndChanges.changes - Suggested state changes. + * @param {string} actionAndChanges.type - The action type for the state + * change (e.g., item selection). + * @returns {Object} - The modified state based on custom logic or default + * changes if no custom logic applies. + */ +function stateReducer(state, actionAndChanges) { + const { changes, type } = actionAndChanges; + + switch (type) { + case ItemMouseMove: + case MenuMouseLeave: + if (changes.highlightedIndex === state.highlightedIndex) { + // Prevent state update if highlightedIndex hasn't changed + return state; + } + return changes; + default: + return changes; + } +} + const Dropdown = React.forwardRef( ( { @@ -247,7 +284,7 @@ const Dropdown = React.forwardRef( className: containerClassName, disabled = false, direction = 'bottom', - items, + items: itemsProp, label, ['aria-label']: ariaLabel, ariaLabel: deprecatedAriaLabel, @@ -329,22 +366,33 @@ const Dropdown = React.forwardRef( const prefix = usePrefix(); const { isFluid } = useContext(FormContext); - const selectProps: UseSelectProps = { - items, - itemToString, - initialSelectedItem, - onSelectedItemChange, - stateReducer, - isItemDisabled(item, _index) { - const isObject = item !== null && typeof item === 'object'; - return isObject && 'disabled' in item && item.disabled === true; + const onSelectedItemChange = useCallback( + ({ selectedItem }: Partial>) => { + if (onChange) { + onChange({ selectedItem: selectedItem ?? null }); + } }, - onHighlightedIndexChange: ({ highlightedIndex }) => { - if (highlightedIndex! > -1 && typeof window !== undefined) { + [onChange] + ); + + const isItemDisabled = useCallback((item, _index) => { + const isObject = item !== null && typeof item === 'object'; + return isObject && 'disabled' in item && item.disabled === true; + }, []); + + const onHighlightedIndexChange = useCallback( + (changes: UseSelectStateChange) => { + const { highlightedIndex } = changes; + + if ( + highlightedIndex !== undefined && + highlightedIndex > -1 && + typeof window !== undefined + ) { const itemArray = document.querySelectorAll( `li.${prefix}--list-box__menu-item[role="option"]` ); - const highlightedItem = itemArray[highlightedIndex!]; + const highlightedItem = itemArray[highlightedIndex]; if (highlightedItem) { highlightedItem.scrollIntoView({ behavior: 'smooth', @@ -353,20 +401,33 @@ const Dropdown = React.forwardRef( } } }, - ...downshiftProps, - }; - const dropdownInstanceId = useId(); - - function stateReducer(state, actionAndChanges) { - const { changes, type } = actionAndChanges; + [prefix] + ); - switch (type) { - case ItemMouseMove: - case MenuMouseLeave: - return { ...changes, highlightedIndex: state.highlightedIndex }; - } - return changes; - } + const items = useMemo(() => itemsProp, [itemsProp]); + const selectProps = useMemo( + () => ({ + items, + itemToString, + initialSelectedItem, + onSelectedItemChange, + stateReducer, + isItemDisabled, + onHighlightedIndexChange, + ...downshiftProps, + }), + [ + items, + itemToString, + initialSelectedItem, + onSelectedItemChange, + stateReducer, + isItemDisabled, + onHighlightedIndexChange, + downshiftProps, + ] + ); + const dropdownInstanceId = useId(); // only set selectedItem if the prop is defined. Setting if it is undefined // will overwrite default selected items from useSelect @@ -432,9 +493,13 @@ const Dropdown = React.forwardRef( // needs to be Capitalized for react to render it correctly const ItemToElement = itemToElement; - const toggleButtonProps = getToggleButtonProps({ - 'aria-label': ariaLabel || deprecatedAriaLabel, - }); + const toggleButtonProps = useMemo( + () => + getToggleButtonProps({ + 'aria-label': ariaLabel || deprecatedAriaLabel, + }), + [getToggleButtonProps, ariaLabel, deprecatedAriaLabel, isOpen] + ); const helper = helperText && !isFluid ? ( @@ -443,14 +508,6 @@ const Dropdown = React.forwardRef( ) : null; - function onSelectedItemChange({ - selectedItem, - }: Partial>) { - if (onChange) { - onChange({ selectedItem: selectedItem ?? null }); - } - } - const handleFocus = (evt: FocusEvent) => { setIsFocused(evt.type === 'focus' ? true : false); }; @@ -459,11 +516,39 @@ const Dropdown = React.forwardRef( const [currTimer, setCurrTimer] = useState(); - // eslint-disable-next-line prefer-const - let [isTyping, setIsTyping] = useState(false); + const [isTyping, setIsTyping] = useState(false); + + const onKeyDownHandler = useCallback( + (evt: React.KeyboardEvent) => { + if ( + evt.code !== 'Space' || + !['ArrowDown', 'ArrowUp', ' ', 'Enter'].includes(evt.key) + ) { + setIsTyping(true); + } + if ( + (isTyping && evt.code === 'Space') || + !['ArrowDown', 'ArrowUp', ' ', 'Enter'].includes(evt.key) + ) { + if (currTimer) { + clearTimeout(currTimer); + } + setCurrTimer( + setTimeout(() => { + setIsTyping(false); + }, 3000) + ); + } + if (toggleButtonProps.onKeyDown) { + toggleButtonProps.onKeyDown(evt); + } + }, + [isTyping, currTimer, toggleButtonProps] + ); - const readOnlyEventHandlers = readOnly - ? { + const readOnlyEventHandlers = useMemo(() => { + if (readOnly) { + return { onClick: (evt: MouseEvent) => { // NOTE: does not prevent click evt.preventDefault(); @@ -477,49 +562,31 @@ const Dropdown = React.forwardRef( evt.preventDefault(); } }, - } - : { - onKeyDown: (evt: React.KeyboardEvent) => { - if ( - evt.code !== 'Space' || - !['ArrowDown', 'ArrowUp', ' ', 'Enter'].includes(evt.key) - ) { - setIsTyping(true); - } - if ( - (isTyping && evt.code === 'Space') || - !['ArrowDown', 'ArrowUp', ' ', 'Enter'].includes(evt.key) - ) { - if (currTimer) { - clearTimeout(currTimer); - } - setCurrTimer( - setTimeout(() => { - setIsTyping(false); - }, 3000) - ); - } - if (toggleButtonProps.onKeyDown) { - toggleButtonProps.onKeyDown(evt); - } - }, }; + } else { + return { + onKeyDown: onKeyDownHandler, + }; + } + }, [readOnly, onKeyDownHandler]); const menuProps = useMemo( () => getMenuProps({ ref: enableFloatingStyles || autoAlign ? refs.setFloating : null, }), - [autoAlign, getMenuProps, refs.setFloating] + [autoAlign, getMenuProps, refs.setFloating, enableFloatingStyles] ); // Slug is always size `mini` - let normalizedSlug; - if (slug && slug['type']?.displayName === 'AILabel') { - normalizedSlug = React.cloneElement(slug as React.ReactElement, { - size: 'mini', - }); - } + const normalizedSlug = useMemo(() => { + if (slug && slug['type']?.displayName === 'AILabel') { + return React.cloneElement(slug as React.ReactElement, { + size: 'mini', + }); + } + return slug; + }, [slug]); return (
From cdc8db7fcd0085f344e8f3942c5f8f48eb24ab5a Mon Sep 17 00:00:00 2001 From: Gururaj J <89023023+Gururajj77@users.noreply.github.com> Date: Thu, 7 Nov 2024 01:40:31 +0530 Subject: [PATCH 2/2] refactor: Layout direction js to tsx (#17853) * refactor: layout direction js to tsx * refactor: added the required stuff --------- Co-authored-by: Nikhil Tomar <63502271+2nikhiltom@users.noreply.github.com> Co-authored-by: Taylor Jones --- ...LayoutDirection.js => LayoutDirection.tsx} | 26 +++++++++++++++++-- ...onContext.js => LayoutDirectionContext.ts} | 0 ...youtDirection.js => useLayoutDirection.ts} | 0 3 files changed, 24 insertions(+), 2 deletions(-) rename packages/react/src/components/LayoutDirection/{LayoutDirection.js => LayoutDirection.tsx} (68%) rename packages/react/src/components/LayoutDirection/{LayoutDirectionContext.js => LayoutDirectionContext.ts} (100%) rename packages/react/src/components/LayoutDirection/{useLayoutDirection.js => useLayoutDirection.ts} (100%) diff --git a/packages/react/src/components/LayoutDirection/LayoutDirection.js b/packages/react/src/components/LayoutDirection/LayoutDirection.tsx similarity index 68% rename from packages/react/src/components/LayoutDirection/LayoutDirection.js rename to packages/react/src/components/LayoutDirection/LayoutDirection.tsx index 6c999718788d..f2ac0551a9b9 100644 --- a/packages/react/src/components/LayoutDirection/LayoutDirection.js +++ b/packages/react/src/components/LayoutDirection/LayoutDirection.tsx @@ -6,15 +6,37 @@ */ import PropTypes from 'prop-types'; -import React from 'react'; +import React, { ElementType, ReactNode } from 'react'; import { LayoutDirectionContext } from './LayoutDirectionContext'; +type Direction = 'ltr' | 'rtl'; + +interface LayoutDirectionContextValue { + direction: Direction; +} + +interface LayoutDirectionProps { + /** + * Customize the element type used to render the outermost node + */ + as?: ElementType; + /** + * Provide child elements or components to be rendered inside of this + * component + */ + children?: ReactNode; + /** + * Specify the layout direction of this part of the page + */ + dir: Direction; +} + function LayoutDirection({ as: BaseComponent = 'div', children, dir, ...rest -}) { +}: LayoutDirectionProps) { const value = React.useMemo(() => { return { direction: dir, diff --git a/packages/react/src/components/LayoutDirection/LayoutDirectionContext.js b/packages/react/src/components/LayoutDirection/LayoutDirectionContext.ts similarity index 100% rename from packages/react/src/components/LayoutDirection/LayoutDirectionContext.js rename to packages/react/src/components/LayoutDirection/LayoutDirectionContext.ts diff --git a/packages/react/src/components/LayoutDirection/useLayoutDirection.js b/packages/react/src/components/LayoutDirection/useLayoutDirection.ts similarity index 100% rename from packages/react/src/components/LayoutDirection/useLayoutDirection.js rename to packages/react/src/components/LayoutDirection/useLayoutDirection.ts