Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update multiple functional components to useGeneratedHtmlId hook #5195

Merged
merged 13 commits into from
Sep 21, 2021

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Sep 16, 2021

Summary

Follow up to #5133: This PR updates our functional components to not regenerate our randomized HTML IDs on every component update/rerender.

Checklist

N/A - Individual component QA steps listed above.

  • Changelog entry

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5195/

Copy link
Contributor Author

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA before/after screencaps

@@ -156,7 +156,7 @@ export const EuiButtonGroup: FunctionComponent<Props> = ({
);

const typeIsSingle = type === 'single';
const nameIfSingle = name || htmlIdGenerator()();
const nameIfSingle = useGeneratedHtmlId({ conditionalId: name });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EuiButtonGroup (c9c846c)

Before:

After:

@@ -244,7 +244,7 @@ export const EuiCard: FunctionComponent<EuiCardProps> = ({
className
);

const ariaId = htmlIdGenerator()();
const ariaId = useGeneratedHtmlId();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EuiCard (b7d9024)

Before:

After:

Comment on lines +67 to +68
const switchId = useGeneratedHtmlId({ conditionalId: id });
const labelId = useGeneratedHtmlId({ conditionalId: labelProps?.id });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[QA] This is a cleanup refactor and no behavior should have changed on https://eui.elastic.co/pr_5195/#/forms/form-controls#switch

@@ -184,7 +184,7 @@ export const EuiKeyPadMenuItem: FunctionComponent<EuiKeyPadMenuItemProps> = ({
if (checkable) Element = 'label';
type ElementType = ReactElementType<typeof Element>;

const itemId = id || htmlIdGenerator()();
const itemId = useGeneratedHtmlId({ conditionalId: id });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EuiKeyPadMenuItem (f6f8df6)

Before:

After:

const editorId = useMemo(() => _editorId || htmlIdGenerator()(), [
_editorId,
]);
const editorId = useGeneratedHtmlId({ conditionalId: _editorId });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[QA] This is a cleanup refactor and no behavior should have changed on https://eui.elastic.co/pr_5195/#/editors-syntax/markdown-editor

@@ -21,7 +21,7 @@ export const CheckboxMarkdownRenderer: FunctionComponent<
const { replaceNode } = useContext(EuiMarkdownContext);
return (
<EuiCheckbox
id={htmlIdGenerator()()}
id={useGeneratedHtmlId()}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EuiMarkDownEditor checkboxes (4385804)

Before:

After:

@@ -113,7 +113,7 @@ export const EuiNotificationEvent: FunctionComponent<EuiNotificationEventProps>
'euiNotificationEvent__title--isRead': isRead,
});

const randomHeadingId = htmlIdGenerator()();
const randomHeadingId = useGeneratedHtmlId();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple EuiNotificationEvent IDs (ba36dec)

Before:

After:

Comment on lines +60 to +63
const resizerId = useGeneratedHtmlId({
prefix: 'resizable-button',
conditionalId: id,
});
Copy link
Contributor Author

@cee-chen cee-chen Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[QA] All resizable_container changes are cleanup refactors and no behavior should have changed on https://eui.elastic.co/pr_5195/#/layout/resizable-container

@@ -122,8 +122,7 @@ export const EuiTourStep: FunctionComponent<EuiTourStepProps> = ({
footerAction,
...rest
}) => {
const generatedId = htmlIdGenerator();
const titleId = generatedId();
const titleId = useGeneratedHtmlId();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EuiTour (d650595)

Before:

After:

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5195/

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@cee-chen cee-chen merged commit 91baf67 into elastic:master Sep 21, 2021
@cee-chen cee-chen deleted the use-generated-html-id-2 branch September 21, 2021 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants