kobaltedev / kobalte

A UI toolkit for building accessible web apps and design systems with SolidJS.
https://kobalte.dev
MIT License
1.3k stars 75 forks source link

feat: color components #508

Closed HBS999 closed 2 weeks ago

HBS999 commented 3 weeks ago

This PR adds 4 new color components: ColorArea, ColorSlider, ColorChannelField and ColorSwatch.

netlify[bot] commented 3 weeks ago

Deploy Preview for kobalte ready!

Name Link
Latest commit 17194620f9ae652e5b3bb0ebda137bd4a893941e
Latest deploy log https://app.netlify.com/sites/kobalte/deploys/672349eb6beb3c0008572f1b
Deploy Preview https://deploy-preview-508--kobalte.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

Lighthouse
1 paths audited
Performance: 84 (🔴 down 1 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 82 (🟢 up 7 from production)
PWA: 60 (no change from production)
View the detailed breakdown and full score reports

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

HBS999 commented 3 weeks ago

Thanks for the review! The ColorArea works flawlessly in the playground but breaks in the docs. I was hoping this was an issue with the dev build of the docs and it will be fixed in prod. However look like the netlify bot doesn’t create deploy links to test so I am kinda lost here. Will look more into it now.

Edit: oh looks like the bot does have deploy links. It wasn’t there earlier today.

HBS999 commented 3 weeks ago

Hi Jeremy,

I fixed the ColorArea issue and I have also added default aria-label for the hidden inputs.

Second issue (ColorArea also); there's no aria label for the currently selected color. This is correctly set for the ColorSwatch and ColorSlider but missing in ColorArea.

Do you mean an aria-label on the thumb with the current value?

jer3m01 commented 3 weeks ago

Do you mean an aria-label on the thumb with the current value?

Yeah I think it that would make it a lot nicer to interact with, I'd put the label on the entire color area rather than just the thumb.

HBS999 commented 3 weeks ago

I’ve put it on the thumb instead. Since the ColorArea is neither interactive nor a landmark, I believe it will be ignored unless I am misunderstanding how aria-label works. Let me know what you think.

jer3m01 commented 3 weeks ago

I’ve put it on the thumb instead. Since the ColorArea is neither interactive nor a landmark, I believe it will be ignored unless I am misunderstanding how aria-label works. Let me know what you think.

I think you're right, the label on the thumb is more precise. Label on the color area would read incorrectly as a color swatch.

HBS999 commented 3 weeks ago

Hmm I don’t understand why CI is failing