Skip to content

Commit

Permalink
Fix multiple class components to not regenerate IDs on rerender (#5201)
Browse files Browse the repository at this point in the history
* [EuiAccordion] Fix rerendering button ID

* [EuiComboBox] Fix regenerating ID (by adding suffix to match other IDs in combobox)

* [EuiFilePicker] Fix generating ID
+ add consistent suffix regardless of custom ID

* [EuiPopover] Fix description ID regenerating every show/hide

* [EuiSideNav] Fix regenerating IDs + prefer suffix+prefix
  • Loading branch information
Constance authored Sep 21, 2021
1 parent 8f8d0b5 commit 987d269
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 37 deletions.
4 changes: 3 additions & 1 deletion src/components/accordion/accordion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ export class EuiAccordion extends Component<
this.childContent = node;
};

generatedId = htmlIdGenerator()();

render() {
const {
children,
Expand Down Expand Up @@ -209,7 +211,7 @@ export class EuiAccordion extends Component<

let icon;
let iconButton;
const buttonId = buttonProps?.id ?? htmlIdGenerator()();
const buttonId = buttonProps?.id ?? this.generatedId;
if (extraAction && arrowDisplay === 'right') {
iconButton = (
<button
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ export class EuiComboBoxInput<T> extends Component<
}Combo box input. ${readPlaceholder} Type some text or, to display a list of choices, press Down Arrow. ` +
'To exit the list of choices, press Escape.';

removeOptionMessageId = htmlIdGenerator()();
removeOptionMessageId = rootId('removeOptionMessage');

// aria-live="assertive" will read this message aloud immediately once it enters the DOM.
// We'll render to the DOM when the input gains focus and remove it when the input loses focus.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ exports[`EuiFilePicker is rendered 1`] = `
class="euiFilePicker__wrap"
>
<input
aria-describedby="generated-id"
aria-describedby="generated-id-filePicker__prompt"
aria-label="aria-label"
class="euiFilePicker__input"
data-test-subj="test subject string"
type="file"
/>
<div
class="euiFilePicker__prompt"
id="generated-id"
id="generated-id-filePicker__prompt"
>
<span
aria-hidden="true"
Expand Down
8 changes: 3 additions & 5 deletions src/components/form/file_picker/file_picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ export class EuiFilePicker extends Component<EuiFilePickerProps> {

fileInput: HTMLInputElement | null = null;

generatedId: string = htmlIdGenerator()();

handleChange = () => {
if (!this.fileInput) return;

Expand Down Expand Up @@ -147,11 +149,7 @@ export class EuiFilePicker extends Component<EuiFilePickerProps> {
...rest
} = this.props;

let promptId: string = htmlIdGenerator()();

if (id) {
promptId = `${id}-filePicker__prompt`;
}
const promptId = `${id || this.generatedId}-filePicker__prompt`;

const isOverridingInitialPrompt = this.state.promptText != null;

Expand Down
9 changes: 3 additions & 6 deletions src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ export type PopoverAnchorPosition =
| 'rightUp'
| 'rightDown';

const generateId = htmlIdGenerator();

export interface EuiPopoverProps {
/**
* Class name passed to the direct parent of the button
Expand Down Expand Up @@ -351,6 +349,7 @@ export class EuiPopover extends Component<Props, State> {
private button: HTMLElement | null = null;
private panel: HTMLElement | null = null;
private hasSetInitialFocus: boolean = false;
private descriptionId: string = htmlIdGenerator()();

constructor(props: Props) {
super(props);
Expand Down Expand Up @@ -699,8 +698,6 @@ export class EuiPopover extends Component<Props, State> {
...rest
} = this.props;

const descriptionId = generateId();

const classes = classNames(
'euiPopover',
anchorPosition ? anchorPositionToClassNameMap[anchorPosition] : null,
Expand Down Expand Up @@ -742,10 +739,10 @@ export class EuiPopover extends Component<Props, State> {

let focusTrapScreenReaderText;
if (ownFocus) {
ariaDescribedby = descriptionId;
ariaDescribedby = this.descriptionId;
focusTrapScreenReaderText = (
<EuiScreenReaderOnly>
<p id={descriptionId}>
<p id={this.descriptionId}>
<EuiI18n
token="euiPopover.screenReaderAnnouncement"
default="You are in a dialog. To close this dialog, hit escape."
Expand Down
40 changes: 20 additions & 20 deletions src/components/side_nav/__snapshots__/side_nav.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ exports[`EuiSideNav is rendered 1`] = `
>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
/>
</nav>
`;
Expand All @@ -28,43 +28,43 @@ exports[`EuiSideNav props heading accepts more headingProps 1`] = `
</h3>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
/>
</nav>
`;

exports[`EuiSideNav props heading is hidden with screenReaderOnly 1`] = `
<nav
aria-labelledby="euiSideNavHeading_generated-id"
aria-labelledby="euiSideNav_generated-id_heading"
class="euiSideNav"
>
<h2
class="euiScreenReaderOnly"
id="euiSideNavHeading_generated-id"
id="euiSideNav_generated-id_heading"
>
Side Nav Heading
</h2>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
/>
</nav>
`;

exports[`EuiSideNav props heading is rendered 1`] = `
<nav
aria-labelledby="euiSideNavHeading_generated-id"
aria-labelledby="euiSideNav_generated-id_heading"
class="euiSideNav"
>
<h2
class="euiTitle euiTitle--xsmall euiSideNav__heading"
id="euiSideNavHeading_generated-id"
id="euiSideNav_generated-id_heading"
>
Side Nav Heading
</h2>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
/>
</nav>
`;
Expand All @@ -75,7 +75,7 @@ exports[`EuiSideNav props isOpenOnMobile defaults to false 1`] = `
>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
/>
</nav>
`;
Expand All @@ -86,7 +86,7 @@ exports[`EuiSideNav props isOpenOnMobile is rendered when specified as true 1`]
>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
/>
</nav>
`;
Expand All @@ -97,7 +97,7 @@ exports[`EuiSideNav props items is rendered 1`] = `
>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
>
<div
class="euiSideNavItem euiSideNavItem--root euiSideNavItem--hasChildItems"
Expand Down Expand Up @@ -170,7 +170,7 @@ exports[`EuiSideNav props items renders items having { forceOpen: true } in open
>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
>
<div
class="euiSideNavItem euiSideNavItem--root euiSideNavItem--hasChildItems"
Expand Down Expand Up @@ -287,7 +287,7 @@ exports[`EuiSideNav props items renders items using a specified callback 1`] = `
>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
>
<div
class="euiSideNavItem euiSideNavItem--root euiSideNavItem--hasChildItems"
Expand Down Expand Up @@ -340,7 +340,7 @@ exports[`EuiSideNav props items renders items which are links 1`] = `
>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
>
<div
class="euiSideNavItem euiSideNavItem--root euiSideNavItem--hasChildItems"
Expand Down Expand Up @@ -413,7 +413,7 @@ exports[`EuiSideNav props items renders selected item and automatically opens pa
>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
>
<div
class="euiSideNavItem euiSideNavItem--root euiSideNavItem--hasChildItems"
Expand Down Expand Up @@ -518,14 +518,14 @@ exports[`EuiSideNav props items renders selected item and automatically opens pa

exports[`EuiSideNav props mobileBreakpoints can be adjusted is rendered 1`] = `
<nav
aria-labelledby="euiSideNavHeading_generated-id"
aria-labelledby="euiSideNav_generated-id_heading"
class="euiSideNav"
>
<h2
id="euiSideNavHeading_generated-id"
id="euiSideNav_generated-id_heading"
>
<button
aria-controls="euiSideNavContent_generated-id"
aria-controls="euiSideNav_generated-id_content"
class="euiButtonEmpty euiButtonEmpty--primary euiSideNav__mobileToggle"
type="button"
>
Expand All @@ -545,7 +545,7 @@ exports[`EuiSideNav props mobileBreakpoints can be adjusted is rendered 1`] = `
</h2>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s euiSideNav__contentMobile-m euiSideNav__contentMobile-l euiSideNav__contentMobile-xl"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
/>
</nav>
`;
Expand All @@ -556,7 +556,7 @@ exports[`EuiSideNav props mobileBreakpoints can be adjusted null is rendered 1`]
>
<div
class="euiSideNav__content euiSideNav__contentMobile-xs euiSideNav__contentMobile-s"
id="euiSideNavContent_generated-id"
id="euiSideNav_generated-id_content"
/>
</nav>
`;
6 changes: 4 additions & 2 deletions src/components/side_nav/side_nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export type EuiSideNavProps<T = {}> = T &
};

export class EuiSideNav<T> extends Component<EuiSideNavProps<T>> {
generateId = htmlIdGenerator('euiSideNav');

static defaultProps = {
items: [],
mobileBreakpoints: ['xs', 's'],
Expand Down Expand Up @@ -183,7 +185,7 @@ export class EuiSideNav<T> extends Component<EuiSideNavProps<T>> {
(breakpointName) => `euiSideNav__contentMobile-${breakpointName}`
)
);
const sideNavContentId = htmlIdGenerator('euiSideNavContent')();
const sideNavContentId = this.generateId('content');
const navContent = (
<div id={sideNavContentId} className={contentClasses}>
{this.renderTree(items)}
Expand All @@ -201,7 +203,7 @@ export class EuiSideNav<T> extends Component<EuiSideNavProps<T>> {
let headingNode;

const sharedHeadingProps = {
id: headingProps?.id || htmlIdGenerator('euiSideNavHeading')(),
id: headingProps?.id || this.generateId('heading'),
className: headingProps?.className,
'data-test-subj': headingProps?.['data-test-subj'],
'aria-label': headingProps?.['aria-label'],
Expand Down

0 comments on commit 987d269

Please sign in to comment.