From 91baf67ba6a9a47db472c320a43cddbe8b816436 Mon Sep 17 00:00:00 2001 From: Constance Date: Tue, 21 Sep 2021 14:08:37 -0700 Subject: [PATCH] Update multiple functional components to useGeneratedHtmlId hook (#5195) * [Fix] EuiButtonGroup generated ID * [Fix] EuiCard aria IDs * [Fix] EuiTour title ID * [Fix] EuiKeyPadMenuItem generated ID * [Fix] Multiple EuiNotificationEvent generated IDs * [Fix] EuiHeaderAlert generated ID * [Fix] EuiMarkdownEditor checkboxes generated IDs * [Refactor] EuiMarkdownEditor ID - remove existing useMemo hook in favor of new helper - behavior should remain same as before * [Refactor] EuiResizeableContainer IDs - replace useRefs with new hook - behavior should remain same as before * [Refactor] EuiSwitch IDs - replace useState with new hook - behavior should remain same as before * Add changelog entry --- CHANGELOG.md | 1 + .../button/button_group/button_group.tsx | 4 +- .../button_group/button_group_button.tsx | 6 +-- src/components/card/card.tsx | 4 +- src/components/form/switch/switch.tsx | 7 ++-- .../header/header_alert/header_alert.tsx | 4 +- .../key_pad_menu/key_pad_menu_item.tsx | 4 +- .../markdown_editor/markdown_editor.tsx | 6 +-- .../plugins/markdown_checkbox/renderer.tsx | 4 +- .../notification/notification_event.tsx | 4 +- .../notification_event_messages.tsx | 8 +++- .../notification/notification_event_meta.tsx | 4 +- .../resizable_container/resizable_button.tsx | 26 ++++++------ .../resizable_container/resizable_panel.tsx | 42 +++++++++---------- src/components/tour/tour_step.tsx | 5 +-- 15 files changed, 62 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0dcaab72ca..20c1eba4bae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Fixed `EuiDataGrid` focus ring to be contained in the cell ([#5194](https://github.com/elastic/eui/pull/5194)) - Fixed `EuiDataGrid` cells when focused getting a higher `z-index` which was causing long content to overlap surrounding cells ([#5194](https://github.com/elastic/eui/pull/5194)) +- Fixed multiple components unnecessarily rerendering generated IDs on every update ([#5195](https://github.com/elastic/eui/pull/5195), [#5196](https://github.com/elastic/eui/pull/5196), [#5197](https://github.com/elastic/eui/pull/5197), [#5200](https://github.com/elastic/eui/pull/#5200), [#5201](https://github.com/elastic/eui/pull/#5201)) **Theme: Amsterdam** diff --git a/src/components/button/button_group/button_group.tsx b/src/components/button/button_group/button_group.tsx index 25becfd6f83..132c2b378e0 100644 --- a/src/components/button/button_group/button_group.tsx +++ b/src/components/button/button_group/button_group.tsx @@ -13,7 +13,7 @@ import { EuiButtonGroupButton } from './button_group_button'; import { colorToClassNameMap, ButtonColor } from '../button'; import { EuiButtonContentProps } from '../button_content'; import { CommonProps } from '../../common'; -import { htmlIdGenerator } from '../../../services'; +import { useGeneratedHtmlId } from '../../../services'; export interface EuiButtonGroupOptionProps extends EuiButtonContentProps, @@ -156,7 +156,7 @@ export const EuiButtonGroup: FunctionComponent = ({ ); const typeIsSingle = type === 'single'; - const nameIfSingle = name || htmlIdGenerator()(); + const nameIfSingle = useGeneratedHtmlId({ conditionalId: name }); return (
diff --git a/src/components/button/button_group/button_group_button.tsx b/src/components/button/button_group/button_group_button.tsx index 26891b11635..19d4376a6b3 100644 --- a/src/components/button/button_group/button_group_button.tsx +++ b/src/components/button/button_group/button_group_button.tsx @@ -7,11 +7,11 @@ */ import classNames from 'classnames'; -import React, { FunctionComponent, useRef } from 'react'; +import React, { FunctionComponent } from 'react'; import { EuiButtonDisplay } from '../button'; import { EuiButtonGroupOptionProps, EuiButtonGroupProps } from './button_group'; import { useInnerText } from '../../inner_text'; -import { htmlIdGenerator } from '../../../services'; +import { useGeneratedHtmlId } from '../../../services'; type Props = EuiButtonGroupOptionProps & { /** @@ -65,7 +65,7 @@ export const EuiButtonGroupButton: FunctionComponent = ({ }) => { // Force element to be a button if disabled const el = isDisabled ? 'button' : element; - const newId = useRef(htmlIdGenerator()()).current; + const newId = useGeneratedHtmlId(); let elementProps = {}; let singleInput; diff --git a/src/components/card/card.tsx b/src/components/card/card.tsx index 1b1273fa386..d09f94406bb 100644 --- a/src/components/card/card.tsx +++ b/src/components/card/card.tsx @@ -26,7 +26,7 @@ import { EuiCardSelectProps, euiCardSelectableColor, } from './card_select'; -import { htmlIdGenerator } from '../../services/accessibility'; +import { useGeneratedHtmlId } from '../../services/accessibility'; import { validateHref } from '../../services/security/href_validator'; import { EuiPanel, EuiPanelProps } from '../panel'; @@ -244,7 +244,7 @@ export const EuiCard: FunctionComponent = ({ className ); - const ariaId = htmlIdGenerator()(); + const ariaId = useGeneratedHtmlId(); const ariaDesc = description ? `${ariaId}Description` : ''; /** diff --git a/src/components/form/switch/switch.tsx b/src/components/form/switch/switch.tsx index 2b7a48457a3..71fa2a0f056 100644 --- a/src/components/form/switch/switch.tsx +++ b/src/components/form/switch/switch.tsx @@ -11,13 +11,12 @@ import React, { HTMLAttributes, FunctionComponent, ReactNode, - useState, useCallback, } from 'react'; import classNames from 'classnames'; import { CommonProps } from '../../common'; -import { htmlIdGenerator } from '../../../services/accessibility'; +import { useGeneratedHtmlId } from '../../../services/accessibility'; import { EuiIcon } from '../../icon'; export type EuiSwitchEvent = React.BaseSyntheticEvent< @@ -65,8 +64,8 @@ export const EuiSwitch: FunctionComponent = ({ labelProps, ...rest }) => { - const [switchId] = useState(id || htmlIdGenerator()()); - const [labelId] = useState(labelProps?.id || htmlIdGenerator()()); + const switchId = useGeneratedHtmlId({ conditionalId: id }); + const labelId = useGeneratedHtmlId({ conditionalId: labelProps?.id }); const onClick = useCallback( (e: React.MouseEvent) => { diff --git a/src/components/header/header_alert/header_alert.tsx b/src/components/header/header_alert/header_alert.tsx index 6826dc91b95..8bf0accd561 100644 --- a/src/components/header/header_alert/header_alert.tsx +++ b/src/components/header/header_alert/header_alert.tsx @@ -12,7 +12,7 @@ import classNames from 'classnames'; import { CommonProps } from '../../common'; import { EuiFlexGroup, EuiFlexItem } from '../../flex'; -import { htmlIdGenerator } from '../../../services'; +import { useGeneratedHtmlId } from '../../../services'; export type EuiHeaderAlertProps = CommonProps & Omit, 'title'> & { @@ -40,7 +40,7 @@ export const EuiHeaderAlert: FunctionComponent = ({ }) => { const classes = classNames('euiHeaderAlert', className); - const ariaId = htmlIdGenerator()(); + const ariaId = useGeneratedHtmlId(); return (
diff --git a/src/components/key_pad_menu/key_pad_menu_item.tsx b/src/components/key_pad_menu/key_pad_menu_item.tsx index ee677683352..62c5bd6b128 100644 --- a/src/components/key_pad_menu/key_pad_menu_item.tsx +++ b/src/components/key_pad_menu/key_pad_menu_item.tsx @@ -24,7 +24,7 @@ import { import { EuiBetaBadge } from '../badge/beta_badge'; -import { getSecureRelForTarget, htmlIdGenerator } from '../../services'; +import { getSecureRelForTarget, useGeneratedHtmlId } from '../../services'; import { IconType } from '../icon'; import { EuiRadio, EuiCheckbox } from '../form'; @@ -184,7 +184,7 @@ export const EuiKeyPadMenuItem: FunctionComponent = ({ if (checkable) Element = 'label'; type ElementType = ReactElementType; - const itemId = id || htmlIdGenerator()(); + const itemId = useGeneratedHtmlId({ conditionalId: id }); const renderCheckableElement = () => { if (!checkable) return; diff --git a/src/components/markdown_editor/markdown_editor.tsx b/src/components/markdown_editor/markdown_editor.tsx index 0ff29d3993c..10dfa296f1c 100644 --- a/src/components/markdown_editor/markdown_editor.tsx +++ b/src/components/markdown_editor/markdown_editor.tsx @@ -31,7 +31,7 @@ import { } from './markdown_editor_text_area'; import { EuiMarkdownFormat, EuiMarkdownFormatProps } from './markdown_format'; import { EuiMarkdownEditorDropZone } from './markdown_editor_drop_zone'; -import { htmlIdGenerator } from '../../services/'; +import { useGeneratedHtmlId } from '../../services/'; import { MARKDOWN_MODE, MODE_EDITING, MODE_VIEWING } from './markdown_modes'; import { @@ -214,9 +214,7 @@ export const EuiMarkdownEditor = forwardRef< ref ) => { const [viewMode, setViewMode] = useState(initialViewMode); - const editorId = useMemo(() => _editorId || htmlIdGenerator()(), [ - _editorId, - ]); + const editorId = useGeneratedHtmlId({ conditionalId: _editorId }); const [pluginEditorPlugin, setPluginEditorPlugin] = useState< EuiMarkdownEditorUiPlugin | undefined diff --git a/src/components/markdown_editor/plugins/markdown_checkbox/renderer.tsx b/src/components/markdown_editor/plugins/markdown_checkbox/renderer.tsx index 2d3f8dc9887..c594543aec9 100644 --- a/src/components/markdown_editor/plugins/markdown_checkbox/renderer.tsx +++ b/src/components/markdown_editor/plugins/markdown_checkbox/renderer.tsx @@ -9,7 +9,7 @@ import React, { FunctionComponent, useContext } from 'react'; import { EuiCheckbox } from '../../../form/checkbox'; import { EuiMarkdownContext } from '../../markdown_context'; -import { htmlIdGenerator } from '../../../../services/accessibility'; +import { useGeneratedHtmlId } from '../../../../services/accessibility'; import { EuiMarkdownAstNodePosition } from '../../markdown_types'; import { CheckboxNodeDetails } from './types'; @@ -21,7 +21,7 @@ export const CheckboxMarkdownRenderer: FunctionComponent< const { replaceNode } = useContext(EuiMarkdownContext); return ( { diff --git a/src/components/notification/notification_event.tsx b/src/components/notification/notification_event.tsx index 261b29d0fab..6ae70c83129 100644 --- a/src/components/notification/notification_event.tsx +++ b/src/components/notification/notification_event.tsx @@ -23,7 +23,7 @@ import { import { EuiButtonEmpty, EuiButtonEmptyProps } from '../button'; import { EuiLink } from '../link'; import { EuiContextMenuItem, EuiContextMenuItemProps } from '../context_menu'; -import { htmlIdGenerator } from '../../services'; +import { useGeneratedHtmlId } from '../../services'; import { EuiNotificationEventReadIcon } from './notification_event_read_icon'; export type EuiNotificationHeadingLevel = 'h2' | 'h3' | 'h4' | 'h5' | 'h6'; @@ -113,7 +113,7 @@ export const EuiNotificationEvent: FunctionComponent 'euiNotificationEvent__title--isRead': isRead, }); - const randomHeadingId = htmlIdGenerator()(); + const randomHeadingId = useGeneratedHtmlId(); const titleProps = { id: randomHeadingId, diff --git a/src/components/notification/notification_event_messages.tsx b/src/components/notification/notification_event_messages.tsx index 5096d37713a..e5fc784babb 100644 --- a/src/components/notification/notification_event_messages.tsx +++ b/src/components/notification/notification_event_messages.tsx @@ -8,7 +8,7 @@ import React, { FunctionComponent, useState } from 'react'; import { EuiAccordion } from '../accordion'; -import { htmlIdGenerator } from '../../services'; +import { useGeneratedHtmlId } from '../../services'; import { useEuiI18n } from '../i18n'; import { EuiText } from '../text'; @@ -30,6 +30,10 @@ export const EuiNotificationEventMessages: FunctionComponent> >([]); - const randomPopoverId = htmlIdGenerator()(); + const randomPopoverId = useGeneratedHtmlId(); const ariaAttribute = iconAriaLabel ? { 'aria-label': iconAriaLabel } diff --git a/src/components/resizable_container/resizable_button.tsx b/src/components/resizable_container/resizable_button.tsx index e391aa15716..80a9676d3fc 100644 --- a/src/components/resizable_container/resizable_button.tsx +++ b/src/components/resizable_container/resizable_button.tsx @@ -18,7 +18,7 @@ import classNames from 'classnames'; import { CommonProps } from '../common'; import { EuiI18n } from '../i18n'; -import { htmlIdGenerator } from '../../services'; +import { useGeneratedHtmlId } from '../../services'; import { useEuiResizableContainerContext } from './context'; import { EuiResizableButtonController, @@ -47,8 +47,6 @@ export interface EuiResizableButtonProps CommonProps, Partial {} -const generatePanelId = htmlIdGenerator('resizable-button'); - export const EuiResizableButton: FunctionComponent = ({ isHorizontal, className, @@ -59,15 +57,16 @@ export const EuiResizableButton: FunctionComponent = ({ onBlur, ...rest }) => { - const resizerId = useRef(id || generatePanelId()); + const resizerId = useGeneratedHtmlId({ + prefix: 'resizable-button', + conditionalId: id, + }); const { registry: { resizers } = { resizers: {} }, } = useEuiResizableContainerContext(); const isDisabled = useMemo( - () => - disabled || - (resizers[resizerId.current] && resizers[resizerId.current].isDisabled), - [resizers, disabled] + () => disabled || (resizers[resizerId] && resizers[resizerId].isDisabled), + [resizers, resizerId, disabled] ); const classes = classNames( 'euiResizableButton', @@ -83,30 +82,29 @@ export const EuiResizableButton: FunctionComponent = ({ const onRef = useCallback( (ref: HTMLElement | null) => { if (!registration) return; - const id = resizerId.current; if (ref) { previousRef.current = ref; registration.register({ - id, + id: resizerId, ref, isFocused: false, isDisabled: disabled || false, }); } else { if (previousRef.current != null) { - registration.deregister(id); + registration.deregister(resizerId); previousRef.current = undefined; } } }, - [registration, disabled] + [registration, resizerId, disabled] ); const setFocus = (e: MouseEvent) => e.currentTarget.focus(); const handleFocus = () => { - onFocus && onFocus(resizerId.current); + onFocus && onFocus(resizerId); }; return ( @@ -122,7 +120,7 @@ export const EuiResizableButton: FunctionComponent = ({ > {([horizontalResizerAriaLabel, verticalResizerAriaLabel]: string[]) => (