-
Notifications
You must be signed in to change notification settings - Fork 364
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
fix: TopMenu related console errors #10398
fix: TopMenu related console errors #10398
Conversation
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.
The only changes here are
- adding
React.forwardRef
- Passing the ref to
a
/RouterLink
@@ -10,7 +10,7 @@ export const Help = () => { | |||
return ( | |||
<TopMenuTooltip title="Help & Support"> | |||
<IconButton | |||
aria-label="Help & Support" | |||
accessibleAriaLabel="Help & Support" |
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.
This makes Link
happy and sets the aria-label properly
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.
Nice, thanks for catching and fixing these! It's so easy for console messages to go unnoticed.
Confirmed on a dev and prod build that the console errors are gone and that the top menu seems to be behaving well! Thanks again!
Coverage Report: ✅ |
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.
Nice! thanks for fixing
- Behavior works as expected on browsers tested (Chrome + FF) ✅
- Semantic HTML + accessibility ✅
Description 📝
Link
component to accept a ref (had to do this to fix Tooltip ref error)Preview 📷
How to test 🧪
aria-label
s 📖As an Author I have considered 🤔