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

[channel-picker] Add support for the compact prop #169

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

DmitrySharabin
Copy link
Member

@DmitrySharabin DmitrySharabin commented Nov 24, 2024

Following our discussion with @LeaVerou on Messenger, this PR is an experiment using radio buttons to select a channel instead of a <select> element.

Works in dark mode, too:

image

I wonder what you think about it.

Live preview: https://deploy-preview-169--color-elements.netlify.app/src/channel-picker/

Here is what it looks like inside charts: https://deploy-preview-169--color-elements.netlify.app/src/color-chart/

Copy link

netlify bot commented Nov 24, 2024

Deploy Preview for color-elements ready!

Name Link
🔨 Latest commit 2f68190
🔍 Latest deploy log https://app.netlify.com/sites/color-elements/deploys/675eb76e31cd0300087c8525
😎 Deploy Preview https://deploy-preview-169--color-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 configuration.

Copy link
Member

@LeaVerou LeaVerou left a comment

Choose a reason for hiding this comment

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

Oh I wasn't thinking of visible radio buttons! More like a button group next to the space picker e.g. see
image (but likely with less background)
The radios would be visually hidden (but still accessible to the keyboard)

@DmitrySharabin
Copy link
Member Author

I changed the styles to look more like a button group; I agree it's much better.

The issue is that I don't know how to show the hierarchy between a color space and its coords. Should we place the space picker above the channels, make it bigger, bolder, or something else? Or should we add some divider, chevron, or arrow?

Do you have any suggestions?

image

@LeaVerou
Copy link
Member

I'd make the text smaller. And maybe the space bold?

But also, I’m thinking there is value in keeping the <select> code around and giving people the option — there are use cases for both

@DmitrySharabin
Copy link
Member Author

I'd make the text smaller.

All the text or only the channels?

But also, I’m thinking there is value in keeping the <select> code around and giving people the option — there are use cases for both

I agree. How about a compact boolean attribute? If provided, the <select> element is used?

@DmitrySharabin
Copy link
Member Author

I worked with the component a bit more, tweaked the styles a bit, and added support for the compact attribute (we can rename it if necessary). Could you please take another look?

https://deploy-preview-169--color-elements.netlify.app/src/channel-picker/

@LeaVerou
Copy link
Member

Better but it still doesn't communicate hierarchy — the channel picker is much more prominent than its color space :/
Perhaps making the letters even smaller?

I wonder if there's any prior art, how does software with a colorspace / channel selector communicate the hierarchy?

PS: Remember that you need more horizontal padding than vertical padding for them to optically look the same

@DmitrySharabin
Copy link
Member Author

What if we use <fieldset> to communicate hierarchy? https://codepen.io/dmitrysharabin/pen/KwPPOrd

image

I couldn't find any examples of software that allows one to choose a specific channel in a specified color space. 🤷‍♂️

@LeaVerou
Copy link
Member

This is huge :( Think of how it would look in the components we want to use it…

@DmitrySharabin
Copy link
Member Author

This is huge :( Think of how it would look in the components we want to use it…

I should have said that I made it bigger for demo purposes. It's not the size it will have IRL; it's just an illustration of the idea. I switched back to 100% font size in the Codepen.

@LeaVerou
Copy link
Member

No I got that. But you usually want something having a height of about one line. Think of any of these above a chart for exmaple.

@DmitrySharabin DmitrySharabin force-pushed the channel-picker-radio-buttons branch 2 times, most recently from a3dcdcb to f4aa696 Compare November 28, 2024 11:15
@DmitrySharabin
Copy link
Member Author

DmitrySharabin commented Nov 28, 2024

What if we go with something as simple as bread crumbs using a slash-like symbol as a delimiter? Or with something fancier, like a connector between the space picker and the channels?

Channel Picker UI

P.S. Those are my drawings, not screenshots of something already implemented. 😅

@LeaVerou
Copy link
Member

I quite like the second one! It has the right horizontal format, and conveys hierarchy better than any of the other options. Not sure we need the gradient, we can just make them both the same border color (make sure it works with dark mode too)

@DmitrySharabin DmitrySharabin force-pushed the channel-picker-radio-buttons branch from f4aa696 to faa78ad Compare December 4, 2024 21:20
@DmitrySharabin DmitrySharabin changed the title [channel-picker] Use radio buttons instead of <select> [channel-picker] Add support for the compact prop Dec 4, 2024
@DmitrySharabin
Copy link
Member Author

@LeaVerou, I implemented the style with connectors. Could you please take a look?

@DmitrySharabin DmitrySharabin force-pushed the channel-picker-radio-buttons branch 7 times, most recently from f20d0a3 to 8e13753 Compare December 9, 2024 17:41
@DmitrySharabin DmitrySharabin force-pushed the channel-picker-radio-buttons branch from 17d2aad to 0bf925e Compare December 13, 2024 16:49
@DmitrySharabin DmitrySharabin force-pushed the channel-picker-radio-buttons branch from 0bf925e to 2f68190 Compare December 15, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants