fix: [M3-8422] - Linode IPv6 rDNS typo #10948
Conversation
| @@ -46,6 +46,7 @@ export const LinkButton = (props: Props) => { | |||
| disabled={isDisabled} | |||
| onClick={onClick} | |||
| style={style} | |||
| tabIndex={0} | |||
There was a problem hiding this comment.
@carrillo-erik @abailly-akamai
This makes LinkButtons focusable via the keyboard. I think this is what we want, but please correct me if I'm wrong
There was a problem hiding this comment.
Are buttons not already focusable? 🤔
There was a problem hiding this comment.
I had the same question 😵💫
There was a problem hiding this comment.
@bnussman-akamai I seem to recall the conversation around this topic being more concerned with links styled as buttons. At the time we were looking at the TopMenu icons, specifically the Help & Support vs Notifications.
In this context, I agree with the decision you made to use a LinkButton component. Although, I also wonder if there's some other underlying issue as to why the previous link element was not focusable.
| @@ -169,21 +170,12 @@ const RangeRDNSCell = (props: { | |||
| } | |||
|
|
|||
| return ( | |||
| <button | |||
| <LinkButton | |||
There was a problem hiding this comment.
I know we generally want to discourage LinkButtons, but I think with the current state of things, this makes the most sense here.
My intention here is not to promote LinkButtons, but to use one here so that we can identify it later on and replace it with the correct long term solution.
There was a problem hiding this comment.
Makes sense to me. According to the tech docs, it used to look like a "link button" anyway 🔗
| color: theme.palette.primary.main, | ||
| }} | ||
| > | ||
| {ipsWithRDNS.length} Addresses2 |
There was a problem hiding this comment.
This was the typo (Addresses2 ➡️ Addresses)
|
Coverage Report: ✅ |
| @@ -46,6 +46,7 @@ export const LinkButton = (props: Props) => { | |||
| disabled={isDisabled} | |||
| onClick={onClick} | |||
| style={style} | |||
| tabIndex={0} | |||
There was a problem hiding this comment.
Are buttons not already focusable? 🤔
| @@ -169,21 +170,12 @@ const RangeRDNSCell = (props: { | |||
| } | |||
|
|
|||
| return ( | |||
| <button | |||
| <LinkButton | |||
There was a problem hiding this comment.
Makes sense to me. According to the tech docs, it used to look like a "link button" anyway 🔗
Description 📝
<LinkButton />instead of an un-styled browser button 🔘Preview 📷
How to test 🧪
As an Author I have considered 🤔