From 4cd12047df814e6783b35bdfdc1bb9cca181d875 Mon Sep 17 00:00:00 2001 From: arpanroy41 Date: Wed, 25 Feb 2026 12:26:06 +0530 Subject: [PATCH 1/9] refactor: #12250: replace GenerateId with useSSRSafeId in multiple components - Updated AboutModal, CalendarMonth, CardHeader, Checkbox, DataListCheck, DrawerPanelContent, DualListSelector, DualListSelectorListItem, DualListSelectorListWrapper, DualListSelectorPane, ExpandableSection, FormGroup, InternalFormFieldGroup, JumpLinks, and MenuItem components to use useSSRSafeId for generating unique IDs instead of GenerateId. - This change improves consistency and simplifies ID generation across components. --- .../src/components/AboutModal/AboutModal.tsx | 56 ++--- .../CalendarMonth/CalendarMonth.tsx | 4 +- .../src/components/Card/CardHeader.tsx | 226 +++++++++--------- .../src/components/Checkbox/Checkbox.tsx | 8 +- .../src/components/DataList/DataListCheck.tsx | 29 +-- .../components/Drawer/DrawerPanelContent.tsx | 189 +++++++-------- .../DualListSelector/DualListSelector.tsx | 25 +- .../DualListSelectorListItem.tsx | 6 +- .../DualListSelectorListWrapper.tsx | 6 +- .../DualListSelector/DualListSelectorPane.tsx | 86 +++---- .../ExpandableSection/ExpandableSection.tsx | 12 +- .../src/components/Form/FormGroup.tsx | 61 +++-- .../Form/InternalFormFieldGroup.tsx | 21 +- .../src/components/JumpLinks/JumpLinks.tsx | 6 +- .../JumpLinks/__tests__/JumpLinks.test.tsx | 9 +- .../src/components/Menu/MenuItem.tsx | 133 +++++------ .../react-core/src/components/Modal/Modal.tsx | 7 +- .../MultipleFileUploadStatus.tsx | 29 +-- .../src/components/Nav/NavExpandable.tsx | 8 +- .../src/components/Nav/NavGroup.tsx | 6 +- .../src/components/Popover/Popover.tsx | 5 +- .../src/components/Progress/Progress.tsx | 5 +- .../react-core/src/components/Radio/Radio.tsx | 8 +- .../SearchInput/AdvancedSearchMenu.tsx | 23 +- .../src/components/Switch/Switch.tsx | 8 +- .../src/components/TimePicker/TimePicker.tsx | 14 +- .../src/components/Toolbar/Toolbar.tsx | 5 +- .../Toolbar/ToolbarLabelGroupContent.tsx | 6 +- .../components/Toolbar/ToolbarToggleGroup.tsx | 4 +- .../src/components/Tooltip/Tooltip.tsx | 8 +- .../components/TreeView/TreeViewListItem.tsx | 67 +++--- .../__tests__/TreeViewListItem.test.tsx | 6 +- .../src/deprecated/components/Modal/Modal.tsx | 7 +- .../src/helpers/GenerateId/GenerateId.ts | 21 +- packages/react-core/src/helpers/OUIA/ouia.ts | 9 +- packages/react-core/src/helpers/index.ts | 1 + .../react-core/src/helpers/useSSRSafeId.ts | 19 ++ 37 files changed, 572 insertions(+), 571 deletions(-) create mode 100644 packages/react-core/src/helpers/useSSRSafeId.ts diff --git a/packages/react-core/src/components/AboutModal/AboutModal.tsx b/packages/react-core/src/components/AboutModal/AboutModal.tsx index 62c4c617a4f..56c0aeb93c7 100644 --- a/packages/react-core/src/components/AboutModal/AboutModal.tsx +++ b/packages/react-core/src/components/AboutModal/AboutModal.tsx @@ -6,7 +6,7 @@ import { AboutModalBoxBrand } from './AboutModalBoxBrand'; import { AboutModalBoxCloseButton } from './AboutModalBoxCloseButton'; import { AboutModalBox } from './AboutModalBox'; import { Modal, ModalVariant } from '../Modal'; -import { GenerateId } from '../../helpers/GenerateId/GenerateId'; +import { useSSRSafeId } from '../../helpers'; export interface AboutModalProps extends React.HTMLProps { /** Content rendered inside the about modal */ @@ -56,6 +56,8 @@ export const AboutModal: React.FunctionComponent = ({ disableFocusTrap, ...props }: AboutModalProps) => { + const ariaLabelledBy = useSSRSafeId('pf-about-modal-title-'); + if (brandImageSrc && !brandImageAlt) { // eslint-disable-next-line no-console console.error( @@ -73,35 +75,29 @@ export const AboutModal: React.FunctionComponent = ({ return null; } return ( - - {(ariaLabelledBy) => ( - - - - - {productName && } - - {children} - - - - )} - + + + + + {productName && } + + {children} + + + ); }; AboutModal.displayName = 'AboutModal'; diff --git a/packages/react-core/src/components/CalendarMonth/CalendarMonth.tsx b/packages/react-core/src/components/CalendarMonth/CalendarMonth.tsx index ebfd522ff90..474e815475f 100644 --- a/packages/react-core/src/components/CalendarMonth/CalendarMonth.tsx +++ b/packages/react-core/src/components/CalendarMonth/CalendarMonth.tsx @@ -8,7 +8,7 @@ import AngleLeftIcon from '@patternfly/react-icons/dist/esm/icons/angle-left-ico import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon'; import { css } from '@patternfly/react-styles'; import styles from '@patternfly/react-styles/css/components/CalendarMonth/calendar-month'; -import { getUniqueId } from '../../helpers/util'; +import { useSSRSafeId } from '../../helpers'; import { isValidDate } from '../../helpers/datetimeUtils'; export enum Weekday { @@ -170,7 +170,7 @@ export const CalendarMonth = ({ const [hoveredDate, setHoveredDate] = useState(undefined); const focusRef = useRef(undefined); - const [hiddenMonthId] = useState(getUniqueId('hidden-month-span')); + const hiddenMonthId = useSSRSafeId('hidden-month-span'); const [shouldFocus, setShouldFocus] = useState(false); const isValidated = (date: Date) => validators.every((validator) => validator(date)); diff --git a/packages/react-core/src/components/Card/CardHeader.tsx b/packages/react-core/src/components/Card/CardHeader.tsx index ae01ead41be..0b7292e1c0b 100644 --- a/packages/react-core/src/components/Card/CardHeader.tsx +++ b/packages/react-core/src/components/Card/CardHeader.tsx @@ -8,7 +8,7 @@ import { Button } from '../Button'; import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon'; import { Radio } from '../Radio'; import { Checkbox } from '../Checkbox'; -import { GenerateId } from '../../helpers/GenerateId/GenerateId'; +import { useSSRSafeId } from '../../helpers'; export interface CardHeaderActionsObject { /** Actions of the card header */ @@ -90,127 +90,127 @@ export const CardHeader: React.FunctionComponent = ({ isToggleRightAligned, hasWrap, ...props -}: CardHeaderProps) => ( - - {(randomId) => ( - - {({ cardId, isClickable, isSelectable, isSelected, isDisabled: isCardDisabled }) => { - const cardHeaderToggle = ( -
-
+}: CardHeaderProps) => { + const randomId = useSSRSafeId(); + + return ( + + {({ cardId, isClickable, isSelectable, isSelected, isDisabled: isCardDisabled }) => { + const cardHeaderToggle = ( +
+
+ ); + + const isClickableOrSelectableOnly = (isClickable && !isSelectable) || (isSelectable && !isClickable); + if (actions?.actions && isClickableOrSelectableOnly) { + // eslint-disable-next-line no-console + console.error( + `Card: ${ + isClickable ? 'Clickable' : 'Selectable' + } only cards should not contain any other actions. If you wish to include additional actions, use a clickable and selectable card.` ); + } - const isClickableOrSelectableOnly = (isClickable && !isSelectable) || (isSelectable && !isClickable); - if (actions?.actions && isClickableOrSelectableOnly) { - // eslint-disable-next-line no-console - console.error( - `Card: ${ - isClickable ? 'Clickable' : 'Selectable' - } only cards should not contain any other actions. If you wish to include additional actions, use a clickable and selectable card.` - ); - } + const isClickableOnlyCard = isClickable && !isSelectable; + if ( + (isClickableOnlyCard || isSelectable) && + !selectableActions?.selectableActionAriaLabel && + !selectableActions?.selectableActionAriaLabelledby + ) { + // eslint-disable-next-line no-console + console.error( + `Card: ${isClickableOnlyCard ? 'Clickable-only cards' : 'Cards with a selectable input'} must have either the selectableActions.selectableActionAriaLabel or selectableActions.selectableActionAriaLabelledby prop passed in order to provide an accessible name to the clickable element.` + ); + } - const isClickableOnlyCard = isClickable && !isSelectable; - if ( - (isClickableOnlyCard || isSelectable) && - !selectableActions?.selectableActionAriaLabel && - !selectableActions?.selectableActionAriaLabelledby - ) { - // eslint-disable-next-line no-console - console.error( - `Card: ${isClickableOnlyCard ? 'Clickable-only cards' : 'Cards with a selectable input'} must have either the selectableActions.selectableActionAriaLabel or selectableActions.selectableActionAriaLabelledby prop passed in order to provide an accessible name to the clickable element.` - ); - } + const SelectableCardInput = selectableActions?.variant === 'single' ? Radio : Checkbox; + const getSelectableProps = () => ({ + className: css('pf-m-standalone'), + inputClassName: css(selectableActions?.isHidden && 'pf-v6-screen-reader'), + label: <>, + 'aria-label': selectableActions.selectableActionAriaLabel, + 'aria-labelledby': selectableActions.selectableActionAriaLabelledby, + id: selectableActions.selectableActionId ?? `card-selectable-${randomId}`, + name: selectableActions.name, + isDisabled: isCardDisabled, + onChange: selectableActions.onChange, + isChecked: selectableActions.isChecked ?? isSelected, + ...selectableActions.selectableActionProps + }); - const SelectableCardInput = selectableActions?.variant === 'single' ? Radio : Checkbox; - const getSelectableProps = () => ({ - className: css('pf-m-standalone'), - inputClassName: css(selectableActions?.isHidden && 'pf-v6-screen-reader'), - label: <>, + const isClickableLinkCard = selectableActions?.to !== undefined; + const ClickableCardComponent = isClickableLinkCard ? 'a' : 'button'; + const getClickableProps = () => { + const isDisabledLinkCard = isCardDisabled && isClickableLinkCard; + const baseProps = { + className: css( + 'pf-v6-c-card__clickable-action', + isDisabledLinkCard && styles.modifiers.disabled, + selectableActions?.isHidden && 'pf-v6-screen-reader' + ), + id: selectableActions.selectableActionId, 'aria-label': selectableActions.selectableActionAriaLabel, 'aria-labelledby': selectableActions.selectableActionAriaLabelledby, - id: selectableActions.selectableActionId ?? `card-selectable-${randomId}`, - name: selectableActions.name, - isDisabled: isCardDisabled, - onChange: selectableActions.onChange, - isChecked: selectableActions.isChecked ?? isSelected, ...selectableActions.selectableActionProps - }); + }; - const isClickableLinkCard = selectableActions?.to !== undefined; - const ClickableCardComponent = isClickableLinkCard ? 'a' : 'button'; - const getClickableProps = () => { - const isDisabledLinkCard = isCardDisabled && isClickableLinkCard; - const baseProps = { - className: css( - 'pf-v6-c-card__clickable-action', - isDisabledLinkCard && styles.modifiers.disabled, - selectableActions?.isHidden && 'pf-v6-screen-reader' - ), - id: selectableActions.selectableActionId, - 'aria-label': selectableActions.selectableActionAriaLabel, - 'aria-labelledby': selectableActions.selectableActionAriaLabelledby, - ...selectableActions.selectableActionProps + if (isClickableLinkCard) { + return { + ...baseProps, + href: selectableActions.to, + ...(isCardDisabled && { tabIndex: -1, 'aria-disabled': true }), + ...(selectableActions.isExternalLink && { target: '_blank' }) }; + } - if (isClickableLinkCard) { - return { - ...baseProps, - href: selectableActions.to, - ...(isCardDisabled && { tabIndex: -1, 'aria-disabled': true }), - ...(selectableActions.isExternalLink && { target: '_blank' }) - }; - } + return { ...baseProps, type: 'button', disabled: isCardDisabled, onClick: selectableActions.onClickAction }; + }; - return { ...baseProps, type: 'button', disabled: isCardDisabled, onClick: selectableActions.onClickAction }; - }; - - return ( -
- {onExpand && !isToggleRightAligned && cardHeaderToggle} - {(actions || (selectableActions && (isClickable || isSelectable))) && ( - - {actions?.actions} - {selectableActions && (isClickable || isSelectable) && ( - - {isSelectable && } - {isClickableOnlyCard && } - - )} - - )} - {children && {children}} - {onExpand && isToggleRightAligned && cardHeaderToggle} -
- ); - }} - - )} - -); + return ( +
+ {onExpand && !isToggleRightAligned && cardHeaderToggle} + {(actions || (selectableActions && (isClickable || isSelectable))) && ( + + {actions?.actions} + {selectableActions && (isClickable || isSelectable) && ( + + {isSelectable && } + {isClickableOnlyCard && } + + )} + + )} + {children && {children}} + {onExpand && isToggleRightAligned && cardHeaderToggle} +
+ ); + }} + + ); +}; CardHeader.displayName = 'CardHeader'; diff --git a/packages/react-core/src/components/Checkbox/Checkbox.tsx b/packages/react-core/src/components/Checkbox/Checkbox.tsx index 5ad9d25e653..aa8bcb84c4e 100644 --- a/packages/react-core/src/components/Checkbox/Checkbox.tsx +++ b/packages/react-core/src/components/Checkbox/Checkbox.tsx @@ -3,11 +3,13 @@ import styles from '@patternfly/react-styles/css/components/Check/check'; import { css } from '@patternfly/react-styles'; import { PickOptional } from '../../helpers/typeUtils'; import { getDefaultOUIAId, getOUIAProps, OUIAProps } from '../../helpers'; -import { getUniqueId } from '../../helpers/util'; import { ASTERISK } from '../../helpers/htmlConstants'; +let checkboxDescriptionId = 0; + export interface CheckboxProps - extends Omit, 'type' | 'onChange' | 'disabled' | 'label'>, OUIAProps { + extends Omit, 'type' | 'onChange' | 'disabled' | 'label'>, + OUIAProps { /** Additional classes added to the checkbox wrapper. This wrapper will be div element by default. It will be a label element if * isLabelWrapped is true, or it can be overridden by any element specified in the component prop. */ @@ -74,7 +76,7 @@ class Checkbox extends Component { super(props); this.state = { ouiaStateId: getDefaultOUIAId(Checkbox.displayName), - descriptionId: getUniqueId('pf-checkbox-description') + descriptionId: `pf-checkbox-description-${checkboxDescriptionId++}` }; } diff --git a/packages/react-core/src/components/DataList/DataListCheck.tsx b/packages/react-core/src/components/DataList/DataListCheck.tsx index 978521a738c..dd968c8b6da 100644 --- a/packages/react-core/src/components/DataList/DataListCheck.tsx +++ b/packages/react-core/src/components/DataList/DataListCheck.tsx @@ -2,7 +2,7 @@ import { Fragment } from 'react'; import { css } from '@patternfly/react-styles'; import styles from '@patternfly/react-styles/css/components/DataList/data-list'; import { Checkbox, CheckboxProps } from '../Checkbox'; -import { GenerateId } from '../../helpers/GenerateId/GenerateId'; +import { useSSRSafeId } from '../../helpers'; export interface DataListCheckProps extends Omit { /** Id of the DataList checkbox. */ @@ -47,23 +47,20 @@ export const DataListCheck: React.FunctionComponent = ({ otherControls = false, ...props }: DataListCheckProps) => { + const randomId = useSSRSafeId(); const check = (
- - {(randomId) => ( - - )} - +
); return ( diff --git a/packages/react-core/src/components/Drawer/DrawerPanelContent.tsx b/packages/react-core/src/components/Drawer/DrawerPanelContent.tsx index 9cafec596dc..4b19b9fecb3 100644 --- a/packages/react-core/src/components/Drawer/DrawerPanelContent.tsx +++ b/packages/react-core/src/components/Drawer/DrawerPanelContent.tsx @@ -3,7 +3,7 @@ import styles from '@patternfly/react-styles/css/components/Drawer/drawer'; import { css } from '@patternfly/react-styles'; import { DrawerColorVariant, DrawerContext } from './Drawer'; import { formatBreakpointMods, getLanguageDirection } from '../../helpers/util'; -import { GenerateId } from '../../helpers/GenerateId/GenerateId'; +import { useSSRSafeId } from '../../helpers'; import { FocusTrap } from '../../helpers/FocusTrap/FocusTrap'; import cssPanelMdFlexBasis from '@patternfly/react-tokens/dist/esm/c_drawer__panel_md_FlexBasis'; import cssPanelMdFlexBasisMin from '@patternfly/react-tokens/dist/esm/c_drawer__panel_md_FlexBasis_min'; @@ -83,6 +83,7 @@ export const DrawerPanelContent: React.FunctionComponent { + const panelId = useSSRSafeId('pf-drawer-panel-'); const panel = useRef(undefined); const splitterRef = useRef(undefined); const [separatorValue, setSeparatorValue] = useState(0); @@ -332,105 +333,99 @@ export const DrawerPanelContent: React.FunctionComponent panel.current, + onActivate: () => { + if (previouslyFocusedElement.current !== document.activeElement) { + previouslyFocusedElement.current = document.activeElement; + } + }, + onDeactivate: () => { + previouslyFocusedElement.current && + previouslyFocusedElement.current.focus && + previouslyFocusedElement.current.focus(); + }, + clickOutsideDeactivates: true, + returnFocusOnDeactivate: false, + // FocusTrap's initialFocus can accept false as a value to prevent initial focus. + // We want to prevent this in case false is ever passed in. + initialFocus: focusTrap?.elementToFocusOnExpand || undefined, + escapeDeactivates: false + } + }; + return ( - - {(panelId) => { - const focusTrapProps = { - tabIndex: -1, - 'aria-modal': true, - role: 'dialog', - active: isFocusTrapActive, - 'aria-labelledby': focusTrap?.['aria-labelledby'] || id || panelId, - focusTrapOptions: { - fallbackFocus: () => panel.current, - onActivate: () => { - if (previouslyFocusedElement.current !== document.activeElement) { - previouslyFocusedElement.current = document.activeElement; - } - }, - onDeactivate: () => { - previouslyFocusedElement.current && - previouslyFocusedElement.current.focus && - previouslyFocusedElement.current.focus(); - }, - clickOutsideDeactivates: true, - returnFocusOnDeactivate: false, - // FocusTrap's initialFocus can accept false as a value to prevent initial focus. - // We want to prevent this in case false is ever passed in. - initialFocus: focusTrap?.elementToFocusOnExpand || undefined, - escapeDeactivates: false + { + if ((ev.target as HTMLElement) === panel.current) { + if (!hidden && ev.nativeEvent.propertyName === 'transform') { + onExpand(ev); } - }; - - return ( - { - if ((ev.target as HTMLElement) === panel.current) { - if (!hidden && ev.nativeEvent.propertyName === 'transform') { - onExpand(ev); - } - setIsExpandedInternal(!hidden); - // We also need to collapse the space when the panel is hidden to prevent automation from scrolling to it - if (hidden && ev.nativeEvent.propertyName === 'transform') { - setShouldCollapseSpace(true); - } - if (isValidFocusTrap && ev.nativeEvent.propertyName === 'transform') { - setIsFocusTrapActive((prevIsFocusTrapActive) => !prevIsFocusTrapActive); - } - } - }} - hidden={hidden} - style={{ - ...((defaultSize || minSize || maxSize) && boundaryCssVars), - ...style - }} - {...(shouldCollapseSpace && { inert: '' })} - {...props} - ref={panel} - > - {isExpandedInternal && ( - - {isResizable && ( - -
-
-
-
{children}
- - )} - {!isResizable && children} - - )} - - ); + setIsExpandedInternal(!hidden); + // We also need to collapse the space when the panel is hidden to prevent automation from scrolling to it + if (hidden && ev.nativeEvent.propertyName === 'transform') { + setShouldCollapseSpace(true); + } + if (isValidFocusTrap && ev.nativeEvent.propertyName === 'transform') { + setIsFocusTrapActive((prevIsFocusTrapActive) => !prevIsFocusTrapActive); + } + } + }} + hidden={hidden} + style={{ + ...((defaultSize || minSize || maxSize) && boundaryCssVars), + ...style }} - + {...(shouldCollapseSpace && { inert: '' })} + {...props} + ref={panel} + > + {isExpandedInternal && ( + + {isResizable && ( + +
+
+
+
{children}
+ + )} + {!isResizable && children} + + )} + ); }; DrawerPanelContent.displayName = 'DrawerPanelContent'; diff --git a/packages/react-core/src/components/DualListSelector/DualListSelector.tsx b/packages/react-core/src/components/DualListSelector/DualListSelector.tsx index 29b6b9d5fd8..1c21c05f3f4 100644 --- a/packages/react-core/src/components/DualListSelector/DualListSelector.tsx +++ b/packages/react-core/src/components/DualListSelector/DualListSelector.tsx @@ -1,6 +1,6 @@ import styles from '@patternfly/react-styles/css/components/DualListSelector/dual-list-selector'; import { css } from '@patternfly/react-styles'; -import { GenerateId } from '../../helpers'; +import { useSSRSafeId } from '../../helpers'; import { DualListSelectorContext } from './DualListSelectorContext'; import { useHasAnimations } from '../../helpers'; @@ -33,24 +33,17 @@ export const DualListSelector: React.FunctionComponent = ...props }: DualListSelectorProps) => { const hasAnimations = useHasAnimations(hasAnimationsProp); + const randomId = useSSRSafeId(); return ( - - {(randomId) => ( -
- {children} -
- )} - +
+ {children} +
); }; diff --git a/packages/react-core/src/components/DualListSelector/DualListSelectorListItem.tsx b/packages/react-core/src/components/DualListSelector/DualListSelectorListItem.tsx index 427ce035cab..c994ffc07ef 100644 --- a/packages/react-core/src/components/DualListSelector/DualListSelectorListItem.tsx +++ b/packages/react-core/src/components/DualListSelector/DualListSelectorListItem.tsx @@ -1,7 +1,7 @@ import { forwardRef, useContext, useRef } from 'react'; import styles from '@patternfly/react-styles/css/components/DualListSelector/dual-list-selector'; import { css } from '@patternfly/react-styles'; -import { getUniqueId } from '../../helpers'; +import { useSSRSafeId } from '../../helpers'; import GripVerticalIcon from '@patternfly/react-icons/dist/esm/icons/grip-vertical-icon'; import { Button, ButtonVariant } from '../Button'; import { DualListSelectorListContext } from './DualListSelectorContext'; @@ -38,7 +38,7 @@ export const DualListSelectorListItemBase: React.FunctionComponent { + const generatedId = useSSRSafeId('dual-list-selector-list-item'); + const id = idProp ?? generatedId; const privateRef = useRef(null); const ref = innerRef || privateRef; const { setFocusedOption } = useContext(DualListSelectorListContext); diff --git a/packages/react-core/src/components/DualListSelector/DualListSelectorListWrapper.tsx b/packages/react-core/src/components/DualListSelector/DualListSelectorListWrapper.tsx index 67cf4e1377f..e4f8bcb73f5 100644 --- a/packages/react-core/src/components/DualListSelector/DualListSelectorListWrapper.tsx +++ b/packages/react-core/src/components/DualListSelector/DualListSelectorListWrapper.tsx @@ -1,7 +1,7 @@ import { forwardRef, useContext, useEffect, useRef, useState } from 'react'; import styles from '@patternfly/react-styles/css/components/DualListSelector/dual-list-selector'; import { css } from '@patternfly/react-styles'; -import { getUniqueId, handleArrows } from '../../helpers'; +import { handleArrows, useSSRSafeId } from '../../helpers'; import { DualListSelectorList } from './DualListSelectorList'; import { DualListSelectorContext, DualListSelectorListContext } from './DualListSelectorContext'; @@ -34,10 +34,12 @@ export const DualListSelectorListWrapperBase: React.FunctionComponent { + const generatedId = useSSRSafeId('dual-list-selector-list'); + const id = idProp ?? generatedId; const [focusedOption, setFocusedOption] = useState(''); const ref = useRef(null); const menuRef = innerRef || ref; diff --git a/packages/react-core/src/components/DualListSelector/DualListSelectorPane.tsx b/packages/react-core/src/components/DualListSelector/DualListSelectorPane.tsx index c3fecbda502..c7b9a8f98a6 100644 --- a/packages/react-core/src/components/DualListSelector/DualListSelectorPane.tsx +++ b/packages/react-core/src/components/DualListSelector/DualListSelectorPane.tsx @@ -1,6 +1,6 @@ import styles from '@patternfly/react-styles/css/components/DualListSelector/dual-list-selector'; import { css } from '@patternfly/react-styles'; -import { getUniqueId } from '../../helpers'; +import { useSSRSafeId } from '../../helpers'; import { DualListSelectorListWrapper } from './DualListSelectorListWrapper'; import { DualListSelectorPaneContext } from './DualListSelectorContext'; import { SearchInput } from '../SearchInput'; @@ -44,50 +44,54 @@ export const DualListSelectorPane: React.FunctionComponent ( -
- {title && ( -
-
-
{title}
+}: DualListSelectorPaneProps) => { + const generatedId = useSSRSafeId('dual-list-selector-pane'); + const id = idProp ?? generatedId; + return ( +
+ {title && ( +
+
+
{title}
+
+
+ )} + {(actions || searchInput) && ( +
+ {searchInput && ( +
+ {searchInput ? searchInput : } +
+ )} + {actions &&
{actions}
}
-
- )} - {(actions || searchInput) && ( -
- {searchInput && ( -
- {searchInput ? searchInput : } + )} + {status && ( +
+
+ {status}
- )} - {actions &&
{actions}
} -
- )} - {status && ( -
-
- {status}
-
- )} - - - {children} - - -
-); + )} + + + {children} + + +
+ ); +}; DualListSelectorPane.displayName = 'DualListSelectorPane'; diff --git a/packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx b/packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx index 220e83bab9d..6375bf6e3d9 100644 --- a/packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx +++ b/packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx @@ -4,7 +4,7 @@ import { css } from '@patternfly/react-styles'; import lineClamp from '@patternfly/react-tokens/dist/esm/c_expandable_section_m_truncate__content_LineClamp'; import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon'; import { PickOptional } from '../../helpers/typeUtils'; -import { debounce, getUniqueId } from '../../helpers/util'; +import { debounce } from '../../helpers/util'; import { getResizeObserver } from '../../helpers/resizeObserver'; import { Button } from '../Button'; @@ -91,6 +91,9 @@ interface ExpandableSectionState { previousWidth: number; } +let expandableSectionContentId = 0; +let expandableSectionToggleId = 0; + const directionClassMap = { up: styles.modifiers.expandTop, down: styles.modifiers.expandBottom @@ -106,6 +109,9 @@ const setLineClamp = (lines: number, element: HTMLDivElement) => { class ExpandableSection extends Component { static displayName = 'ExpandableSection'; + private generatedContentId = `expandable-section-content-${expandableSectionContentId++}`; + private generatedToggleId = `expandable-section-toggle-${expandableSectionToggleId++}`; + constructor(props: ExpandableSectionProps) { super(props); @@ -242,8 +248,8 @@ class ExpandableSection extends Component, 'label'> { /** Anything that can be rendered as FormGroup content. */ @@ -47,6 +47,7 @@ export const FormGroup: React.FunctionComponent = ({ role, ...props }: FormGroupProps) => { + const randomId = useSSRSafeId(); const isGroupOrRadioGroup = role === 'group' || role === 'radiogroup'; const LabelComponent = isGroupOrRadioGroup ? 'span' : 'label'; @@ -66,44 +67,36 @@ export const FormGroup: React.FunctionComponent = ({ ); return ( - - {(randomId) => ( +
+ {label && (
- {label && ( -
- {labelInfo && ( - -
{labelContent}
-
{labelInfo}
- - )} - {!labelInfo && labelContent} -
+ {labelInfo && ( + +
{labelContent}
+
{labelInfo}
+ )} -
- {children} -
+ {!labelInfo && labelContent}
)} - +
+ {children} +
+
); }; FormGroup.displayName = 'FormGroup'; diff --git a/packages/react-core/src/components/Form/InternalFormFieldGroup.tsx b/packages/react-core/src/components/Form/InternalFormFieldGroup.tsx index bdd55af3e00..b8ea658482f 100644 --- a/packages/react-core/src/components/Form/InternalFormFieldGroup.tsx +++ b/packages/react-core/src/components/Form/InternalFormFieldGroup.tsx @@ -1,7 +1,7 @@ import styles from '@patternfly/react-styles/css/components/Form/form'; import { css } from '@patternfly/react-styles'; import { FormFieldGroupToggle } from './FormFieldGroupToggle'; -import { GenerateId } from '../../helpers'; +import { useSSRSafeId } from '../../helpers'; import { useHasAnimations } from '../../helpers'; export interface InternalFormFieldGroupProps extends Omit, 'label' | 'onToggle'> { @@ -37,6 +37,7 @@ export const InternalFormFieldGroup: React.FunctionComponent { + const toggleId = useSSRSafeId('form-field-group-toggle'); const hasAnimations = useHasAnimations(hasAnimationsProp); const headerTitleText = header ? header.props.titleText : null; if (isExpandable && !toggleAriaLabel && !headerTitleText) { @@ -59,17 +60,13 @@ export const InternalFormFieldGroup: React.FunctionComponent {isExpandable && ( - - {(id) => ( - - )} - + )} {header && header} {(!isExpandable || (isExpandable && isExpanded) || (hasAnimations && isExpandable)) && ( diff --git a/packages/react-core/src/components/JumpLinks/JumpLinks.tsx b/packages/react-core/src/components/JumpLinks/JumpLinks.tsx index 8b3923add37..334823f7712 100644 --- a/packages/react-core/src/components/JumpLinks/JumpLinks.tsx +++ b/packages/react-core/src/components/JumpLinks/JumpLinks.tsx @@ -7,7 +7,8 @@ import cssToggleDisplayVar from '@patternfly/react-tokens/dist/esm/c_jump_links_ import { Button } from '../Button'; import { JumpLinksItem, JumpLinksItemProps } from './JumpLinksItem'; import { JumpLinksList } from './JumpLinksList'; -import { canUseDOM, formatBreakpointMods, getUniqueId } from '../../helpers/util'; +import { canUseDOM, formatBreakpointMods } from '../../helpers/util'; +import { useSSRSafeId } from '../../helpers'; export interface JumpLinksProps extends Omit, 'label'> { /** Whether to center children. */ @@ -99,6 +100,7 @@ export const JumpLinks: React.FunctionComponent = ({ labelId, ...props }: JumpLinksProps) => { + const generatedId = useSSRSafeId(); const hasScrollSpy = Boolean(scrollableRef || scrollableSelector); const [scrollItems, setScrollItems] = useState(hasScrollSpy ? getScrollItems(children, []) : []); const [activeIndex, setActiveIndex] = useState(activeIndexProp); @@ -244,7 +246,7 @@ export const JumpLinks: React.FunctionComponent = ({ return child; }); - const id = labelId ?? getUniqueId(); + const id = labelId ?? generatedId; const hasAriaLabelledBy = expandable || (label && alwaysShowLabel); const computedAriaLabel = hasAriaLabelledBy ? null : ariaLabel; const computedAriaLabelledBy = hasAriaLabelledBy ? id : null; diff --git a/packages/react-core/src/components/JumpLinks/__tests__/JumpLinks.test.tsx b/packages/react-core/src/components/JumpLinks/__tests__/JumpLinks.test.tsx index 1380822db97..0bf589d116b 100644 --- a/packages/react-core/src/components/JumpLinks/__tests__/JumpLinks.test.tsx +++ b/packages/react-core/src/components/JumpLinks/__tests__/JumpLinks.test.tsx @@ -2,9 +2,6 @@ import { render, screen } from '@testing-library/react'; import { JumpLinks } from '../JumpLinks'; import { JumpLinksItem } from '../JumpLinksItem'; import { JumpLinksList } from '../JumpLinksList'; -import * as utils from '../../../helpers/util'; - -jest.spyOn(utils, 'getUniqueId').mockReturnValue('unique_id_mock'); test('simple jumplinks', () => { const { asFragment } = render( @@ -140,7 +137,8 @@ test('uses aria-labelledby over aria-label when expandable is passed in', () => test('random id is used with aria-labelledby by default in expandable case', () => { render({expandableJumpLinksItems}); const navigation = screen.getByRole('navigation', { name: /Toggle jump links/i }); - expect(navigation).toHaveAttribute('aria-labelledby', 'unique_id_mock'); + expect(navigation).toHaveAttribute('aria-labelledby'); + expect(navigation.getAttribute('aria-labelledby')).toBeTruthy(); }); test('random id is used with aria-labelledby by default in label case', () => { @@ -150,7 +148,8 @@ test('random id is used with aria-labelledby by default in label case', () => { ); const navigation = screen.getByRole('navigation', { name: /Jump to section/i }); - expect(navigation).toHaveAttribute('aria-labelledby', 'unique_id_mock'); + expect(navigation).toHaveAttribute('aria-labelledby'); + expect(navigation.getAttribute('aria-labelledby')).toBeTruthy(); }); test('custom labelId is used with aria-labelledby when it is passed in in expandable case', () => { diff --git a/packages/react-core/src/components/Menu/MenuItem.tsx b/packages/react-core/src/components/Menu/MenuItem.tsx index 3e5f037bd73..e02308467ed 100644 --- a/packages/react-core/src/components/Menu/MenuItem.tsx +++ b/packages/react-core/src/components/Menu/MenuItem.tsx @@ -14,7 +14,7 @@ import { MenuItemAction } from './MenuItemAction'; import { Tooltip, TooltipProps } from '../Tooltip'; import { canUseDOM } from '../../helpers/util'; import { useIsomorphicLayoutEffect } from '../../helpers/useIsomorphicLayout'; -import { GenerateId } from '../../helpers/GenerateId/GenerateId'; +import { useSSRSafeId } from '../../helpers'; export interface MenuItemProps extends Omit, 'onClick'> { /** Content rendered inside the menu list item. */ @@ -151,6 +151,7 @@ const MenuItemBase: React.FunctionComponent = ({ const innerComponentRef = innerRef || privateRef; const flyoutVisible = ref === flyoutRef; + const randomId = useSSRSafeId(); const hasFlyout = flyoutMenu !== undefined; const showFlyout = (show: boolean) => { if (!flyoutVisible && show) { @@ -343,75 +344,71 @@ const MenuItemBase: React.FunctionComponent = ({ const renderItem = ( <> - - {(randomId) => ( - { - if (!isAriaDisabled) { - onItemSelect(event, onSelect); - drill && drill(event); - flyoutMenu && handleFlyout(event); - } else { - event.preventDefault(); - } - } - })} - {...(hasCheckbox && { htmlFor: randomId })} - {...additionalProps} - > - - {direction === 'up' && ( - - - - )} - {icon && {icon}} - {hasCheckbox && ( - - onItemSelect(event, onSelect)} - isDisabled={isDisabled} - aria-disabled={isAriaDisabled} - /> - - )} - {children} - {isExternalLink && ( - - - - )} - {(flyoutMenu || direction === 'down') && ( - - - - )} - {getIsSelected() && ( - - - - )} + { + if (!isAriaDisabled) { + onItemSelect(event, onSelect); + drill && drill(event); + flyoutMenu && handleFlyout(event); + } else { + event.preventDefault(); + } + } + })} + {...(hasCheckbox && { htmlFor: randomId })} + {...additionalProps} + > + + {direction === 'up' && ( + + + + )} + {icon && {icon}} + {hasCheckbox && ( + + onItemSelect(event, onSelect)} + isDisabled={isDisabled} + aria-disabled={isAriaDisabled} + /> + + )} + {children} + {isExternalLink && ( + + + + )} + {(flyoutMenu || direction === 'down') && ( + + + + )} + {getIsSelected() && ( + + - {description && direction !== 'up' && ( - - {description} - - )} - + )} + + {description && direction !== 'up' && ( + + {description} + )} - + {flyoutVisible && ( {flyoutMenu} diff --git a/packages/react-core/src/components/Modal/Modal.tsx b/packages/react-core/src/components/Modal/Modal.tsx index 99a12d77b48..d6b930a6588 100644 --- a/packages/react-core/src/components/Modal/Modal.tsx +++ b/packages/react-core/src/components/Modal/Modal.tsx @@ -63,6 +63,7 @@ export enum ModalVariant { interface ModalState { ouiaStateId: string; + mounted: boolean; } class Modal extends Component { @@ -87,7 +88,8 @@ class Modal extends Component { this.backdropId = `pf-modal-part-${backdropId}`; this.state = { - ouiaStateId: getDefaultOUIAId(Modal.displayName, props.variant) + ouiaStateId: getDefaultOUIAId(Modal.displayName, props.variant), + mounted: false }; } @@ -119,6 +121,7 @@ class Modal extends Component { isEmpty = (value: string | null | undefined) => value === null || value === undefined || value === ''; componentDidMount() { + this.setState({ mounted: true }); const { appendTo } = this.props; const target: HTMLElement = this.getElement(appendTo); target.addEventListener('keydown', this.handleEscKeyClick, false); @@ -166,7 +169,7 @@ class Modal extends Component { ...props } = this.props; - if (!canUseDOM || !this.getElement(appendTo)) { + if (!this.state.mounted || !canUseDOM || !this.getElement(appendTo)) { return null; } diff --git a/packages/react-core/src/components/MultipleFileUpload/MultipleFileUploadStatus.tsx b/packages/react-core/src/components/MultipleFileUpload/MultipleFileUploadStatus.tsx index e987b89932f..9e0641477ee 100644 --- a/packages/react-core/src/components/MultipleFileUpload/MultipleFileUploadStatus.tsx +++ b/packages/react-core/src/components/MultipleFileUpload/MultipleFileUploadStatus.tsx @@ -2,7 +2,7 @@ import { useEffect, useState } from 'react'; import styles from '@patternfly/react-styles/css/components/MultipleFileUpload/multiple-file-upload'; import { css } from '@patternfly/react-styles'; import { ExpandableSection } from '../ExpandableSection'; -import { GenerateId } from '../../helpers/GenerateId/GenerateId'; +import { useSSRSafeId } from '../../helpers'; import InProgressIcon from '@patternfly/react-icons/dist/esm/icons/in-progress-icon'; import CheckCircleIcon from '@patternfly/react-icons/dist/esm/icons/check-circle-icon'; import TimesCircleIcon from '@patternfly/react-icons/dist/esm/icons/times-circle-icon'; @@ -34,6 +34,7 @@ export const MultipleFileUploadStatus: React.FunctionComponent { + const expandableSectionId = useSSRSafeId('pf-expandable-section-'); const [icon, setIcon] = useState(); const [isOpen, setIsOpen] = useState(true); @@ -66,21 +67,17 @@ export const MultipleFileUploadStatus: React.FunctionComponent - - {(expandableSectionId) => ( - -
    - {children} -
- - )} - + +
    + {children} +
+
); }; diff --git a/packages/react-core/src/components/Nav/NavExpandable.tsx b/packages/react-core/src/components/Nav/NavExpandable.tsx index bb40ad633fc..4c9304432aa 100644 --- a/packages/react-core/src/components/Nav/NavExpandable.tsx +++ b/packages/react-core/src/components/Nav/NavExpandable.tsx @@ -2,14 +2,16 @@ import { Component } from 'react'; import styles from '@patternfly/react-styles/css/components/Nav/nav'; import { css } from '@patternfly/react-styles'; import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon'; -import { getUniqueId } from '../../helpers/util'; import { NavContext } from './Nav'; import { PageSidebarContext } from '../Page/PageSidebar'; import { PickOptional } from '../../helpers/typeUtils'; import { getOUIAProps, OUIAProps, getDefaultOUIAId } from '../../helpers'; +let navExpandableId = 0; + export interface NavExpandableProps - extends Omit, HTMLLIElement>, 'title'>, OUIAProps { + extends Omit, HTMLLIElement>, 'title'>, + OUIAProps { /** Title content shown for the expandable list */ title: React.ReactNode; /** If defined, screen readers will read this text instead of the list title */ @@ -51,7 +53,7 @@ class NavExpandable extends Component { id: '' }; - id = this.props.id || getUniqueId(); + id = this.props.id || `pf-nav-expandable-${navExpandableId++}`; state = { expandedState: this.props.isExpanded, diff --git a/packages/react-core/src/components/Nav/NavGroup.tsx b/packages/react-core/src/components/Nav/NavGroup.tsx index 85f61a20746..310247d5f14 100644 --- a/packages/react-core/src/components/Nav/NavGroup.tsx +++ b/packages/react-core/src/components/Nav/NavGroup.tsx @@ -1,6 +1,6 @@ import styles from '@patternfly/react-styles/css/components/Nav/nav'; import { css } from '@patternfly/react-styles'; -import { getUniqueId } from '../../helpers/util'; +import { useSSRSafeId } from '../../helpers'; export interface NavGroupProps extends React.HTMLProps { /** Title shown for the group */ @@ -17,9 +17,11 @@ export const NavGroup: React.FunctionComponent = ({ title, children = null, className = '', - id = getUniqueId(), + id: idProp, ...props }: NavGroupProps) => { + const generatedId = useSSRSafeId(); + const id = idProp ?? generatedId; if (!title && !props['aria-label']) { // eslint-disable-next-line no-console console.warn("For accessibility reasons an aria-label should be specified on nav groups if a title isn't"); diff --git a/packages/react-core/src/components/Popover/Popover.tsx b/packages/react-core/src/components/Popover/Popover.tsx index ec7557d6630..9585a13bf7d 100644 --- a/packages/react-core/src/components/Popover/Popover.tsx +++ b/packages/react-core/src/components/Popover/Popover.tsx @@ -13,7 +13,7 @@ import popoverMaxWidth from '@patternfly/react-tokens/dist/esm/c_popover_MaxWidt import popoverMinWidth from '@patternfly/react-tokens/dist/esm/c_popover_MinWidth'; import { FocusTrap } from '../../helpers'; import { Popper } from '../../helpers/Popper/Popper'; -import { getUniqueId } from '../../helpers/util'; +import { useSSRSafeId } from '../../helpers'; export enum PopoverPosition { auto = 'auto', @@ -276,7 +276,8 @@ export const Popover: React.FunctionComponent = ({ }: PopoverProps) => { // could make this a prop in the future (true | false | 'toggle') // const hideOnClick = true; - const uniqueId = id || getUniqueId(); + const generatedId = useSSRSafeId(); + const uniqueId = id || generatedId; const triggerManually = isVisible !== null; const [visible, setVisible] = useState(false); const [focusTrapActive, setFocusTrapActive] = useState(Boolean(propWithFocusTrap)); diff --git a/packages/react-core/src/components/Progress/Progress.tsx b/packages/react-core/src/components/Progress/Progress.tsx index ab1ec5151d5..64024a99b95 100644 --- a/packages/react-core/src/components/Progress/Progress.tsx +++ b/packages/react-core/src/components/Progress/Progress.tsx @@ -3,7 +3,8 @@ import styles from '@patternfly/react-styles/css/components/Progress/progress'; import { css } from '@patternfly/react-styles'; import { ProgressContainer, ProgressMeasureLocation } from './ProgressContainer'; import { AriaProps } from './ProgressBar'; -import { getUniqueId } from '../../helpers/util'; + +let progressId = 0; export enum ProgressSize { sm = 'sm', @@ -73,7 +74,7 @@ class Progress extends Component { 'aria-describedby': null as string }; - id = this.props.id || getUniqueId(); + id = this.props.id || `pf-progress-${progressId++}`; render() { const { diff --git a/packages/react-core/src/components/Radio/Radio.tsx b/packages/react-core/src/components/Radio/Radio.tsx index d2fb4127056..ec4886c103f 100644 --- a/packages/react-core/src/components/Radio/Radio.tsx +++ b/packages/react-core/src/components/Radio/Radio.tsx @@ -3,10 +3,12 @@ import styles from '@patternfly/react-styles/css/components/Radio/radio'; import { css } from '@patternfly/react-styles'; import { PickOptional } from '../../helpers/typeUtils'; import { getOUIAProps, OUIAProps, getDefaultOUIAId } from '../../helpers'; -import { getUniqueId } from '../../helpers/util'; + +let radioDescriptionId = 0; export interface RadioProps - extends Omit, 'disabled' | 'label' | 'onChange' | 'type'>, OUIAProps { + extends Omit, 'disabled' | 'label' | 'onChange' | 'type'>, + OUIAProps { /** Additional classes added to the radio wrapper. This wrapper will be div element by default. It will be a label element if * isLabelWrapped is true, or it can be overridden by any element specified in the component prop. */ @@ -66,7 +68,7 @@ class Radio extends Component { + const hasWordsId = useSSRSafeId(); const firstAttrRef = useRef(null); const [putFocusBackOnInput, setPutFocusBackOnInput] = useState(false); @@ -182,18 +183,14 @@ export const AdvancedSearchMenu: React.FunctionComponent - {(randomId) => ( - - handleValueChange('haswords', value, evt)} - /> - - )} - + + handleValueChange('haswords', value, evt)} + /> + ); return formGroups; }; diff --git a/packages/react-core/src/components/Switch/Switch.tsx b/packages/react-core/src/components/Switch/Switch.tsx index 567319e9c0f..dbb7a95276b 100644 --- a/packages/react-core/src/components/Switch/Switch.tsx +++ b/packages/react-core/src/components/Switch/Switch.tsx @@ -2,11 +2,13 @@ import { Component, Fragment } from 'react'; import styles from '@patternfly/react-styles/css/components/Switch/switch'; import { css } from '@patternfly/react-styles'; import CheckIcon from '@patternfly/react-icons/dist/esm/icons/check-icon'; -import { getUniqueId } from '../../helpers/util'; import { getOUIAProps, OUIAProps, getDefaultOUIAId } from '../../helpers'; +let switchId = 0; + export interface SwitchProps - extends Omit, 'type' | 'onChange' | 'disabled' | 'label'>, OUIAProps { + extends Omit, 'type' | 'onChange' | 'disabled' | 'label'>, + OUIAProps { /** id for the label. */ id?: string; /** Additional classes added to the switch */ @@ -61,7 +63,7 @@ class Switch extends Component ); } - this.id = props.id || getUniqueId(); + this.id = props.id || `pf-switch-${switchId++}`; this.state = { ouiaStateId: getDefaultOUIAId(Switch.displayName) }; diff --git a/packages/react-core/src/components/TimePicker/TimePicker.tsx b/packages/react-core/src/components/TimePicker/TimePicker.tsx index f46b010c504..7b3ea8c0fa0 100644 --- a/packages/react-core/src/components/TimePicker/TimePicker.tsx +++ b/packages/react-core/src/components/TimePicker/TimePicker.tsx @@ -2,7 +2,6 @@ import { Component, createRef } from 'react'; import { css } from '@patternfly/react-styles'; import datePickerStyles from '@patternfly/react-styles/css/components/DatePicker/date-picker'; import menuStyles from '@patternfly/react-styles/css/components/Menu/menu'; -import { getUniqueId } from '../../helpers'; import { Popper } from '../../helpers/Popper/Popper'; import { Menu, MenuContent, MenuList, MenuItem } from '../Menu'; import { InputGroup, InputGroupItem } from '../InputGroup'; @@ -23,10 +22,10 @@ import { HelperText, HelperTextItem } from '../HelperText'; import OutlinedClockIcon from '@patternfly/react-icons/dist/esm/icons/outlined-clock-icon'; import cssDatePickerFormControlWidth from '@patternfly/react-tokens/dist/esm/c_date_picker__input_c_form_control_Width'; -export interface TimePickerProps extends Omit< - React.HTMLProps, - 'onChange' | 'onFocus' | 'onBlur' | 'disabled' | 'ref' -> { +let timePickerId = 0; + +export interface TimePickerProps + extends Omit, 'onChange' | 'onFocus' | 'onBlur' | 'disabled' | 'ref'> { /** Additional classes added to the time picker. */ className?: string; /** Accessible label for the time picker */ @@ -125,8 +124,11 @@ class TimePicker extends Component { zIndex: 9999 }; + private generatedId: string; + constructor(props: TimePickerProps) { super(props); + this.generatedId = props.id || `time-picker-${timePickerId++}`; const { is24Hour, delimiter, time, includeSeconds, isOpen } = this.props; let { minTime, maxTime } = this.props; if (minTime === '') { @@ -479,7 +481,7 @@ class TimePicker extends Component { const style = { [cssDatePickerFormControlWidth.name]: width } as React.CSSProperties; const options = makeTimeOptions(stepMinutes, !is24Hour, delimiter, minTimeState, maxTimeState, includeSeconds); const isValidFormat = this.isValidFormat(timeState); - const randomId = id || getUniqueId('time-picker'); + const randomId = id || this.generatedId; const getParentElement = () => { if (this.baseComponentRef && this.baseComponentRef.current) { diff --git a/packages/react-core/src/components/Toolbar/Toolbar.tsx b/packages/react-core/src/components/Toolbar/Toolbar.tsx index 1b64ab84461..9e43a712f54 100644 --- a/packages/react-core/src/components/Toolbar/Toolbar.tsx +++ b/packages/react-core/src/components/Toolbar/Toolbar.tsx @@ -86,7 +86,7 @@ class Toolbar extends Component { state = { isManagedToggleExpanded: false, filterInfo: {}, - windowWidth: canUseDOM ? window.innerWidth : 1200, + windowWidth: 1200, ouiaStateId: getDefaultOUIAId(Toolbar.displayName) }; @@ -108,6 +108,9 @@ class Toolbar extends Component { }; componentDidMount() { + if (canUseDOM) { + this.setState({ windowWidth: window.innerWidth }); + } if (this.isToggleManaged() && canUseDOM) { window.addEventListener('resize', this.closeExpandableContent); } diff --git a/packages/react-core/src/components/Toolbar/ToolbarLabelGroupContent.tsx b/packages/react-core/src/components/Toolbar/ToolbarLabelGroupContent.tsx index 6a729aee4f0..0b0aac39ac7 100644 --- a/packages/react-core/src/components/Toolbar/ToolbarLabelGroupContent.tsx +++ b/packages/react-core/src/components/Toolbar/ToolbarLabelGroupContent.tsx @@ -62,9 +62,9 @@ class ToolbarLabelGroupContent extends Component let collapseListedFilters = false; if (collapseListedFiltersBreakpoint === 'all') { collapseListedFilters = true; - } else if (canUseDOM) { - collapseListedFilters = - (canUseDOM ? window.innerWidth : 1200) < globalBreakpoints[collapseListedFiltersBreakpoint]; + } else if (collapseListedFiltersBreakpoint) { + const viewportWidth = canUseDOM ? window.innerWidth : 1200; + collapseListedFilters = viewportWidth < globalBreakpoints[collapseListedFiltersBreakpoint]; } const isHidden = numberOfFilters === 0 || isExpanded; diff --git a/packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx b/packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx index 408434a654c..d5caed8a187 100644 --- a/packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx +++ b/packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx @@ -6,7 +6,7 @@ import { ToolbarGroupProps } from './ToolbarGroup'; import { ToolbarContext, ToolbarContentContext } from './ToolbarUtils'; import { Button } from '../Button'; import globalBreakpointLg from '@patternfly/react-tokens/dist/esm/t_global_breakpoint_lg'; -import { formatBreakpointMods, toCamel, canUseDOM } from '../../helpers/util'; +import { formatBreakpointMods, toCamel } from '../../helpers/util'; import { PageContext } from '../Page/PageContext'; import { ToolbarExpandableContent } from './ToolbarExpandableContent'; @@ -157,7 +157,7 @@ class ToolbarToggleGroup extends Component { expandableContentRef = createRef(); isContentPopup = () => { - const viewportSize = canUseDOM ? window.innerWidth : 1200; + const viewportSize = typeof window !== 'undefined' ? window.innerWidth : 1200; const lgBreakpointValue = parseInt(globalBreakpointLg.value); return viewportSize < lgBreakpointValue; }; diff --git a/packages/react-core/src/components/Tooltip/Tooltip.tsx b/packages/react-core/src/components/Tooltip/Tooltip.tsx index 8a36eb56494..24905a8c72a 100644 --- a/packages/react-core/src/components/Tooltip/Tooltip.tsx +++ b/packages/react-core/src/components/Tooltip/Tooltip.tsx @@ -4,6 +4,7 @@ import { css } from '@patternfly/react-styles'; import { TooltipContent } from './TooltipContent'; import { TooltipArrow } from './TooltipArrow'; import { KeyTypes } from '../../helpers/constants'; +import { useSSRSafeId } from '../../helpers'; import tooltipMaxWidth from '@patternfly/react-tokens/dist/esm/c_tooltip_MaxWidth'; import { Popper } from '../../helpers/Popper/Popper'; @@ -136,9 +137,6 @@ export interface TooltipProps extends Omit, 'con animationDuration?: number; } -// id for associating trigger with the content aria-describedby or aria-labelledby -let pfTooltipIdCounter = 1; - export const Tooltip: React.FunctionComponent = ({ content: bodyContent, position = 'top', @@ -157,7 +155,7 @@ export const Tooltip: React.FunctionComponent = ({ aria = 'describedby', // For every initial starting position, there are 3 escape positions flipBehavior = ['top', 'right', 'bottom', 'left', 'top', 'right', 'bottom'], - id = `pf-tooltip-${pfTooltipIdCounter++}`, + id: idProp, children, animationDuration = 300, triggerRef, @@ -165,6 +163,8 @@ export const Tooltip: React.FunctionComponent = ({ onTooltipHidden = () => {}, ...rest }: TooltipProps) => { + const generatedId = useSSRSafeId('pf-tooltip-'); + const id = idProp ?? generatedId; // could make this a prop in the future (true | false | 'toggle') const hideOnClick = true; const triggerOnMouseenter = trigger.includes('mouseenter'); diff --git a/packages/react-core/src/components/TreeView/TreeViewListItem.tsx b/packages/react-core/src/components/TreeView/TreeViewListItem.tsx index 5ae835dc483..657f496f3be 100644 --- a/packages/react-core/src/components/TreeView/TreeViewListItem.tsx +++ b/packages/react-core/src/components/TreeView/TreeViewListItem.tsx @@ -4,7 +4,7 @@ import styles from '@patternfly/react-styles/css/components/TreeView/tree-view'; import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon'; import { TreeViewDataItem } from './TreeView'; import { Badge } from '../Badge'; -import { GenerateId } from '../../helpers/GenerateId/GenerateId'; +import { useSSRSafeId } from '../../helpers'; import { useHasAnimations } from '../../helpers'; export interface TreeViewCheckProps extends Omit>, 'checked'> { @@ -113,6 +113,7 @@ const TreeViewListItemBase: React.FunctionComponent = ({ // eslint-disable-next-line @typescript-eslint/no-unused-vars useMemo }: TreeViewListItemProps) => { + const randomId = useSSRSafeId(isSelectable ? 'selectable-id' : 'checkbox-id'); const hasAnimations = useHasAnimations(hasAnimationsProp); const [internalIsExpanded, setIsExpanded] = useState(defaultExpanded); useEffect(() => { @@ -243,41 +244,37 @@ const TreeViewListItemBase: React.FunctionComponent = ({ {...(isFullyDisabled && { 'aria-disabled': true })} >
- - {(randomId) => ( - { - if (!hasCheckbox) { - !isDisabled && onSelect && onSelect(evt, itemData, parentItem); - if (!isDisabled && !isSelectable && children && evt.isDefaultPrevented() !== true) { - if (internalIsExpanded) { - onCollapse && onCollapse(evt, itemData, parentItem); - } else { - onExpand && onExpand(evt, itemData, parentItem); - } - setIsExpanded(!internalIsExpanded); - } - } - }} - {...(hasCheckbox && { htmlFor: randomId })} - {...((hasCheckbox || (isSelectable && children)) && { id: `label-${randomId}` })} - {...(Component === 'button' && { type: 'button', disabled: isDisabled })} - > - - {children && renderToggle(randomId)} - {hasCheckbox && renderCheck(randomId)} - {icon && iconRendered} - {renderNodeContent()} - {badgeRendered} - - + + onClick={(evt: React.MouseEvent) => { + if (!hasCheckbox) { + !isDisabled && onSelect && onSelect(evt, itemData, parentItem); + if (!isDisabled && !isSelectable && children && evt.isDefaultPrevented() !== true) { + if (internalIsExpanded) { + onCollapse && onCollapse(evt, itemData, parentItem); + } else { + onExpand && onExpand(evt, itemData, parentItem); + } + setIsExpanded(!internalIsExpanded); + } + } + }} + {...(hasCheckbox && { htmlFor: randomId })} + {...((hasCheckbox || (isSelectable && children)) && { id: `label-${randomId}` })} + {...(Component === 'button' && { type: 'button', disabled: isDisabled })} + > + + {children && renderToggle(randomId)} + {hasCheckbox && renderCheck(randomId)} + {icon && iconRendered} + {renderNodeContent()} + {badgeRendered} + + {action &&
{action}
}
{(internalIsExpanded || hasAnimations) && clonedChildren} diff --git a/packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx b/packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx index ea735d53002..9cfcffceb18 100644 --- a/packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx +++ b/packages/react-core/src/components/TreeView/__tests__/TreeViewListItem.test.tsx @@ -359,8 +359,8 @@ test(`Renders ${styles.treeViewNode} element with for and id attributes when has const treeViewNode = screen.getByRole('treeitem').querySelector(`.${styles.treeViewNode}`); - expect(treeViewNode).toHaveAttribute('for', expect.stringMatching(/checkbox-id\d+/)); - expect(treeViewNode).toHaveAttribute('id', expect.stringMatching(/label-checkbox-id\d+/)); + expect(treeViewNode).toHaveAttribute('for', expect.stringMatching(/checkbox-id/)); + expect(treeViewNode).toHaveAttribute('id', expect.stringMatching(/label-checkbox-id/)); }); test(`Renders ${styles.treeViewNode} element with id attribute when isSelectable and children are passed`, () => { @@ -372,7 +372,7 @@ test(`Renders ${styles.treeViewNode} element with id attribute when isSelectable const treeViewNode = screen.getByRole('treeitem').querySelector(`.${styles.treeViewNode}`); - expect(treeViewNode).toHaveAttribute('id', expect.stringMatching(/selectable-id\d+/)); + expect(treeViewNode).toHaveAttribute('id', expect.stringMatching(/selectable-id/)); }); test(`Does not render ${styles.treeViewNode} element with additional classes by default`, () => { diff --git a/packages/react-core/src/deprecated/components/Modal/Modal.tsx b/packages/react-core/src/deprecated/components/Modal/Modal.tsx index 530232e4659..4e361b4533f 100644 --- a/packages/react-core/src/deprecated/components/Modal/Modal.tsx +++ b/packages/react-core/src/deprecated/components/Modal/Modal.tsx @@ -93,6 +93,7 @@ export enum ModalVariant { interface ModalState { ouiaStateId: string; + mounted: boolean; } class Modal extends Component { @@ -135,7 +136,8 @@ class Modal extends Component { this.backdropId = `pf-modal-part-${backdropIdNum}`; this.state = { - ouiaStateId: getDefaultOUIAId(Modal.displayName, props.variant) + ouiaStateId: getDefaultOUIAId(Modal.displayName, props.variant), + mounted: false }; } @@ -167,6 +169,7 @@ class Modal extends Component { isEmpty = (value: string | null | undefined) => value === null || value === undefined || value === ''; componentDidMount() { + this.setState({ mounted: true }); const { appendTo, title, @@ -238,7 +241,7 @@ class Modal extends Component { ...props } = this.props; - if (!canUseDOM || !this.getElement(appendTo)) { + if (!this.state.mounted || !canUseDOM || !this.getElement(appendTo)) { return null; } diff --git a/packages/react-core/src/helpers/GenerateId/GenerateId.ts b/packages/react-core/src/helpers/GenerateId/GenerateId.ts index 8732012caa9..a9a96b81e38 100644 --- a/packages/react-core/src/helpers/GenerateId/GenerateId.ts +++ b/packages/react-core/src/helpers/GenerateId/GenerateId.ts @@ -1,4 +1,6 @@ -/** This Component can be used to wrap a functional component in order to generate a random ID +/** This Component can be used to wrap a functional component in order to generate a deterministic ID. + * For function components, prefer the useSSRSafeId hook instead. + * * Example of how to use this component * * const Component = ({id}: {id: string}) => ( @@ -11,25 +13,15 @@ * ); */ import { Component } from 'react'; -import { getUniqueId } from '../util'; let currentId = 0; -// gives us a unique (enough) id to add to the generated id that it should prevent issues with duplicate ids -function getRandomId() { - if (typeof crypto !== 'undefined' && crypto.randomUUID) { - return crypto.randomUUID(); - } else { - return getUniqueId(); - } -} - export interface GenerateIdProps { - /** String to prefix the random id with */ + /** String to prefix the id with */ prefix?: string; /** Component to be rendered with the generated id */ children(id: string): React.ReactNode; - /** Flag to add randomness to the generated id, should be used whenever possible */ + /** @deprecated Has no effect. Kept for backward compatibility. */ isRandom?: boolean; } @@ -39,8 +31,7 @@ class GenerateId extends Component { prefix: 'pf-random-id-', isRandom: false }; - uniqueElement = this.props.isRandom ? getRandomId() : currentId++; - id = `${this.props.prefix}${this.uniqueElement}`; + id = `${this.props.prefix}${currentId++}`; render() { return this.props.children(this.id); diff --git a/packages/react-core/src/helpers/OUIA/ouia.ts b/packages/react-core/src/helpers/OUIA/ouia.ts index 1b3d7ab20cd..83243c4bb39 100644 --- a/packages/react-core/src/helpers/OUIA/ouia.ts +++ b/packages/react-core/src/helpers/OUIA/ouia.ts @@ -79,14 +79,7 @@ export function getDefaultOUIAId(componentType: string, variant?: string) { } */ try { - let key; - if (typeof window !== 'undefined') { - // browser environments - key = `${window.location.href}-${componentType}-${variant || ''}`; - } else { - // node/SSR environments - key = `${componentType}-${variant || ''}`; - } + const key = `${componentType}-${variant || ''}`; if (!ouiaIdByRoute[key]) { ouiaIdByRoute[key] = 0; } diff --git a/packages/react-core/src/helpers/index.ts b/packages/react-core/src/helpers/index.ts index 7e13cd4b17c..d371c957da6 100644 --- a/packages/react-core/src/helpers/index.ts +++ b/packages/react-core/src/helpers/index.ts @@ -8,6 +8,7 @@ export * from './OUIA/ouia'; export * from './util'; export * from './Popper/Popper'; export * from './useIsomorphicLayout'; +export * from './useSSRSafeId'; export * from './KeyboardHandler'; export * from './resizeObserver'; export * from './useInterval'; diff --git a/packages/react-core/src/helpers/useSSRSafeId.ts b/packages/react-core/src/helpers/useSSRSafeId.ts new file mode 100644 index 00000000000..272e36f0404 --- /dev/null +++ b/packages/react-core/src/helpers/useSSRSafeId.ts @@ -0,0 +1,19 @@ +import { useState } from 'react'; + +const react = require('react'); +const reactUseId: (() => string) | undefined = typeof react.useId === 'function' ? react.useId : undefined; + +let counter = 0; + +/** + * SSR-safe ID generation hook. Uses React.useId() when available (React 18+) + * for guaranteed SSR/CSR match. Falls back to a counter-based approach for React 17. + */ +export const useSSRSafeId: (prefix?: string) => string = reactUseId + ? function useSSRSafeId(prefix = 'pf-'): string { + return `${prefix}${reactUseId!()}`; + } + : function useSSRSafeId(prefix = 'pf-'): string { + const [id] = useState(() => `${prefix}${++counter}`); + return id; + }; From 1ff98e02268c36d9629b23ebc3e9edf422f1ce86 Mon Sep 17 00:00:00 2001 From: arpanroy41 Date: Wed, 25 Feb 2026 21:58:52 +0530 Subject: [PATCH 2/9] refactor: replace default ID generation with SSRSafeIds in multiple components - Updated Checkbox, ExpandableSection, FormSelect, Menu, MenuToggle, MenuToggleCheckbox, Modal, Nav, NavExpandable, and Progress components to utilize SSRSafeIds for generating unique IDs. - This change enhances ID management and ensures consistency across components. --- .../src/components/Checkbox/Checkbox.tsx | 155 +++++----- .../ExpandableSection/ExpandableSection.tsx | 115 ++++---- .../src/components/FormSelect/FormSelect.tsx | 67 +++-- .../react-core/src/components/Menu/Menu.tsx | 139 ++++----- .../src/components/MenuToggle/MenuToggle.tsx | 255 ++++++++-------- .../MenuToggle/MenuToggleCheckbox.tsx | 61 ++-- .../react-core/src/components/Modal/Modal.tsx | 43 +-- .../react-core/src/components/Nav/Nav.tsx | 98 +++---- .../src/components/Nav/NavExpandable.tsx | 121 ++++---- .../src/components/Progress/Progress.tsx | 119 ++++---- .../react-core/src/components/Radio/Radio.tsx | 128 ++++---- .../src/components/Select/Select.tsx | 10 +- .../src/components/Switch/Switch.tsx | 113 ++++--- .../react-core/src/components/Tabs/Tabs.tsx | 275 +++++++++--------- .../src/components/TextInput/TextInput.tsx | 85 +++--- .../src/components/TimePicker/TimePicker.tsx | 177 +++++------ .../src/components/Toolbar/Toolbar.tsx | 20 +- .../Toolbar/ToolbarLabelGroupContent.tsx | 14 +- .../src/deprecated/components/Chip/Chip.tsx | 45 +-- .../DualListSelectorListItem.tsx | 6 +- .../DualListSelectorListWrapper.tsx | 6 +- .../DualListSelector/DualListSelectorPane.tsx | 6 +- .../src/deprecated/components/Modal/Modal.tsx | 55 ++-- .../src/helpers/GenerateId/GenerateId.ts | 23 +- packages/react-core/src/helpers/OUIA/ouia.ts | 53 +--- .../src/helpers/SSRSafeIds/SSRSafeIds.tsx | 24 ++ packages/react-core/src/helpers/index.ts | 1 + 27 files changed, 1123 insertions(+), 1091 deletions(-) create mode 100644 packages/react-core/src/helpers/SSRSafeIds/SSRSafeIds.tsx diff --git a/packages/react-core/src/components/Checkbox/Checkbox.tsx b/packages/react-core/src/components/Checkbox/Checkbox.tsx index aa8bcb84c4e..867d9561f75 100644 --- a/packages/react-core/src/components/Checkbox/Checkbox.tsx +++ b/packages/react-core/src/components/Checkbox/Checkbox.tsx @@ -2,11 +2,10 @@ import { Component } from 'react'; import styles from '@patternfly/react-styles/css/components/Check/check'; import { css } from '@patternfly/react-styles'; import { PickOptional } from '../../helpers/typeUtils'; -import { getDefaultOUIAId, getOUIAProps, OUIAProps } from '../../helpers'; +import { getOUIAProps, OUIAProps } from '../../helpers'; +import { SSRSafeIds } from '../../helpers/SSRSafeIds/SSRSafeIds'; import { ASTERISK } from '../../helpers/htmlConstants'; -let checkboxDescriptionId = 0; - export interface CheckboxProps extends Omit, 'type' | 'onChange' | 'disabled' | 'label'>, OUIAProps { @@ -54,12 +53,7 @@ export interface CheckboxProps // tslint:disable-next-line:no-empty const defaultOnChange = () => {}; -interface CheckboxState { - ouiaStateId: string; - descriptionId: string; -} - -class Checkbox extends Component { +class Checkbox extends Component { static displayName = 'Checkbox'; static defaultProps: PickOptional = { className: '', @@ -72,14 +66,6 @@ class Checkbox extends Component { ouiaSafe: true }; - constructor(props: any) { - super(props); - this.state = { - ouiaStateId: getDefaultOUIAId(Checkbox.displayName), - descriptionId: `pf-checkbox-description-${checkboxDescriptionId++}` - }; - } - private handleChange = (event: React.FormEvent): void => { this.props.onChange(event, event.currentTarget.checked); }; @@ -122,76 +108,81 @@ class Checkbox extends Component { checkedProps.defaultChecked = defaultChecked; } - // Handle aria-describedby logic - let ariaDescribedByValue: string | undefined; - if (ariaDescribedBy !== undefined) { - ariaDescribedByValue = ariaDescribedBy === '' ? undefined : ariaDescribedBy; - } else if (description) { - ariaDescribedByValue = this.state.descriptionId; - } + return ( + + {(descriptionId, generatedOuiaId) => { + let ariaDescribedByValue: string | undefined; + if (ariaDescribedBy !== undefined) { + ariaDescribedByValue = ariaDescribedBy === '' ? undefined : ariaDescribedBy; + } else if (description) { + ariaDescribedByValue = descriptionId; + } - const inputRendered = ( - { - elem && (elem.indeterminate = isChecked === null); - }} - {...checkedProps} - {...getOUIAProps(Checkbox.displayName, ouiaId !== undefined ? ouiaId : this.state.ouiaStateId, ouiaSafe)} - /> - ); + const inputRendered = ( + { + elem && (elem.indeterminate = isChecked === null); + }} + {...checkedProps} + {...getOUIAProps(Checkbox.displayName, ouiaId !== undefined ? ouiaId : generatedOuiaId, ouiaSafe)} + /> + ); - const wrapWithLabel = (isLabelWrapped && !component) || component === 'label'; + const wrapWithLabel = (isLabelWrapped && !component) || component === 'label'; - const Label = wrapWithLabel ? 'span' : 'label'; - const labelRendered = label ? ( - - ) : null; + const Label = wrapWithLabel ? 'span' : 'label'; + const labelRendered = label ? ( + + ) : null; - const Component = component ?? (wrapWithLabel ? 'label' : 'div'); + const WrapperComponent = component ?? (wrapWithLabel ? 'label' : 'div'); - checkedProps.checked = checkedProps.checked === null ? false : checkedProps.checked; - return ( - - {labelPosition === 'start' ? ( - <> - {labelRendered} - {inputRendered} - - ) : ( - <> - {inputRendered} - {labelRendered} - - )} - {description && ( - - {description} - - )} - {body && {body}} - + checkedProps.checked = checkedProps.checked === null ? false : checkedProps.checked; + return ( + + {labelPosition === 'start' ? ( + <> + {labelRendered} + {inputRendered} + + ) : ( + <> + {inputRendered} + {labelRendered} + + )} + {description && ( + + {description} + + )} + {body && {body}} + + ); + }} + ); } } diff --git a/packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx b/packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx index 6375bf6e3d9..ec496f8cd21 100644 --- a/packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx +++ b/packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx @@ -6,6 +6,7 @@ import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-i import { PickOptional } from '../../helpers/typeUtils'; import { debounce } from '../../helpers/util'; import { getResizeObserver } from '../../helpers/resizeObserver'; +import { GenerateId } from '../../helpers'; import { Button } from '../Button'; export enum ExpandableSectionVariant { @@ -91,9 +92,6 @@ interface ExpandableSectionState { previousWidth: number; } -let expandableSectionContentId = 0; -let expandableSectionToggleId = 0; - const directionClassMap = { up: styles.modifiers.expandTop, down: styles.modifiers.expandBottom @@ -109,8 +107,6 @@ const setLineClamp = (lines: number, element: HTMLDivElement) => { class ExpandableSection extends Component { static displayName = 'ExpandableSection'; - private generatedContentId = `expandable-section-content-${expandableSectionContentId++}`; - private generatedToggleId = `expandable-section-toggle-${expandableSectionToggleId++}`; constructor(props: ExpandableSectionProps) { super(props); @@ -248,8 +244,6 @@ class ExpandableSection extends Component - - - ); - return ( -
+ {(genContentId) => ( + + {(genToggleId) => { + const uniqueContentId = contentId || genContentId; + const uniqueToggleId = toggleId || genToggleId; + + const expandableToggle = !isDetached && ( + + + + ); + + return ( +
+ {variant === ExpandableSectionVariant.default && expandableToggle} + + {variant === ExpandableSectionVariant.truncate && this.state.hasToggle && expandableToggle} +
+ ); + }} + )} - {...props} - > - {variant === ExpandableSectionVariant.default && expandableToggle} - - {variant === ExpandableSectionVariant.truncate && this.state.hasToggle && expandableToggle} -
+ ); } } diff --git a/packages/react-core/src/components/FormSelect/FormSelect.tsx b/packages/react-core/src/components/FormSelect/FormSelect.tsx index 275644a0c63..af2b71d42f3 100644 --- a/packages/react-core/src/components/FormSelect/FormSelect.tsx +++ b/packages/react-core/src/components/FormSelect/FormSelect.tsx @@ -4,11 +4,13 @@ import { css } from '@patternfly/react-styles'; import { PickOptional } from '../../helpers/typeUtils'; import { ValidatedOptions } from '../../helpers/constants'; import { FormControlIcon } from '../FormControl/FormControlIcon'; -import { getOUIAProps, OUIAProps, getDefaultOUIAId } from '../../helpers'; +import { getOUIAProps, OUIAProps } from '../../helpers'; +import { SSRSafeIds } from '../../helpers/SSRSafeIds/SSRSafeIds'; import CaretDownIcon from '@patternfly/react-icons/dist/esm/icons/caret-down-icon'; export interface FormSelectProps - extends Omit, 'onChange' | 'onBlur' | 'onFocus' | 'disabled'>, OUIAProps { + extends Omit, 'onChange' | 'onBlur' | 'onFocus' | 'disabled'>, + OUIAProps { /** content rendered inside the FormSelect */ children: React.ReactNode; /** additional classes added to the FormSelect control */ @@ -38,7 +40,7 @@ export interface FormSelectProps ouiaSafe?: boolean; } -class FormSelect extends Component { +class FormSelect extends Component { static displayName = 'FormSelect'; constructor(props: FormSelectProps) { super(props); @@ -46,9 +48,6 @@ class FormSelect extends Component { // eslint-disable-next-line no-console console.error('FormSelect requires either an id or aria-label to be specified'); } - this.state = { - ouiaStateId: getDefaultOUIAId(FormSelect.displayName, props.validated) - }; } static defaultProps: PickOptional = { @@ -75,33 +74,37 @@ class FormSelect extends Component { const hasStatusIcon = ['success', 'error', 'warning'].includes(validated); return ( - - - - {hasStatusIcon && } - - + + {(_, generatedOuiaId) => ( + + + + {hasStatusIcon && } + + + + - - + )} + ); } } diff --git a/packages/react-core/src/components/Menu/Menu.tsx b/packages/react-core/src/components/Menu/Menu.tsx index 672ba179037..73f7d4ac20b 100644 --- a/packages/react-core/src/components/Menu/Menu.tsx +++ b/packages/react-core/src/components/Menu/Menu.tsx @@ -2,7 +2,8 @@ import { Component, createRef, forwardRef } from 'react'; import styles from '@patternfly/react-styles/css/components/Menu/menu'; import breadcrumbStyles from '@patternfly/react-styles/css/components/Breadcrumb/breadcrumb'; import { css } from '@patternfly/react-styles'; -import { getOUIAProps, OUIAProps, getDefaultOUIAId } from '../../helpers'; +import { getOUIAProps, OUIAProps } from '../../helpers'; +import { SSRSafeIds } from '../../helpers/SSRSafeIds/SSRSafeIds'; import { MenuContext } from './MenuContext'; import type { MenuItemProps } from './MenuItem'; import { canUseDOM } from '../../helpers/util'; @@ -67,7 +68,6 @@ export interface MenuProps extends Omit, 'r } export interface MenuState { - ouiaStateId: string; transitionMoveTarget: HTMLElement; flyoutRef: React.Ref | null; currentDrilldownMenuId: string; @@ -95,7 +95,6 @@ class MenuBase extends Component { } state: MenuState = { - ouiaStateId: getDefaultOUIAId(Menu.displayName), transitionMoveTarget: null, flyoutRef: null, currentDrilldownMenuId: this.props.id @@ -264,72 +263,76 @@ class MenuBase extends Component { } = this.props; const _isMenuDrilledIn = isMenuDrilledIn || (drilledInMenus && drilledInMenus.includes(id)) || false; return ( - this.setState({ flyoutRef }), - disableHover: this.context?.disableHover ?? false, - role - }} - > - {isRootMenu && ( - ) || null} - additionalKeyHandler={this.handleExtraKeys} - createNavigableElements={this.createNavigableElements} - isActiveElement={(element: Element) => - document.activeElement.closest('li') === element || // if element is a basic MenuItem - document.activeElement.parentElement === element || - document.activeElement.closest(`.${styles.menuSearch}`) === element || // if element is a MenuSearch - (document.activeElement.closest('ol') && document.activeElement.closest('ol').firstChild === element) - } - getFocusableElement={(navigableElement: Element) => - (navigableElement?.tagName === 'DIV' && navigableElement.querySelector('input')) || // for MenuSearchInput - ((navigableElement.firstChild as Element)?.tagName === 'LABEL' && - navigableElement.querySelector('input')) || // for MenuItem checkboxes - ((navigableElement.firstChild as Element)?.tagName === 'DIV' && - navigableElement.querySelector('a, button, input')) || // For aria-disabled element that is rendered inside a div with "display: contents" styling - (navigableElement.firstChild as Element) - } - noHorizontalArrowHandling={ - document.activeElement && - (document.activeElement.classList.contains(breadcrumbStyles.breadcrumbLink) || - document.activeElement.tagName === 'INPUT') - } - noEnterHandling - noSpaceHandling - /> + + {(_, generatedOuiaId) => ( + this.setState({ flyoutRef }), + disableHover: this.context?.disableHover ?? false, + role + }} + > + {isRootMenu && ( + ) || null} + additionalKeyHandler={this.handleExtraKeys} + createNavigableElements={this.createNavigableElements} + isActiveElement={(element: Element) => + document.activeElement.closest('li') === element || // if element is a basic MenuItem + document.activeElement.parentElement === element || + document.activeElement.closest(`.${styles.menuSearch}`) === element || // if element is a MenuSearch + (document.activeElement.closest('ol') && document.activeElement.closest('ol').firstChild === element) + } + getFocusableElement={(navigableElement: Element) => + (navigableElement?.tagName === 'DIV' && navigableElement.querySelector('input')) || // for MenuSearchInput + ((navigableElement.firstChild as Element)?.tagName === 'LABEL' && + navigableElement.querySelector('input')) || // for MenuItem checkboxes + ((navigableElement.firstChild as Element)?.tagName === 'DIV' && + navigableElement.querySelector('a, button, input')) || // For aria-disabled element that is rendered inside a div with "display: contents" styling + (navigableElement.firstChild as Element) + } + noHorizontalArrowHandling={ + document.activeElement && + (document.activeElement.classList.contains(breadcrumbStyles.breadcrumbLink) || + document.activeElement.tagName === 'INPUT') + } + noEnterHandling + noSpaceHandling + /> + )} +
+ {children} +
+ )} -
- {children} -
- + ); } } diff --git a/packages/react-core/src/components/MenuToggle/MenuToggle.tsx b/packages/react-core/src/components/MenuToggle/MenuToggle.tsx index e6b6c7ec1e5..b3c2c7a376d 100644 --- a/packages/react-core/src/components/MenuToggle/MenuToggle.tsx +++ b/packages/react-core/src/components/MenuToggle/MenuToggle.tsx @@ -7,7 +7,8 @@ import CogIcon from '@patternfly/react-icons/dist/esm/icons/cog-icon'; import CheckCircleIcon from '@patternfly/react-icons/dist/esm/icons/check-circle-icon'; import ExclamationCircleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon'; import ExclamationTriangleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-triangle-icon'; -import { OUIAProps, getDefaultOUIAId, getOUIAProps } from '../../helpers'; +import { OUIAProps, getOUIAProps } from '../../helpers'; +import { SSRSafeIds } from '../../helpers/SSRSafeIds/SSRSafeIds'; export enum MenuToggleStatus { success = 'success', @@ -23,8 +24,7 @@ export enum MenuToggleSize { export type MenuToggleElement = HTMLDivElement | HTMLButtonElement; export interface MenuToggleProps - extends - Omit< + extends Omit< React.DetailedHTMLProps< React.ButtonHTMLAttributes & React.HTMLAttributes, MenuToggleElement @@ -74,11 +74,7 @@ export interface MenuToggleProps ouiaSafe?: boolean; } -interface MenuToggleState { - ouiaStateId: string; -} - -class MenuToggleBase extends Component { +class MenuToggleBase extends Component { displayName = 'MenuToggleBase'; static defaultProps: MenuToggleProps = { className: '', @@ -92,10 +88,6 @@ class MenuToggleBase extends Component { ouiaSafe: true }; - state: MenuToggleState = { - ouiaStateId: getDefaultOUIAId(MenuToggle.displayName, this.props.variant) - }; - render() { const { children, @@ -125,121 +117,132 @@ class MenuToggleBase extends Component { const isPlainText = variant === 'plainText'; const isTypeahead = variant === 'typeahead'; - const ouiaProps = getOUIAProps(MenuToggle.displayName, ouiaId ?? this.state.ouiaStateId, ouiaSafe); - - let _statusIcon = statusIcon; - if (!statusIcon) { - switch (status) { - case MenuToggleStatus.success: - _statusIcon = ; - break; - case MenuToggleStatus.warning: - _statusIcon = ; - break; - case MenuToggleStatus.danger: - _statusIcon = ; - break; - } - } - - const toggleControls = ( - - {status !== undefined && {_statusIcon}} - - - - - ); - - const content = ( - <> - {(icon || isSettings) && {isSettings ? : icon}} - {isTypeahead ? children : children && {children}} - {isValidElement(badge) && {badge}} - {isTypeahead ? ( - - ) : ( - !isPlain && toggleControls - )} - - ); - - const commonStyles = css( - styles.menuToggle, - isExpanded && styles.modifiers.expanded, - variant === 'primary' && styles.modifiers.primary, - variant === 'secondary' && styles.modifiers.secondary, - status && styles.modifiers[status], - (isPlain || isPlainText) && styles.modifiers.plain, - isPlainText && 'pf-m-text', - isFullHeight && styles.modifiers.fullHeight, - isFullWidth && styles.modifiers.fullWidth, - isDisabled && styles.modifiers.disabled, - isPlaceholder && styles.modifiers.placeholder, - isSettings && styles.modifiers.settings, - size === MenuToggleSize.sm && styles.modifiers.small, - className - ); - - const componentProps = { - children: content, - ...(isDisabled && { disabled: true }), - ...otherProps - }; - - if (isTypeahead) { - return ( -
} - className={css(commonStyles, styles.modifiers.typeahead)} - {...componentProps} - /> - ); - } - - if (splitButtonItems) { - return ( -
} className={css(commonStyles, styles.modifiers.splitButton)}> - {splitButtonItems} - -
- ); - } - return ( - + ) : ( + !isPlain && toggleControls + )} + + ); + + const commonStyles = css( + styles.menuToggle, + isExpanded && styles.modifiers.expanded, + variant === 'primary' && styles.modifiers.primary, + variant === 'secondary' && styles.modifiers.secondary, + status && styles.modifiers[status], + (isPlain || isPlainText) && styles.modifiers.plain, + isPlainText && 'pf-m-text', + isFullHeight && styles.modifiers.fullHeight, + isFullWidth && styles.modifiers.fullWidth, + isDisabled && styles.modifiers.disabled, + isPlaceholder && styles.modifiers.placeholder, + isSettings && styles.modifiers.settings, + size === MenuToggleSize.sm && styles.modifiers.small, + className + ); + + const componentProps = { + children: content, + ...(isDisabled && { disabled: true }), + ...otherProps + }; + + if (isTypeahead) { + return ( +
} + className={css(commonStyles, styles.modifiers.typeahead)} + {...componentProps} + /> + ) as React.ReactElement; + } + + if (splitButtonItems) { + return ( +
} + className={css(commonStyles, styles.modifiers.splitButton)} + > + {splitButtonItems} + +
+ ) as React.ReactElement; + } + + return ( + - )} - - - - )} - + + ); + }} + ); } } diff --git a/packages/react-core/src/components/Progress/Progress.tsx b/packages/react-core/src/components/Progress/Progress.tsx index 64024a99b95..2f5ee59d2dd 100644 --- a/packages/react-core/src/components/Progress/Progress.tsx +++ b/packages/react-core/src/components/Progress/Progress.tsx @@ -3,8 +3,7 @@ import styles from '@patternfly/react-styles/css/components/Progress/progress'; import { css } from '@patternfly/react-styles'; import { ProgressContainer, ProgressMeasureLocation } from './ProgressContainer'; import { AriaProps } from './ProgressBar'; - -let progressId = 0; +import { GenerateId } from '../../helpers'; export enum ProgressSize { sm = 'sm', @@ -74,14 +73,11 @@ class Progress extends Component { 'aria-describedby': null as string }; - id = this.props.id || `pf-progress-${progressId++}`; - render() { const { - /* eslint-disable @typescript-eslint/no-unused-vars */ - id, + id: idProp, size, - /* eslint-enable @typescript-eslint/no-unused-vars */ + className, value, title, @@ -101,28 +97,6 @@ class Progress extends Component { ...props } = this.props; - const progressBarAriaProps: AriaProps = { - 'aria-valuemin': min, - 'aria-valuenow': value, - 'aria-valuemax': max - }; - - if (title || ariaLabelledBy) { - progressBarAriaProps['aria-labelledby'] = title ? `${this.id}-description` : ariaLabelledBy; - } - - if (ariaLabel) { - progressBarAriaProps['aria-label'] = ariaLabel; - } - - if (ariaDescribedBy) { - progressBarAriaProps['aria-describedby'] = ariaDescribedBy; - } - - if (valueText) { - progressBarAriaProps['aria-valuetext'] = valueText; - } - if (!title && !ariaLabelledBy && !ariaLabel) { /* eslint-disable no-console */ console.warn( @@ -130,34 +104,67 @@ class Progress extends Component { ); } - const scaledValue = Math.min(100, Math.max(0, Math.floor(((value - min) / (max - min)) * 100))) || 0; return ( -
- -
+ + {(generatedId) => { + const id = idProp || generatedId; + + const progressBarAriaProps: AriaProps = { + 'aria-valuemin': min, + 'aria-valuenow': value, + 'aria-valuemax': max + }; + + if (title || ariaLabelledBy) { + progressBarAriaProps['aria-labelledby'] = title ? `${id}-description` : ariaLabelledBy; + } + + if (ariaLabel) { + progressBarAriaProps['aria-label'] = ariaLabel; + } + + if (ariaDescribedBy) { + progressBarAriaProps['aria-describedby'] = ariaDescribedBy; + } + + if (valueText) { + progressBarAriaProps['aria-valuetext'] = valueText; + } + + const scaledValue = Math.min(100, Math.max(0, Math.floor(((value - min) / (max - min)) * 100))) || 0; + return ( +
+ +
+ ); + }} + ); } } diff --git a/packages/react-core/src/components/Radio/Radio.tsx b/packages/react-core/src/components/Radio/Radio.tsx index ec4886c103f..d5a4d65c07e 100644 --- a/packages/react-core/src/components/Radio/Radio.tsx +++ b/packages/react-core/src/components/Radio/Radio.tsx @@ -2,9 +2,8 @@ import { Component } from 'react'; import styles from '@patternfly/react-styles/css/components/Radio/radio'; import { css } from '@patternfly/react-styles'; import { PickOptional } from '../../helpers/typeUtils'; -import { getOUIAProps, OUIAProps, getDefaultOUIAId } from '../../helpers'; - -let radioDescriptionId = 0; +import { getOUIAProps, OUIAProps } from '../../helpers'; +import { SSRSafeIds } from '../../helpers/SSRSafeIds/SSRSafeIds'; export interface RadioProps extends Omit, 'disabled' | 'label' | 'onChange' | 'type'>, @@ -51,7 +50,7 @@ export interface RadioProps ouiaSafe?: boolean; } -class Radio extends Component { +class Radio extends Component { static displayName = 'Radio'; static defaultProps: PickOptional = { className: '', @@ -66,10 +65,6 @@ class Radio extends Component) => { @@ -104,67 +99,72 @@ class Radio extends Component + {(descriptionId, generatedOuiaId) => { + let ariaDescribedByValue: string | undefined; + if (ariaDescribedBy !== undefined) { + ariaDescribedByValue = ariaDescribedBy === '' ? undefined : ariaDescribedBy; + } else if (description) { + ariaDescribedByValue = descriptionId; + } - const inputRendered = ( - - ); + const inputRendered = ( + + ); - const wrapWithLabel = (isLabelWrapped && !component) || component === 'label'; + const wrapWithLabel = (isLabelWrapped && !component) || component === 'label'; - const Label = wrapWithLabel ? 'span' : 'label'; - const labelRendered = label ? ( - - ) : null; + const Label = wrapWithLabel ? 'span' : 'label'; + const labelRendered = label ? ( + + ) : null; - const Component = component ?? (wrapWithLabel ? 'label' : 'div'); + const WrapperComponent = component ?? (wrapWithLabel ? 'label' : 'div'); - return ( - - {labelPosition === 'start' ? ( - <> - {labelRendered} - {inputRendered} - - ) : ( - <> - {inputRendered} - {labelRendered} - - )} - {description && ( - - {description} - - )} - {body && {body}} - + return ( + + {labelPosition === 'start' ? ( + <> + {labelRendered} + {inputRendered} + + ) : ( + <> + {inputRendered} + {labelRendered} + + )} + {description && ( + + {description} + + )} + {body && {body}} + + ); + }} + ); } } diff --git a/packages/react-core/src/components/Select/Select.tsx b/packages/react-core/src/components/Select/Select.tsx index 50315750862..172ed6fe1dd 100644 --- a/packages/react-core/src/components/Select/Select.tsx +++ b/packages/react-core/src/components/Select/Select.tsx @@ -2,7 +2,8 @@ import { forwardRef, useEffect, useRef } from 'react'; import { css } from '@patternfly/react-styles'; import { Menu, MenuContent, MenuProps } from '../Menu'; import { Popper, PopperOptions } from '../../helpers/Popper/Popper'; -import { getOUIAProps, OUIAProps, getDefaultOUIAId, onToggleArrowKeydownDefault } from '../../helpers'; +import { getOUIAProps, OUIAProps, onToggleArrowKeydownDefault } from '../../helpers'; +import { useOUIAId } from '../../helpers/OUIA/ouia'; import type { SelectOptionProps } from './SelectOption'; /** @deprecated Use PopperOptions instead */ @@ -92,6 +93,7 @@ const SelectBase: React.FunctionComponent = ({ focusTimeoutDelay = 0, ...props }: SelectProps & OUIAProps) => { + const generatedOuiaId = useOUIAId(Select.displayName, props.ouiaId); const localMenuRef = useRef(undefined); const localToggleRef = useRef(undefined); @@ -179,11 +181,7 @@ const SelectBase: React.FunctionComponent = ({ isPlain={isPlain} selected={selected} isScrollable={isScrollable ?? (menuHeight !== undefined || maxMenuHeight !== undefined)} - {...getOUIAProps( - Select.displayName, - props.ouiaId !== undefined ? props.ouiaId : getDefaultOUIAId(Select.displayName), - props.ouiaSafe !== undefined ? props.ouiaSafe : true - )} + {...getOUIAProps(Select.displayName, generatedOuiaId, props.ouiaSafe !== undefined ? props.ouiaSafe : true)} {...props} > diff --git a/packages/react-core/src/components/Switch/Switch.tsx b/packages/react-core/src/components/Switch/Switch.tsx index dbb7a95276b..4dea285f2eb 100644 --- a/packages/react-core/src/components/Switch/Switch.tsx +++ b/packages/react-core/src/components/Switch/Switch.tsx @@ -2,9 +2,8 @@ import { Component, Fragment } from 'react'; import styles from '@patternfly/react-styles/css/components/Switch/switch'; import { css } from '@patternfly/react-styles'; import CheckIcon from '@patternfly/react-icons/dist/esm/icons/check-icon'; -import { getOUIAProps, OUIAProps, getDefaultOUIAId } from '../../helpers'; - -let switchId = 0; +import { getOUIAProps, OUIAProps } from '../../helpers'; +import { SSRSafeIds } from '../../helpers/SSRSafeIds/SSRSafeIds'; export interface SwitchProps extends Omit, 'type' | 'onChange' | 'disabled' | 'label'>, @@ -41,9 +40,8 @@ export interface SwitchProps ouiaSafe?: boolean; } -class Switch extends Component { +class Switch extends Component { static displayName = 'Switch'; - id: string; static defaultProps: SwitchProps = { isChecked: true, @@ -62,17 +60,11 @@ class Switch extends Component 'Switch: Switch requires at least one of label, aria-labelledby, or aria-label props to be specified' ); } - - this.id = props.id || `pf-switch-${switchId++}`; - this.state = { - ouiaStateId: getDefaultOUIAId(Switch.displayName) - }; } render() { const { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - id, + id: idProp, className, label, isChecked, @@ -88,54 +80,61 @@ class Switch extends Component ...props } = this.props; - const hasAccessibleName = label || ariaLabel || ariaLabelledBy; - const isAriaLabelledBy = hasAccessibleName && (!ariaLabel || ariaLabelledBy); - const useDefaultAriaLabelledBy = !ariaLabelledBy && !ariaLabel; - const ariaLabelledByIds = ariaLabelledBy ?? `${this.id}-label`; - return ( - + + ); + }} + ); } } diff --git a/packages/react-core/src/components/Tabs/Tabs.tsx b/packages/react-core/src/components/Tabs/Tabs.tsx index 520f0f7d157..ac0a4a0be41 100644 --- a/packages/react-core/src/components/Tabs/Tabs.tsx +++ b/packages/react-core/src/components/Tabs/Tabs.tsx @@ -6,7 +6,6 @@ import AngleLeftIcon from '@patternfly/react-icons/dist/esm/icons/angle-left-ico import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon'; import PlusIcon from '@patternfly/react-icons/dist/esm/icons/plus-icon'; import { - getUniqueId, isElementInView, formatBreakpointMods, getLanguageDirection, @@ -17,7 +16,8 @@ import { TabProps } from './Tab'; import { TabsContextProvider } from './TabsContext'; import { OverflowTab, HorizontalOverflowPopperProps } from './OverflowTab'; import { Button } from '../Button'; -import { getOUIAProps, OUIAProps, getDefaultOUIAId, canUseDOM } from '../../helpers'; +import { getOUIAProps, OUIAProps, canUseDOM } from '../../helpers'; +import { SSRSafeIds } from '../../helpers/SSRSafeIds/SSRSafeIds'; import { GenerateId } from '../../helpers/GenerateId/GenerateId'; import linkAccentLength from '@patternfly/react-tokens/dist/esm/c_tabs_link_accent_length'; import linkAccentStart from '@patternfly/react-tokens/dist/esm/c_tabs_link_accent_start'; @@ -41,7 +41,8 @@ type TabElement = React.ReactElement, 'onSelect' | 'onToggle'>, OUIAProps { + extends Omit, 'onSelect' | 'onToggle'>, + OUIAProps { /** Content rendered inside the tabs component. Only `Tab` components or expressions resulting in a falsy value are allowed here. */ children: TabsChild | TabsChild[]; /** Additional classes added to the tabs */ @@ -153,7 +154,6 @@ interface TabsState { shownKeys: (string | number)[]; uncontrolledActiveKey: number | string; uncontrolledIsExpandedLocal: boolean; - ouiaStateId: string; overflowingTabCount: number; isInitializingAccent: boolean; currentLinkAccentLength: string; @@ -176,7 +176,6 @@ class Tabs extends Component { shownKeys: this.props.defaultActiveKey !== undefined ? [this.props.defaultActiveKey] : [this.props.activeKey], // only for mountOnEnter case uncontrolledActiveKey: this.props.defaultActiveKey, uncontrolledIsExpandedLocal: this.props.defaultIsExpanded, - ouiaStateId: getDefaultOUIAId(Tabs.displayName), overflowingTabCount: 0, isInitializingAccent: true, currentLinkAccentLength: linkAccentLength.value, @@ -532,7 +531,6 @@ class Tabs extends Component { const filteredChildrenOverflowing = filteredChildren.slice(filteredChildren.length - overflowingTabCount); const overflowingTabProps = filteredChildrenOverflowing.map((child: React.ReactElement) => child.props); - const uniqueId = id || getUniqueId(); const defaultComponent = isNav && !component ? 'nav' : 'div'; const Component: any = component !== undefined ? component : defaultComponent; const localActiveKey = defaultActiveKey !== undefined ? uncontrolledActiveKey : activeKey; @@ -551,138 +549,145 @@ class Tabs extends Component { const overflowObjectProps = typeof isOverflowHorizontal === 'object' ? { ...isOverflowHorizontal } : {}; return ( - this.handleTabClick(...args), - handleTabClose: onClose - }} - > - - {expandable && isVertical && ( - - {(randomId) => ( -
-
+ + {(generatedId, generatedOuiaId) => { + const uniqueId = id || generatedId; + return ( + this.handleTabClick(...args), + handleTabClose: onClose + }} + > + + {expandable && isVertical && ( + + {(randomId) => ( +
+
+ +
+
+ )} + + )} + {renderScrollButtons && ( +
+
+ )} +
    + {isOverflowHorizontal ? filteredChildrenWithoutOverflow : filteredChildren} + {hasOverflowTab && } +
+ {renderScrollButtons && ( +
+ icon={} + />
-
- )} - - )} - {renderScrollButtons && ( -
-
- )} -
    - {isOverflowHorizontal ? filteredChildrenWithoutOverflow : filteredChildren} - {hasOverflowTab && } -
- {renderScrollButtons && ( -
-
- )} - {onAdd !== undefined && ( - - @@ -55,7 +41,7 @@ exports[`AboutModalBoxCloseButton Test close button aria label 1`] = ` @@ -102,7 +74,7 @@ exports[`AboutModalBoxCloseButton Test onclose 1`] = ` diff --git a/packages/react-core/src/components/AboutModal/__tests__/__snapshots__/AboutModalBoxHeader.test.tsx.snap b/packages/react-core/src/components/AboutModal/__tests__/__snapshots__/AboutModalBoxHeader.test.tsx.snap index 5a42a7d7030..aac3b538c69 100644 --- a/packages/react-core/src/components/AboutModal/__tests__/__snapshots__/AboutModalBoxHeader.test.tsx.snap +++ b/packages/react-core/src/components/AboutModal/__tests__/__snapshots__/AboutModalBoxHeader.test.tsx.snap @@ -7,7 +7,7 @@ exports[`AboutModalBoxHeader Test 1`] = ` >

, HTMLButtonElement> { +export interface AccordionToggleProps extends React.DetailedHTMLProps< + React.ButtonHTMLAttributes, + HTMLButtonElement +> { /** Content rendered inside the Accordion toggle */ children?: React.ReactNode; /** Additional classes added to the Accordion Toggle */ diff --git a/packages/react-core/src/components/Avatar/Avatar.tsx b/packages/react-core/src/components/Avatar/Avatar.tsx index f7108e8c33e..1f1c2f2cde5 100644 --- a/packages/react-core/src/components/Avatar/Avatar.tsx +++ b/packages/react-core/src/components/Avatar/Avatar.tsx @@ -1,8 +1,10 @@ import styles from '@patternfly/react-styles/css/components/Avatar/avatar'; import { css } from '@patternfly/react-styles'; -export interface AvatarProps - extends React.DetailedHTMLProps, HTMLImageElement> { +export interface AvatarProps extends React.DetailedHTMLProps< + React.ImgHTMLAttributes, + HTMLImageElement +> { /** Additional classes added to the avatar. */ className?: string; /** Attribute that specifies the URL of the image for the avatar. */ diff --git a/packages/react-core/src/components/Brand/Brand.tsx b/packages/react-core/src/components/Brand/Brand.tsx index 89186f369c6..02c3695a9d4 100644 --- a/packages/react-core/src/components/Brand/Brand.tsx +++ b/packages/react-core/src/components/Brand/Brand.tsx @@ -4,8 +4,10 @@ import { setBreakpointCssVars } from '../../helpers'; import cssBrandHeight from '@patternfly/react-tokens/dist/esm/c_brand_Height'; import cssBrandWidth from '@patternfly/react-tokens/dist/esm/c_brand_Width'; -export interface BrandProps - extends React.DetailedHTMLProps, HTMLImageElement> { +export interface BrandProps extends React.DetailedHTMLProps< + React.ImgHTMLAttributes, + HTMLImageElement +> { /** Transforms the Brand into a element from an element. Container for child elements. */ children?: React.ReactNode; /** Additional classes added to the either type of Brand. */ diff --git a/packages/react-core/src/components/Checkbox/Checkbox.tsx b/packages/react-core/src/components/Checkbox/Checkbox.tsx index 867d9561f75..fe5bdc3bcf0 100644 --- a/packages/react-core/src/components/Checkbox/Checkbox.tsx +++ b/packages/react-core/src/components/Checkbox/Checkbox.tsx @@ -7,8 +7,7 @@ import { SSRSafeIds } from '../../helpers/SSRSafeIds/SSRSafeIds'; import { ASTERISK } from '../../helpers/htmlConstants'; export interface CheckboxProps - extends Omit, 'type' | 'onChange' | 'disabled' | 'label'>, - OUIAProps { + extends Omit, 'type' | 'onChange' | 'disabled' | 'label'>, OUIAProps { /** Additional classes added to the checkbox wrapper. This wrapper will be div element by default. It will be a label element if * isLabelWrapped is true, or it can be overridden by any element specified in the component prop. */ diff --git a/packages/react-core/src/components/ClipboardCopy/ClipboardCopyButton.tsx b/packages/react-core/src/components/ClipboardCopy/ClipboardCopyButton.tsx index 0ebc63e98b9..863a59120ab 100644 --- a/packages/react-core/src/components/ClipboardCopy/ClipboardCopyButton.tsx +++ b/packages/react-core/src/components/ClipboardCopy/ClipboardCopyButton.tsx @@ -3,8 +3,10 @@ import CopyIcon from '@patternfly/react-icons/dist/esm/icons/copy-icon'; import { Button } from '../Button'; import { Tooltip, TooltipPosition } from '../Tooltip'; -export interface ClipboardCopyButtonProps - extends Omit, HTMLButtonElement>, 'ref'> { +export interface ClipboardCopyButtonProps extends Omit< + React.DetailedHTMLProps, HTMLButtonElement>, + 'ref' +> { /** Callback for the copy when the button is clicked */ onClick: (event: React.MouseEvent) => void; /** Content of the copy button */ diff --git a/packages/react-core/src/components/ClipboardCopy/ClipboardCopyToggle.tsx b/packages/react-core/src/components/ClipboardCopy/ClipboardCopyToggle.tsx index e0a29c9537e..0eb90a65dc0 100644 --- a/packages/react-core/src/components/ClipboardCopy/ClipboardCopyToggle.tsx +++ b/packages/react-core/src/components/ClipboardCopy/ClipboardCopyToggle.tsx @@ -3,8 +3,10 @@ import { css } from '@patternfly/react-styles'; import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon'; import { Button } from '../Button'; -export interface ClipboardCopyToggleProps - extends Omit, HTMLButtonElement>, 'ref'> { +export interface ClipboardCopyToggleProps extends Omit< + React.DetailedHTMLProps, HTMLButtonElement>, + 'ref' +> { onClick: (event: React.MouseEvent) => void; id: string; contentId: string; diff --git a/packages/react-core/src/components/DatePicker/DatePicker.tsx b/packages/react-core/src/components/DatePicker/DatePicker.tsx index 2d780689627..652310fed6f 100644 --- a/packages/react-core/src/components/DatePicker/DatePicker.tsx +++ b/packages/react-core/src/components/DatePicker/DatePicker.tsx @@ -24,7 +24,8 @@ export interface DatePickerRequiredObject { /** The main date picker component. */ export interface DatePickerProps - extends CalendarFormat, + extends + CalendarFormat, Omit, 'onChange' | 'onFocus' | 'onBlur' | 'disabled' | 'ref'> { /** The container to append the menu to. Defaults to 'inline'. * If your menu is being cut off you can append it to an element higher up the DOM tree. diff --git a/packages/react-core/src/components/DualListSelector/__tests__/__snapshots__/DualListSelector.test.tsx.snap b/packages/react-core/src/components/DualListSelector/__tests__/__snapshots__/DualListSelector.test.tsx.snap index f9529ee933f..57cafb41b30 100644 --- a/packages/react-core/src/components/DualListSelector/__tests__/__snapshots__/DualListSelector.test.tsx.snap +++ b/packages/react-core/src/components/DualListSelector/__tests__/__snapshots__/DualListSelector.test.tsx.snap @@ -90,24 +90,10 @@ exports[`DualListSelector with search inputs 1`] = ` fill="currentColor" height="1em" role="img" + viewBox="0 0 " width="1em" > - - - - - - + - - - - - - + @@ -76,7 +62,7 @@ exports[`ExpandableSection 1`] = `

@@ -58,7 +44,7 @@ exports[`Switch no label switch is not checked 1`] = `
@@ -111,7 +83,7 @@ exports[`Switch switch is checked 1`] = `
@@ -196,7 +154,7 @@ exports[`Switch switch is not checked 1`] = `
@@ -279,7 +223,7 @@ exports[`Switch switch with only label is checked 1`] = `
+
+ Tab 1 section +
+ + +`; + exports[`should render accessible tabs 1`] = `