Skip to content

Commit 25722a4

Browse files
committed
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.
1 parent 04dc092 commit 25722a4

File tree

37 files changed

+572
-571
lines changed

37 files changed

+572
-571
lines changed

packages/react-core/src/components/AboutModal/AboutModal.tsx

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { AboutModalBoxBrand } from './AboutModalBoxBrand';
66
import { AboutModalBoxCloseButton } from './AboutModalBoxCloseButton';
77
import { AboutModalBox } from './AboutModalBox';
88
import { Modal, ModalVariant } from '../Modal';
9-
import { GenerateId } from '../../helpers/GenerateId/GenerateId';
9+
import { useSSRSafeId } from '../../helpers';
1010

1111
export interface AboutModalProps extends React.HTMLProps<HTMLDivElement> {
1212
/** Content rendered inside the about modal */
@@ -56,6 +56,8 @@ export const AboutModal: React.FunctionComponent<AboutModalProps> = ({
5656
disableFocusTrap,
5757
...props
5858
}: AboutModalProps) => {
59+
const ariaLabelledBy = useSSRSafeId('pf-about-modal-title-');
60+
5961
if (brandImageSrc && !brandImageAlt) {
6062
// eslint-disable-next-line no-console
6163
console.error(
@@ -73,35 +75,29 @@ export const AboutModal: React.FunctionComponent<AboutModalProps> = ({
7375
return null;
7476
}
7577
return (
76-
<GenerateId prefix="pf-about-modal-title-">
77-
{(ariaLabelledBy) => (
78-
<Modal
79-
isOpen={isOpen}
80-
variant={ModalVariant.large}
81-
{...(productName && { 'aria-labelledby': ariaLabelledBy })}
82-
aria-label={ariaLabel}
83-
onEscapePress={onClose}
84-
appendTo={appendTo}
85-
disableFocusTrap={disableFocusTrap}
86-
>
87-
<AboutModalBox
88-
style={
89-
backgroundImageSrc
90-
? ({ [backgroundImage.name]: `url(${backgroundImageSrc})` } as React.CSSProperties)
91-
: {}
92-
}
93-
className={css(className)}
94-
>
95-
<AboutModalBoxBrand src={brandImageSrc} alt={brandImageAlt} />
96-
<AboutModalBoxCloseButton aria-label={closeButtonAriaLabel} onClose={onClose} />
97-
{productName && <AboutModalBoxHeader id={ariaLabelledBy} productName={productName} />}
98-
<AboutModalBoxContent trademark={trademark} hasNoContentContainer={hasNoContentContainer} {...props}>
99-
{children}
100-
</AboutModalBoxContent>
101-
</AboutModalBox>
102-
</Modal>
103-
)}
104-
</GenerateId>
78+
<Modal
79+
isOpen={isOpen}
80+
variant={ModalVariant.large}
81+
{...(productName && { 'aria-labelledby': ariaLabelledBy })}
82+
aria-label={ariaLabel}
83+
onEscapePress={onClose}
84+
appendTo={appendTo}
85+
disableFocusTrap={disableFocusTrap}
86+
>
87+
<AboutModalBox
88+
style={
89+
backgroundImageSrc ? ({ [backgroundImage.name]: `url(${backgroundImageSrc})` } as React.CSSProperties) : {}
90+
}
91+
className={css(className)}
92+
>
93+
<AboutModalBoxBrand src={brandImageSrc} alt={brandImageAlt} />
94+
<AboutModalBoxCloseButton aria-label={closeButtonAriaLabel} onClose={onClose} />
95+
{productName && <AboutModalBoxHeader id={ariaLabelledBy} productName={productName} />}
96+
<AboutModalBoxContent trademark={trademark} hasNoContentContainer={hasNoContentContainer} {...props}>
97+
{children}
98+
</AboutModalBoxContent>
99+
</AboutModalBox>
100+
</Modal>
105101
);
106102
};
107103
AboutModal.displayName = 'AboutModal';

packages/react-core/src/components/CalendarMonth/CalendarMonth.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import AngleLeftIcon from '@patternfly/react-icons/dist/esm/icons/angle-left-ico
88
import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon';
99
import { css } from '@patternfly/react-styles';
1010
import styles from '@patternfly/react-styles/css/components/CalendarMonth/calendar-month';
11-
import { getUniqueId } from '../../helpers/util';
11+
import { useSSRSafeId } from '../../helpers';
1212
import { isValidDate } from '../../helpers/datetimeUtils';
1313

1414
export enum Weekday {
@@ -170,7 +170,7 @@ export const CalendarMonth = ({
170170

171171
const [hoveredDate, setHoveredDate] = useState<Date>(undefined);
172172
const focusRef = useRef<HTMLButtonElement>(undefined);
173-
const [hiddenMonthId] = useState(getUniqueId('hidden-month-span'));
173+
const hiddenMonthId = useSSRSafeId('hidden-month-span');
174174
const [shouldFocus, setShouldFocus] = useState(false);
175175

176176
const isValidated = (date: Date) => validators.every((validator) => validator(date));

packages/react-core/src/components/Card/CardHeader.tsx

Lines changed: 113 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { Button } from '../Button';
88
import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon';
99
import { Radio } from '../Radio';
1010
import { Checkbox } from '../Checkbox';
11-
import { GenerateId } from '../../helpers/GenerateId/GenerateId';
11+
import { useSSRSafeId } from '../../helpers';
1212

1313
export interface CardHeaderActionsObject {
1414
/** Actions of the card header */
@@ -90,127 +90,127 @@ export const CardHeader: React.FunctionComponent<CardHeaderProps> = ({
9090
isToggleRightAligned,
9191
hasWrap,
9292
...props
93-
}: CardHeaderProps) => (
94-
<GenerateId>
95-
{(randomId) => (
96-
<CardContext.Consumer>
97-
{({ cardId, isClickable, isSelectable, isSelected, isDisabled: isCardDisabled }) => {
98-
const cardHeaderToggle = (
99-
<div className={css(styles.cardHeaderToggle)}>
100-
<Button
101-
variant="plain"
102-
type="button"
103-
onClick={(evt) => {
104-
onExpand(evt, cardId);
105-
}}
106-
{...toggleButtonProps}
107-
icon={
108-
<span className={css(styles.cardHeaderToggleIcon)}>
109-
<AngleRightIcon />
110-
</span>
111-
}
112-
/>
113-
</div>
93+
}: CardHeaderProps) => {
94+
const randomId = useSSRSafeId();
95+
96+
return (
97+
<CardContext.Consumer>
98+
{({ cardId, isClickable, isSelectable, isSelected, isDisabled: isCardDisabled }) => {
99+
const cardHeaderToggle = (
100+
<div className={css(styles.cardHeaderToggle)}>
101+
<Button
102+
variant="plain"
103+
type="button"
104+
onClick={(evt) => {
105+
onExpand(evt, cardId);
106+
}}
107+
{...toggleButtonProps}
108+
icon={
109+
<span className={css(styles.cardHeaderToggleIcon)}>
110+
<AngleRightIcon />
111+
</span>
112+
}
113+
/>
114+
</div>
115+
);
116+
117+
const isClickableOrSelectableOnly = (isClickable && !isSelectable) || (isSelectable && !isClickable);
118+
if (actions?.actions && isClickableOrSelectableOnly) {
119+
// eslint-disable-next-line no-console
120+
console.error(
121+
`Card: ${
122+
isClickable ? 'Clickable' : 'Selectable'
123+
} only cards should not contain any other actions. If you wish to include additional actions, use a clickable and selectable card.`
114124
);
125+
}
115126

116-
const isClickableOrSelectableOnly = (isClickable && !isSelectable) || (isSelectable && !isClickable);
117-
if (actions?.actions && isClickableOrSelectableOnly) {
118-
// eslint-disable-next-line no-console
119-
console.error(
120-
`Card: ${
121-
isClickable ? 'Clickable' : 'Selectable'
122-
} only cards should not contain any other actions. If you wish to include additional actions, use a clickable and selectable card.`
123-
);
124-
}
127+
const isClickableOnlyCard = isClickable && !isSelectable;
128+
if (
129+
(isClickableOnlyCard || isSelectable) &&
130+
!selectableActions?.selectableActionAriaLabel &&
131+
!selectableActions?.selectableActionAriaLabelledby
132+
) {
133+
// eslint-disable-next-line no-console
134+
console.error(
135+
`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.`
136+
);
137+
}
125138

126-
const isClickableOnlyCard = isClickable && !isSelectable;
127-
if (
128-
(isClickableOnlyCard || isSelectable) &&
129-
!selectableActions?.selectableActionAriaLabel &&
130-
!selectableActions?.selectableActionAriaLabelledby
131-
) {
132-
// eslint-disable-next-line no-console
133-
console.error(
134-
`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.`
135-
);
136-
}
139+
const SelectableCardInput = selectableActions?.variant === 'single' ? Radio : Checkbox;
140+
const getSelectableProps = () => ({
141+
className: css('pf-m-standalone'),
142+
inputClassName: css(selectableActions?.isHidden && 'pf-v6-screen-reader'),
143+
label: <></>,
144+
'aria-label': selectableActions.selectableActionAriaLabel,
145+
'aria-labelledby': selectableActions.selectableActionAriaLabelledby,
146+
id: selectableActions.selectableActionId ?? `card-selectable-${randomId}`,
147+
name: selectableActions.name,
148+
isDisabled: isCardDisabled,
149+
onChange: selectableActions.onChange,
150+
isChecked: selectableActions.isChecked ?? isSelected,
151+
...selectableActions.selectableActionProps
152+
});
137153

138-
const SelectableCardInput = selectableActions?.variant === 'single' ? Radio : Checkbox;
139-
const getSelectableProps = () => ({
140-
className: css('pf-m-standalone'),
141-
inputClassName: css(selectableActions?.isHidden && 'pf-v6-screen-reader'),
142-
label: <></>,
154+
const isClickableLinkCard = selectableActions?.to !== undefined;
155+
const ClickableCardComponent = isClickableLinkCard ? 'a' : 'button';
156+
const getClickableProps = () => {
157+
const isDisabledLinkCard = isCardDisabled && isClickableLinkCard;
158+
const baseProps = {
159+
className: css(
160+
'pf-v6-c-card__clickable-action',
161+
isDisabledLinkCard && styles.modifiers.disabled,
162+
selectableActions?.isHidden && 'pf-v6-screen-reader'
163+
),
164+
id: selectableActions.selectableActionId,
143165
'aria-label': selectableActions.selectableActionAriaLabel,
144166
'aria-labelledby': selectableActions.selectableActionAriaLabelledby,
145-
id: selectableActions.selectableActionId ?? `card-selectable-${randomId}`,
146-
name: selectableActions.name,
147-
isDisabled: isCardDisabled,
148-
onChange: selectableActions.onChange,
149-
isChecked: selectableActions.isChecked ?? isSelected,
150167
...selectableActions.selectableActionProps
151-
});
168+
};
152169

153-
const isClickableLinkCard = selectableActions?.to !== undefined;
154-
const ClickableCardComponent = isClickableLinkCard ? 'a' : 'button';
155-
const getClickableProps = () => {
156-
const isDisabledLinkCard = isCardDisabled && isClickableLinkCard;
157-
const baseProps = {
158-
className: css(
159-
'pf-v6-c-card__clickable-action',
160-
isDisabledLinkCard && styles.modifiers.disabled,
161-
selectableActions?.isHidden && 'pf-v6-screen-reader'
162-
),
163-
id: selectableActions.selectableActionId,
164-
'aria-label': selectableActions.selectableActionAriaLabel,
165-
'aria-labelledby': selectableActions.selectableActionAriaLabelledby,
166-
...selectableActions.selectableActionProps
170+
if (isClickableLinkCard) {
171+
return {
172+
...baseProps,
173+
href: selectableActions.to,
174+
...(isCardDisabled && { tabIndex: -1, 'aria-disabled': true }),
175+
...(selectableActions.isExternalLink && { target: '_blank' })
167176
};
177+
}
168178

169-
if (isClickableLinkCard) {
170-
return {
171-
...baseProps,
172-
href: selectableActions.to,
173-
...(isCardDisabled && { tabIndex: -1, 'aria-disabled': true }),
174-
...(selectableActions.isExternalLink && { target: '_blank' })
175-
};
176-
}
179+
return { ...baseProps, type: 'button', disabled: isCardDisabled, onClick: selectableActions.onClickAction };
180+
};
177181

178-
return { ...baseProps, type: 'button', disabled: isCardDisabled, onClick: selectableActions.onClickAction };
179-
};
180-
181-
return (
182-
<div
183-
className={css(
184-
styles.cardHeader,
185-
isToggleRightAligned && styles.modifiers.toggleRight,
186-
hasWrap && styles.modifiers.wrap,
187-
className
188-
)}
189-
id={id}
190-
{...props}
191-
>
192-
{onExpand && !isToggleRightAligned && cardHeaderToggle}
193-
{(actions || (selectableActions && (isClickable || isSelectable))) && (
194-
<CardActions
195-
className={actions?.className}
196-
hasNoOffset={actions?.hasNoOffset || selectableActions?.hasNoOffset}
197-
>
198-
{actions?.actions}
199-
{selectableActions && (isClickable || isSelectable) && (
200-
<CardSelectableActions className={selectableActions?.className}>
201-
{isSelectable && <SelectableCardInput {...getSelectableProps()} />}
202-
{isClickableOnlyCard && <ClickableCardComponent {...getClickableProps()} />}
203-
</CardSelectableActions>
204-
)}
205-
</CardActions>
206-
)}
207-
{children && <CardHeaderMain>{children}</CardHeaderMain>}
208-
{onExpand && isToggleRightAligned && cardHeaderToggle}
209-
</div>
210-
);
211-
}}
212-
</CardContext.Consumer>
213-
)}
214-
</GenerateId>
215-
);
182+
return (
183+
<div
184+
className={css(
185+
styles.cardHeader,
186+
isToggleRightAligned && styles.modifiers.toggleRight,
187+
hasWrap && styles.modifiers.wrap,
188+
className
189+
)}
190+
id={id}
191+
{...props}
192+
>
193+
{onExpand && !isToggleRightAligned && cardHeaderToggle}
194+
{(actions || (selectableActions && (isClickable || isSelectable))) && (
195+
<CardActions
196+
className={actions?.className}
197+
hasNoOffset={actions?.hasNoOffset || selectableActions?.hasNoOffset}
198+
>
199+
{actions?.actions}
200+
{selectableActions && (isClickable || isSelectable) && (
201+
<CardSelectableActions className={selectableActions?.className}>
202+
{isSelectable && <SelectableCardInput {...getSelectableProps()} />}
203+
{isClickableOnlyCard && <ClickableCardComponent {...getClickableProps()} />}
204+
</CardSelectableActions>
205+
)}
206+
</CardActions>
207+
)}
208+
{children && <CardHeaderMain>{children}</CardHeaderMain>}
209+
{onExpand && isToggleRightAligned && cardHeaderToggle}
210+
</div>
211+
);
212+
}}
213+
</CardContext.Consumer>
214+
);
215+
};
216216
CardHeader.displayName = 'CardHeader';

packages/react-core/src/components/Checkbox/Checkbox.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ import styles from '@patternfly/react-styles/css/components/Check/check';
33
import { css } from '@patternfly/react-styles';
44
import { PickOptional } from '../../helpers/typeUtils';
55
import { getDefaultOUIAId, getOUIAProps, OUIAProps } from '../../helpers';
6-
import { getUniqueId } from '../../helpers/util';
76
import { ASTERISK } from '../../helpers/htmlConstants';
87

8+
let checkboxDescriptionId = 0;
9+
910
export interface CheckboxProps
10-
extends Omit<React.HTMLProps<HTMLInputElement>, 'type' | 'onChange' | 'disabled' | 'label'>, OUIAProps {
11+
extends Omit<React.HTMLProps<HTMLInputElement>, 'type' | 'onChange' | 'disabled' | 'label'>,
12+
OUIAProps {
1113
/** Additional classes added to the checkbox wrapper. This wrapper will be div element by default. It will be a label element if
1214
* isLabelWrapped is true, or it can be overridden by any element specified in the component prop.
1315
*/
@@ -74,7 +76,7 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
7476
super(props);
7577
this.state = {
7678
ouiaStateId: getDefaultOUIAId(Checkbox.displayName),
77-
descriptionId: getUniqueId('pf-checkbox-description')
79+
descriptionId: `pf-checkbox-description-${checkboxDescriptionId++}`
7880
};
7981
}
8082

0 commit comments

Comments
 (0)