-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Dashboard] Fix unsaved changes badge React error #154607
[Dashboard] Fix unsaved changes badge React error #154607
Conversation
786dfe0
to
080b8b8
Compare
6587e47
to
a1d84bb
Compare
a1d84bb
to
7bd06a1
Compare
💚 Build Succeeded
Metrics [docs]Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Pinging @elastic/kibana-presentation (Team:Presentation) |
Pinging @elastic/appex-sharedux (Team:SharedUX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh, thanks for fixing this! lgtm! code review only
Pinging @elastic/kibana-accessibility (Project:Accessibility) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks!!
Summary
Before
Since adding the tooltip to the unsaved changes badge in #154253, React was throwing an error to the console because the element in the top nav no longer had a unique key:
Screen.Recording.2023-04-10.at.8.57.26.AM.mov
After
This PR fixes this by adding the key to the tooltip if the badge has one; if it doesn't have a tooltip, then the key is added directly to the
EuiBadge
as expected. It also ensures that the tooltip has the proper a11y support (cc @elastic/kibana-accessibility) by adding the badge to the tab order and using the pass-through-props to ensure that the tooltip shows up on focus:Screen.Recording.2023-04-10.at.9.07.04.AM.mov
Checklist
For maintainers