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

ColorPicker: Color format toggle looks like a dropdown #18678

Closed
enriquesanchez opened this issue Nov 21, 2019 · 16 comments
Closed

ColorPicker: Color format toggle looks like a dropdown #18678

enriquesanchez opened this issue Nov 21, 2019 · 16 comments
Assignees
Labels
Needs Dev Ready for, and needs developer efforts Storybook Storybook and its stories for components

Comments

@enriquesanchez
Copy link
Contributor

Describe the bug
The color format toggle (Hex, RGB, HSL) in the ColorPicker looks like a drodpown:

68704932-c479df80-0552-11ea-90ea-ddf488f5e1d8

Expected behavior
The element should look like toggle instead. It was suggested on #18448 to change the arrow for a different icon, to use a a ButtonGroup instead or to convert it to an actual Dropdown:

68784658-e5057080-060a-11ea-9494-f41b8faac347

69289643-ce3ebb00-0bc2-11ea-856b-081b2d83480c

69289815-63da4a80-0bc3-11ea-9424-a993f8f6744a

Let's explore these ideas and see which one feels the best.

@enriquesanchez enriquesanchez self-assigned this Nov 21, 2019
@enriquesanchez enriquesanchez added Needs Design Needs design efforts. Storybook Storybook and its stories for components labels Nov 21, 2019
@enriquesanchez
Copy link
Contributor Author

Here's a proposal for the color format selector using the dropdown pattern already found elsewhere. I felt that the button group takes too much space, and it's already a bit tight due to the color input fields.

Frame 2

Using the dropdown approach allows us to keep the UI clean and provides the necessary affordance.

Would love to hear what you think @richtabor @jasmussen @mapk.

@enriquesanchez enriquesanchez added Needs Design Feedback Needs general design feedback. and removed Needs Design Needs design efforts. labels Dec 4, 2019
@jasmussen
Copy link
Contributor

As a mockup, this looks good to me! I'm assuming it will use the standard popover component? Alternatively, maybe it could just be a stock dropdown?

Screenshot 2019-12-04 at 09 30 06

@richtabor
Copy link
Member

This is great @enriquesanchez! Perhaps the stock dropdown would work best as @jasmussen suggested. That's what we've migrated other similar dropdown utilizing components to (such as the FontSizePicker).

Just to confirm, what would the label update to, if RGB and HSL we're selected? Currently its "Color value in hexadecimal."

@enriquesanchez
Copy link
Contributor Author

Thanks @richtabor and @jasmussen!

I agree that the stock dropdown should be used here.

Just to confirm, what would the label update to, if RGB and HSL we're selected?

Thanks for raising this. I think it should just say "Color value", because we now have the color format specified on the dropdown. What do you think?

@richtabor
Copy link
Member

Thanks for raising this. I think it should just say "Color value", because we now have the color format specified on the dropdown. What do you think?

Good idea; I think that'll work.

@enriquesanchez
Copy link
Contributor Author

Here's an update that takes into account all 3 formats and also adds a label to the format select for accessibility:

Screen Shot 2019-12-05 at 14 10 18

@richtabor
Copy link
Member

What about using Hexadecimal instead of Value for the Hex view?

@richtabor
Copy link
Member

I wonder if Color value would be fine, instead of changing it per format.

@enriquesanchez
Copy link
Contributor Author

I wonder if Color value would be fine, instead of changing it per format.

@richtabor I think we'd still want to keep R G B and H S L above the corresponding fields when those formats are selected.

@mapk
Copy link
Contributor

mapk commented Dec 17, 2019

I like this direction, @enriquesanchez! Because we are using hsl and rgb in the label, maybe we can do what @richtabor mentioned about using hexidecimal there for the hex? Or is it important to indicate somewhere here that it's a "value" of sorts?

@enriquesanchez
Copy link
Contributor Author

@mapk @richtabor Agreed, I think Hexadecimal works great here. This way all three toggle states will show the corresponding label for the color format:

Screen Shot 2019-12-17 at 14 07 40

@mapk mapk added Needs Dev Ready for, and needs developer efforts and removed Needs Design Feedback Needs general design feedback. labels Dec 18, 2019
@shaunandrews
Copy link
Contributor

shaunandrews commented Jan 15, 2020

Was just thinking about this yesterday and spent some time on a few variations. Here's one of my fav's:

image

I also explored having the hex/rgb/hsl input outside of the color picker:

image

Checkout the Figma to see a bunch of options: https://www.figma.com/file/O7epVG8iWfNiNzdSm5RNyd/Component-Color-Picker?node-id=0%3A1

@mapk
Copy link
Contributor

mapk commented Jan 16, 2020

@shaunandrews I really like your mockups with the hex outside the color picker for two reasons:

  1. It's much quick to identify the color value and verify it's what I wanted.
  2. I don't have to open anything to edit it.

Thanks for these explorations!

@enriquesanchez
Copy link
Contributor Author

  1. It's much quick to identify the color value and verify it's what I wanted.
  2. I don't have to open anything to edit it.

I agree with @mapk on these!

One thing I'm not sure about is how to have this coexist with the current pattern of having the theme palette visible. From your mockups @shaunandrews, it looks like the color palette will be visible only when the picker is invoked, right?

@enriquesanchez
Copy link
Contributor Author

enriquesanchez commented Feb 20, 2020

Here's another version riffing off what @shaunandrews shared above:

Screen Shot 2020-02-19 at 19 36 35

I still think that the color palette should be just one-click away, instead of hidden inside the color picker. I expect that to be useful for the majority of users, who might not know what those color formats are.

I got rid of the label on each field (R, G, B, etc.) that I had on #18678 (comment) because this is not a common pattern and it's probably not really necessary here.

Also in this version, I like how the selected color is bigger and clearer

@richtabor @mapk @shaunandrews what do you think?

@enriquesanchez
Copy link
Contributor Author

Closing this in favor of #19785, which is exploring similar iterations to the color picker.

Thanks @richtabor @mapk @shaunandrews for all the feedback and help. Let's continue moving this forward on that other issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Ready for, and needs developer efforts Storybook Storybook and its stories for components
Projects
None yet
Development

No branches or pull requests

5 participants