Skip to content

Conversation

@OnkarRuikar
Copy link
Contributor

The PR

  • makes color pickers keyboard navigable
  • fixed width for labels so UI layout won't change

@OnkarRuikar OnkarRuikar requested a review from a team as a code owner July 26, 2024 08:28
@OnkarRuikar OnkarRuikar requested review from pepelsbey and removed request for a team July 26, 2024 08:28
Copy link
Member

@pepelsbey pepelsbey left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you for improving the accessibility of this tool :)

I’d also suggest removing the role and tabindex attributes from buttons since they already have this role and are focusable by default. There’s one selector in the script that relies on the tabindex attribute, but it likely from the picker. I haven’t noticed any difference in the behavior.

Also, I noticed an error in DevTools because the source map file mentioned in the JS file is missing:

//# sourceMappingURL=pickr.es5.min.js.map

How about we remove this line to calm it down a bit? :)

OnkarRuikar and others added 2 commits August 23, 2024 08:28
Co-authored-by: Vadim Makeev <hi@pepelsbey.dev>
@OnkarRuikar
Copy link
Contributor Author

There’s one selector in the script that relies on the tabindex attribute, but it likely from the picker.

Yes, it's from picker. And the current changes don't affect it.

@OnkarRuikar OnkarRuikar requested a review from pepelsbey August 23, 2024 03:02
Copy link
Member

@pepelsbey pepelsbey left a comment

Choose a reason for hiding this comment

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

Perfect! Thank you :)

@pepelsbey pepelsbey merged commit cd8a45a into mdn:main Aug 23, 2024
@OnkarRuikar OnkarRuikar deleted the css_color_mix branch August 24, 2024 04:00
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants