New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 621852 link

Starred by 4 users

Issue metadata

Status: Archived
Owner: ----
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Incrementing/decrementing values in color picker could be improved

Project Member Reported by chowse@chromium.org, Jun 21 2016

Issue description

Chrome Version: 53.0.2769.0

What steps will reproduce the problem?
(1) Open any web page
(2) Use Inspect on any element (ideally one with a different text/background color)
(3) Find a color/background-color/etc property in the style pane
(4) Click on the swatch beside the color to open the Color Picker
(5) If a HEX value, switch to RGBA or HSLA mode (use the double arrows beside the HEX field)
(6) Put keyboard focus inside the alpha (A) field
(7) Push the up/down arrows

What is the expected result?
The alpha value should increment by a small step (e.g. ±0.1 or ±0.001) and should be clamped between 0 and 1.

What happens instead?
The value increments by ±1, making it instantly >1 or <0.


Alpha is the worst offender, but other values could be improved:

* When incrementing/decrementing, values could be clamped to valid values:
  * RGB: 0-255
  * A: 0-1
  * H: 0-360
  * SL: 0-100
* Cyclical values (like H) could rotate when incremented/decremented
  * H(356) + H(1) = H(0)
  * H(0) - H(1) = H(356)
* If the user types an invalid value into a field (e.g. 300 for R), it should be
  auto-corrected when the field loses focus (we already appear to be doing this
  correction in the style property itself).

 
picker-alpha-increment.png
39.5 KB View Download
Hello! This seems like a good first bug. I'm a decently experienced JavaScript developer and would like to take this on. I have the repo cloned and building. Where should I begin in terms of looking for files to edit?
I traced down the relevant code to third_party/WebKit/Source/devtools/front_end/elements/Spectrum.js, and believe I need to add some code to check the input values before accepting them. Is that correct? Do you anticipate any other files being changed? Thank you!
Ok, I think I have a good idea on how to proceed!

I'll first modify how the inputs are rendered so that we have more context on which input was modified. (Right now we only know that one of the input elements was modified, and don't have a way to identify which one.)

I'll then modify the _inputChanged() function to take action on this change event. I'll check which input was modified, and based on that modify the newValue and also make sure it's clipped to its limits.
Worked on this over the last few hours, and have a working patch for this. My network connection is slow so running `git cl upload` is taking a while. Will post here when ready. I'll also include you as a reviewer. :)
Update: I've gotten the custom deltas for alpha, clamping for RGB and SL, and rotation for H done. I'm yet to do:

- Resetting the field on blur if the user inputs something invalid
- Fix a bug where the scroll direction doesn't match the direction of value update
- Remove uses of `KeyboardEvent.keyIdentifier` since it's being deprecated later this year. See https://www.chromestatus.com/features/5316065118650368.

Once these three are done I'll upload a CL. Should be done by tomorrow evening PT!
Here's a 45 second screencast of my progress so far: https://www.dropbox.com/s/nja2hp6o2l8ycye/Chromium%20Color%20Picker%20Progress%20I.mov?dl=0
Cc: lushnikov@chromium.org
Status: Available (was: Unconfirmed)
 athyuttamre, this sounds excellent. Please add paulirish @ chromium  and lushnikov @ chromium as reviewers for your CL.

Thanks
Will do, thank you! Sorry this past week and a half have been busy at work, and I haven't been able to get to this. Will try to push it over the weekend!

Could you assign me as the owner / set the status to Started please?
Cc: athyutta...@gmail.com
Status: Started (was: Available)
I can't set owner to non-chromium accounts, but this works for now.

No rush, but happy to hear you have something working. :)
Status: Archived (was: Started)
Bulk DevTools triage, closing low priority issues with no action plan.

Sign in to add a comment