fix(link-with-icon): removed keyboard focus on disable #4082
Conversation
|
Deploy preview created for package Built with commit: 55503c762e3e26ead98c481f1098df507cdb61ae |
|
Deploy preview created for package Built with commit: 55503c762e3e26ead98c481f1098df507cdb61ae |
There was a problem hiding this comment.
Hi @IgnacioBecerra I think the internal <Link> is keyboard-focusable even without tabIndex specified given it uses <a> internally. The internal <Link> automatically turns off such focusability if it's disabled, by turning it to <p>. That said, would you try simply removing tabIndex from the internal <Link>? Thanks!
|
@asudoh You're right, Link does turn it into |
|
@IgnacioBecerra looks like you need to update the snapshot file to reflect the change. You can do this with |
There was a problem hiding this comment.
Thank you for the update @IgnacioBecerra - Below is what I actually meant in case I wasn't clear enough (my apologies if it's the case). Please let me know if it doesn't work. Thanks!
| @@ -37,7 +38,7 @@ const LinkWithIcon = ({ | |||
| )} | |||
| data-autoid={`${stablePrefix}--link-with-icon`}> | |||
| <Link | |||
| tabIndex={0} | |||
| tabIndex={disabled ? -1 : 0} | |||
There was a problem hiding this comment.
| tabIndex={disabled ? -1 : 0} |
| @@ -21,6 +21,7 @@ const { prefix } = settings; | |||
| const LinkWithIcon = ({ | |||
| children, | |||
| className: customClassName, | |||
| disabled, | |||
| /** | ||
| * Toggles keyboard tab focus for element | ||
| */ | ||
| disabled: PropTypes.bool, | ||
|
|
There was a problem hiding this comment.
| /** | |
| * Toggles keyboard tab focus for element | |
| */ | |
| disabled: PropTypes.bool, |
| @@ -78,6 +84,7 @@ LinkWithIcon.propTypes = { | |||
|
|
|||
| LinkWithIcon.defaultProps = { | |||
| children: [], | |||
| disabled: false, | |||
There was a problem hiding this comment.
| disabled: false, |
There was a problem hiding this comment.
Cool, LGTM 👍 once the snapshot is recreated to resolve the merge conflict. Thanks a lot @IgnacioBecerra!
IgnacioBecerra
force-pushed
the
link-with-icon-tab-disable
branch
from
4427efe to
8eea69b
Compare
|
@IgnacioBecerra you may need to re-run the updateSnapshot task to fix this. |
jeffchew
added
the
Ready to merge
…-system#4082) ### Related Ticket(s) carbon-design-system#3712 ### Description The main issue that caused the element to still be focused even while disabled. Removed `tabIndex` as it should still respect the focus order while active and not focus during disabled state. ### Changelog **Removed** - Removed tabIndex from the `Link` element <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "package: vanilla": Vanilla --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "package: styles": Carbon Expressive, React (Expressive) --> <!-- *** "RTL": React (RTL) --> <!-- *** "feature flag": React (experimental) -->
Related Ticket(s)
#3712
Description
The main issue that caused the element to still be focused even while disabled. Removed
tabIndexas it should still respect the focus order while active and not focus during disabled state.Changelog
Removed
Linkelement