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

Add color picker popover to ColorInput #910

Open
jmaen opened this issue Jan 5, 2024 · 9 comments
Open

Add color picker popover to ColorInput #910

jmaen opened this issue Jan 5, 2024 · 9 comments
Assignees
Labels
a-ui Relates to the ui package b-enhancement New feature or request c-accepted The issue is ready to be worked on

Comments

@jmaen
Copy link
Contributor

jmaen commented Jan 5, 2024

Description
Add a color picker popover to the existing ColorInput component. One could then select a color with instant visual feedback instead of having to type the color explicitly, which I think is way more convenient.

Proposed solution
The picker could open when the ColorPreview component inside the ColorInput is clicked.

I looked for existing solutions and found react-colorful, which I think looks promising.

@jmaen jmaen added the b-enhancement New feature or request label Jan 5, 2024
@jmaen
Copy link
Contributor Author

jmaen commented Jan 6, 2024

I played around with it a little just for fun, I imagine it could look something like that (sorry for the color artifacts):

color-picker

@jmaen jmaen closed this as not planned Won't fix, can't repro, duplicate, stale Jan 6, 2024
@jmaen jmaen reopened this Jan 6, 2024
@aarthificial
Copy link
Contributor

While it's true that we'll need a color picker sooner or later for the sake of editable signals, I am strongly against using third-party components in the editor as they destroy any sense of consistency in terms of the visuals and the feel.

It's important to keep in mind that Motion Canvas is meant to feel like a desktop application and not a website.
Here's an example of the color picker from Blender:
image
Most notably, it uses HSV sliders that both look and work the same as any other field in the application.

As such, the color picker should be implemented using our own components.
This shouldn't be too complex as it's just a few sliders and two CSS gradients on top of each other.
We also already depend on chroma-js which provides all the logic necessary for color conversion an manipulation.

@aarthificial aarthificial removed their assignment Jan 11, 2024
@aarthificial aarthificial added c-accepted The issue is ready to be worked on a-ui Relates to the ui package labels Jan 11, 2024
@jmaen
Copy link
Contributor Author

jmaen commented Jan 11, 2024

I'll look into that. I guess the new NumberInput will come in handy for the sliders.

@jmaen
Copy link
Contributor Author

jmaen commented Jan 12, 2024

Does it have to be the same layout as in Blender, i.e. one (circular) map for hue and saturation and another one for brightness, or would you also be fine with one map for saturation and brightness and another linear one for hue (as in the gif I sent)? I think the latter would be easier to implement and I like the easy way of keeping the saturation and brightness while playing around with the hue, but I guess that comes down to personal preference.

@aarthificial
Copy link
Contributor

Yes, the square + hue slider from the gif is better. It's common in most drawing software so I think it would be a good default.

@jmaen
Copy link
Contributor Author

jmaen commented Feb 16, 2024

I had some other things going on but I finally got to work on it again. The functionality is there now but before spending time on styling I wanted to clarify how exactly you imagine the component to look like.

That's the current (temporary) state:
grafik

Some feedback would be great @aarthificial !

@aarthificial
Copy link
Contributor

aarthificial commented Feb 17, 2024

Looking good so far!
Here are some tweaks:
image

  1. The vertical gap should be 8px everywhere, and the padding should be 16px to be consistent with the rest of the editor.
    Here's it on a 4x4px grid:
    image
  2. Selection circles should be 12x12px with the one over the hue being black to stand out better.
  3. Values should be presented with 4 decimal places
  4. Inputs should have nested labels on the left
  5. There should be an alpha/opacity slider at the bottom.
  6. Hue should range from 0 to 1 (Consistent with Blender)

@jmaen
Copy link
Contributor Author

jmaen commented Feb 17, 2024

Great, got everything looking good now. Just one more thing, where do you want the popover to be located relative to the color preview? Just some fixed distance to the right? If it just opens where the click happens, the background blends with the rest of the settings pane.
Or do you want to make the popover background darker, the way Blender handles it?

@aarthificial
Copy link
Contributor

Having it fixed on the right is ok for now, 12px to match the padding around the zoom picker.
Tho I'm also considering making it collapsible, similarly to how colors in the inspector are presented:
image
I think in the future this would work better with the inspector on the right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-ui Relates to the ui package b-enhancement New feature or request c-accepted The issue is ready to be worked on
Projects
None yet
Development

No branches or pull requests

2 participants