Skip to content

[EuiBasicTable] Fix generated IDs regenerating on rerender#5196

Merged
cee-chen merged 2 commits intoelastic:masterfrom
cee-chen:use-generated-html-id-basic-table
Sep 20, 2021
Merged

[EuiBasicTable] Fix generated IDs regenerating on rerender#5196
cee-chen merged 2 commits intoelastic:masterfrom
cee-chen:use-generated-html-id-basic-table

Conversation

@cee-chen
Copy link
Copy Markdown
Contributor

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

Summary

closes #3111

Related parent PR:

Checklist

QA screencaps below in code comments.

Changelog will be a single entry in #5195.

Copy link
Copy Markdown
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

{(selectAllRows: string) => (
<EuiCheckbox
id={`_selection_column-checkbox_${htmlIdGenerator()()}`}
id={this.selectAllCheckboxId}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Before:
select_all_checkbox_before

After:
select_all_checkbox_after

let button;
const actionContent =
typeof action.name === 'function' ? action.name(item) : action.name;
const ariaLabelId = useGeneratedHtmlId();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Before:
basic_table_action_before

After:
basic_table_action_after

@kibanamachine
Copy link
Copy Markdown

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

@cee-chen cee-chen added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Sep 16, 2021
Copy link
Copy Markdown
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 👍
I think it's worth adding a bugfix changelog entry. Rerender behavior is changing so it seems like more than just an "internals" update. If you want to add a single entry that references all the related PRs, I'm good with that.

@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Sep 20, 2021

Sweet, thanks Greg! I'll likely end up using #5195 as the main changelog PR (since it links out to all the other related PRs) and merge this one as-is!

@cee-chen cee-chen merged commit 571ebc4 into elastic:master Sep 20, 2021
@cee-chen cee-chen deleted the use-generated-html-id-basic-table branch September 20, 2021 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[EuiBasicTable] "Select all" checkbox regenerates id on each render

3 participants