Skip to content
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(Filename): remove tabindex when status is complete #12105

Merged
merged 2 commits into from
Sep 16, 2022

Conversation

tw15egan
Copy link
Collaborator

Closes #12101

Removes the tabindex when the status is complete, as this is just a status icon, and no action can be taken by clicking the icon in this state

Changelog

Changed

  • Removed the default tabIndex when the status is complete
  • Removed cursor styles when the status is complete

Testing / Reviewing

Run a11y checker on the Filename example when the status is complete. No violations should appear

@netlify
Copy link

netlify bot commented Sep 15, 2022

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit b7e8045
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/6324c1a3209c690008bd0555
😎 Deploy Preview https://deploy-preview-12105--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Sep 15, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit b7e8045
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/6324c1a344cb620009c58164
😎 Deploy Preview https://deploy-preview-12105--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@@ -36,7 +36,8 @@ function Filename({ iconDescription, status, invalid, ...rest }) {
<CheckmarkFilled
aria-label={iconDescription}
className={`${prefix}--file-complete`}
{...rest}>
{...rest}
tabIndex={null}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, what would the difference, if any, be between having tabIndex equal null or tabIndex equal to -1?

Copy link
Collaborator Author

@tw15egan tw15egan Sep 16, 2022

Choose a reason for hiding this comment

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

When a prop is set to null it shouldn't render that prop on the component at all. Since it's an SVG and they don't support tabIndex, I figured I'd just remove it in that case. Usability wise they accomplish the same thing!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the main use case of this would be a disabled prop where you would do something like disabled={disabled ? true : null}

@kodiakhq kodiakhq bot merged commit 7ebb737 into carbon-design-system:main Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[a11y]: File uploader item icon is tabbable & has incorrect role.
3 participants