Skip to content

Commit b69f6fb

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

File tree

27 files changed

+1121
-1084
lines changed

27 files changed

+1121
-1084
lines changed

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

Lines changed: 73 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@ import { Component } from 'react';
22
import styles from '@patternfly/react-styles/css/components/Check/check';
33
import { css } from '@patternfly/react-styles';
44
import { PickOptional } from '../../helpers/typeUtils';
5-
import { getDefaultOUIAId, getOUIAProps, OUIAProps } from '../../helpers';
5+
import { getOUIAProps, OUIAProps } from '../../helpers';
6+
import { SSRSafeIds } from '../../helpers/SSRSafeIds/SSRSafeIds';
67
import { ASTERISK } from '../../helpers/htmlConstants';
78

8-
let checkboxDescriptionId = 0;
9-
109
export interface CheckboxProps
1110
extends Omit<React.HTMLProps<HTMLInputElement>, 'type' | 'onChange' | 'disabled' | 'label'>,
1211
OUIAProps {
@@ -54,12 +53,7 @@ export interface CheckboxProps
5453
// tslint:disable-next-line:no-empty
5554
const defaultOnChange = () => {};
5655

57-
interface CheckboxState {
58-
ouiaStateId: string;
59-
descriptionId: string;
60-
}
61-
62-
class Checkbox extends Component<CheckboxProps, CheckboxState> {
56+
class Checkbox extends Component<CheckboxProps> {
6357
static displayName = 'Checkbox';
6458
static defaultProps: PickOptional<CheckboxProps> = {
6559
className: '',
@@ -72,14 +66,6 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
7266
ouiaSafe: true
7367
};
7468

75-
constructor(props: any) {
76-
super(props);
77-
this.state = {
78-
ouiaStateId: getDefaultOUIAId(Checkbox.displayName),
79-
descriptionId: `pf-checkbox-description-${checkboxDescriptionId++}`
80-
};
81-
}
82-
8369
private handleChange = (event: React.FormEvent<HTMLInputElement>): void => {
8470
this.props.onChange(event, event.currentTarget.checked);
8571
};
@@ -122,76 +108,81 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
122108
checkedProps.defaultChecked = defaultChecked;
123109
}
124110

125-
// Handle aria-describedby logic
126-
let ariaDescribedByValue: string | undefined;
127-
if (ariaDescribedBy !== undefined) {
128-
ariaDescribedByValue = ariaDescribedBy === '' ? undefined : ariaDescribedBy;
129-
} else if (description) {
130-
ariaDescribedByValue = this.state.descriptionId;
131-
}
111+
return (
112+
<SSRSafeIds prefix="pf-checkbox-description-" ouiaComponentType="Checkbox">
113+
{(descriptionId, generatedOuiaId) => {
114+
let ariaDescribedByValue: string | undefined;
115+
if (ariaDescribedBy !== undefined) {
116+
ariaDescribedByValue = ariaDescribedBy === '' ? undefined : ariaDescribedBy;
117+
} else if (description) {
118+
ariaDescribedByValue = descriptionId;
119+
}
132120

133-
const inputRendered = (
134-
<input
135-
{...props}
136-
className={css(styles.checkInput, inputClassName)}
137-
type="checkbox"
138-
onChange={this.handleChange}
139-
aria-invalid={!isValid}
140-
aria-label={ariaLabel}
141-
aria-describedby={ariaDescribedByValue}
142-
disabled={isDisabled}
143-
required={isRequired}
144-
ref={(elem) => {
145-
elem && (elem.indeterminate = isChecked === null);
146-
}}
147-
{...checkedProps}
148-
{...getOUIAProps(Checkbox.displayName, ouiaId !== undefined ? ouiaId : this.state.ouiaStateId, ouiaSafe)}
149-
/>
150-
);
121+
const inputRendered = (
122+
<input
123+
{...props}
124+
className={css(styles.checkInput, inputClassName)}
125+
type="checkbox"
126+
onChange={this.handleChange}
127+
aria-invalid={!isValid}
128+
aria-label={ariaLabel}
129+
aria-describedby={ariaDescribedByValue}
130+
disabled={isDisabled}
131+
required={isRequired}
132+
ref={(elem) => {
133+
elem && (elem.indeterminate = isChecked === null);
134+
}}
135+
{...checkedProps}
136+
{...getOUIAProps(Checkbox.displayName, ouiaId !== undefined ? ouiaId : generatedOuiaId, ouiaSafe)}
137+
/>
138+
);
151139

152-
const wrapWithLabel = (isLabelWrapped && !component) || component === 'label';
140+
const wrapWithLabel = (isLabelWrapped && !component) || component === 'label';
153141

154-
const Label = wrapWithLabel ? 'span' : 'label';
155-
const labelRendered = label ? (
156-
<Label
157-
className={css(styles.checkLabel, isDisabled && styles.modifiers.disabled)}
158-
htmlFor={!wrapWithLabel ? props.id : undefined}
159-
>
160-
{label}
161-
{isRequired && (
162-
<span className={css(styles.checkLabelRequired)} aria-hidden="true">
163-
{ASTERISK}
164-
</span>
165-
)}
166-
</Label>
167-
) : null;
142+
const Label = wrapWithLabel ? 'span' : 'label';
143+
const labelRendered = label ? (
144+
<Label
145+
className={css(styles.checkLabel, isDisabled && styles.modifiers.disabled)}
146+
htmlFor={!wrapWithLabel ? props.id : undefined}
147+
>
148+
{label}
149+
{isRequired && (
150+
<span className={css(styles.checkLabelRequired)} aria-hidden="true">
151+
{ASTERISK}
152+
</span>
153+
)}
154+
</Label>
155+
) : null;
168156

169-
const Component = component ?? (wrapWithLabel ? 'label' : 'div');
157+
const WrapperComponent = component ?? (wrapWithLabel ? 'label' : 'div');
170158

171-
checkedProps.checked = checkedProps.checked === null ? false : checkedProps.checked;
172-
return (
173-
<Component
174-
className={css(styles.check, !label && styles.modifiers.standalone, className)}
175-
htmlFor={wrapWithLabel ? props.id : undefined}
176-
>
177-
{labelPosition === 'start' ? (
178-
<>
179-
{labelRendered}
180-
{inputRendered}
181-
</>
182-
) : (
183-
<>
184-
{inputRendered}
185-
{labelRendered}
186-
</>
187-
)}
188-
{description && (
189-
<span id={this.state.descriptionId} className={css(styles.checkDescription)}>
190-
{description}
191-
</span>
192-
)}
193-
{body && <span className={css(styles.checkBody)}>{body}</span>}
194-
</Component>
159+
checkedProps.checked = checkedProps.checked === null ? false : checkedProps.checked;
160+
return (
161+
<WrapperComponent
162+
className={css(styles.check, !label && styles.modifiers.standalone, className)}
163+
htmlFor={wrapWithLabel ? props.id : undefined}
164+
>
165+
{labelPosition === 'start' ? (
166+
<>
167+
{labelRendered}
168+
{inputRendered}
169+
</>
170+
) : (
171+
<>
172+
{inputRendered}
173+
{labelRendered}
174+
</>
175+
)}
176+
{description && (
177+
<span id={descriptionId} className={css(styles.checkDescription)}>
178+
{description}
179+
</span>
180+
)}
181+
{body && <span className={css(styles.checkBody)}>{body}</span>}
182+
</WrapperComponent>
183+
);
184+
}}
185+
</SSRSafeIds>
195186
);
196187
}
197188
}

‎packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx‎

Lines changed: 61 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-i
66
import { PickOptional } from '../../helpers/typeUtils';
77
import { debounce } from '../../helpers/util';
88
import { getResizeObserver } from '../../helpers/resizeObserver';
9+
import { GenerateId } from '../../helpers';
910
import { Button } from '../Button';
1011

1112
export enum ExpandableSectionVariant {
@@ -91,9 +92,6 @@ interface ExpandableSectionState {
9192
previousWidth: number;
9293
}
9394

94-
let expandableSectionContentId = 0;
95-
let expandableSectionToggleId = 0;
96-
9795
const directionClassMap = {
9896
up: styles.modifiers.expandTop,
9997
down: styles.modifiers.expandBottom
@@ -109,8 +107,6 @@ const setLineClamp = (lines: number, element: HTMLDivElement) => {
109107

110108
class ExpandableSection extends Component<ExpandableSectionProps, ExpandableSectionState> {
111109
static displayName = 'ExpandableSection';
112-
private generatedContentId = `expandable-section-content-${expandableSectionContentId++}`;
113-
private generatedToggleId = `expandable-section-toggle-${expandableSectionToggleId++}`;
114110

115111
constructor(props: ExpandableSectionProps) {
116112
super(props);
@@ -248,8 +244,6 @@ class ExpandableSection extends Component<ExpandableSectionProps, ExpandableSect
248244

249245
let onToggle = onToggleProp;
250246
let propOrStateIsExpanded = isExpanded;
251-
const uniqueContentId = contentId || this.generatedContentId;
252-
const uniqueToggleId = toggleId || this.generatedToggleId;
253247

254248
// uncontrolled
255249
if (isExpanded === undefined) {
@@ -270,55 +264,68 @@ class ExpandableSection extends Component<ExpandableSectionProps, ExpandableSect
270264
typeof toggleContent === 'function' ? toggleContent(propOrStateIsExpanded) : toggleContent;
271265
const ToggleWrapper = toggleWrapper as any;
272266

273-
const expandableToggle = !isDetached && (
274-
<ToggleWrapper className={`${styles.expandableSection}__toggle`}>
275-
<Button
276-
variant="link"
277-
{...(variant === ExpandableSectionVariant.truncate && { isInline: true })}
278-
aria-expanded={propOrStateIsExpanded}
279-
aria-controls={uniqueContentId}
280-
id={uniqueToggleId}
281-
onClick={(event) => onToggle(event, !propOrStateIsExpanded)}
282-
{...(variant !== ExpandableSectionVariant.truncate &&
283-
hasToggleIcon && {
284-
icon: <span className={css(styles.expandableSectionToggleIcon)}>{toggleIcon}</span>
285-
})}
286-
aria-label={toggleAriaLabel}
287-
aria-labelledby={toggleAriaLabelledBy}
288-
>
289-
{computedToggleContent || computedToggleText}
290-
</Button>
291-
</ToggleWrapper>
292-
);
293-
294267
return (
295-
<div
296-
className={css(
297-
styles.expandableSection,
298-
propOrStateIsExpanded && styles.modifiers.expanded,
299-
displaySize === 'lg' && styles.modifiers.displayLg,
300-
isWidthLimited && styles.modifiers.limitWidth,
301-
isIndented && styles.modifiers.indented,
302-
isDetached && direction && directionClassMap[direction],
303-
isDetached && direction && 'pf-m-detached',
304-
variant === ExpandableSectionVariant.truncate && styles.modifiers.truncate,
305-
className
268+
<GenerateId prefix="expandable-section-content-">
269+
{(genContentId) => (
270+
<GenerateId prefix="expandable-section-toggle-">
271+
{(genToggleId) => {
272+
const uniqueContentId = contentId || genContentId;
273+
const uniqueToggleId = toggleId || genToggleId;
274+
275+
const expandableToggle = !isDetached && (
276+
<ToggleWrapper className={`${styles.expandableSection}__toggle`}>
277+
<Button
278+
variant="link"
279+
{...(variant === ExpandableSectionVariant.truncate && { isInline: true })}
280+
aria-expanded={propOrStateIsExpanded}
281+
aria-controls={uniqueContentId}
282+
id={uniqueToggleId}
283+
onClick={(event) => onToggle(event, !propOrStateIsExpanded)}
284+
{...(variant !== ExpandableSectionVariant.truncate &&
285+
hasToggleIcon && {
286+
icon: <span className={css(styles.expandableSectionToggleIcon)}>{toggleIcon}</span>
287+
})}
288+
aria-label={toggleAriaLabel}
289+
aria-labelledby={toggleAriaLabelledBy}
290+
>
291+
{computedToggleContent || computedToggleText}
292+
</Button>
293+
</ToggleWrapper>
294+
);
295+
296+
return (
297+
<div
298+
className={css(
299+
styles.expandableSection,
300+
propOrStateIsExpanded && styles.modifiers.expanded,
301+
displaySize === 'lg' && styles.modifiers.displayLg,
302+
isWidthLimited && styles.modifiers.limitWidth,
303+
isIndented && styles.modifiers.indented,
304+
isDetached && direction && directionClassMap[direction],
305+
isDetached && direction && 'pf-m-detached',
306+
variant === ExpandableSectionVariant.truncate && styles.modifiers.truncate,
307+
className
308+
)}
309+
{...props}
310+
>
311+
{variant === ExpandableSectionVariant.default && expandableToggle}
312+
<div
313+
ref={this.expandableContentRef}
314+
className={css(styles.expandableSectionContent)}
315+
hidden={variant !== ExpandableSectionVariant.truncate && !propOrStateIsExpanded}
316+
id={uniqueContentId}
317+
aria-labelledby={uniqueToggleId}
318+
role="region"
319+
>
320+
{children}
321+
</div>
322+
{variant === ExpandableSectionVariant.truncate && this.state.hasToggle && expandableToggle}
323+
</div>
324+
);
325+
}}
326+
</GenerateId>
306327
)}
307-
{...props}
308-
>
309-
{variant === ExpandableSectionVariant.default && expandableToggle}
310-
<div
311-
ref={this.expandableContentRef}
312-
className={css(styles.expandableSectionContent)}
313-
hidden={variant !== ExpandableSectionVariant.truncate && !propOrStateIsExpanded}
314-
id={uniqueContentId}
315-
aria-labelledby={uniqueToggleId}
316-
role="region"
317-
>
318-
{children}
319-
</div>
320-
{variant === ExpandableSectionVariant.truncate && this.state.hasToggle && expandableToggle}
321-
</div>
328+
</GenerateId>
322329
);
323330
}
324331
}

0 commit comments

Comments
 (0)