adobe-photoshop / spaces-design

Adobe Photoshop Design Space
http://adobe-photoshop.github.io/
Other
852 stars 74 forks source link

Color picker slider position doesn't update on undo #3658

Closed iwehrman closed 8 years ago

iwehrman commented 8 years ago
  1. Change the color of a shape by dragging the slider on the color picker map.
  2. Undo.

Result: the color of the shape is updated on canvas, but neither the color slider position nor the color input are updated.

Bonus: If you change the color to be pure black with the slider in the bottom right corner, undo moves the slider to the bottom left corner.

image

mcilroyc commented 8 years ago

More/different badness: After making several color changes, undo changes the color picker but incorrectly. This may be coalesce related? ss

mcilroyc commented 8 years ago

bisect indicates first bad commit: 5d7ae63e6c4f9b410f52f9be2fbad1b8ccd9f09d

mcilroyc commented 8 years ago

You break it, you bought it. @shaoshing can you please take a look at this since it seems that your "Delay running action to promote browser painting" performance hack has significantly regressed the color picker "timeTravel" handling. You can assign it back to me if you think we need to re-jigger the way this works wrt to history ... but I figured you're the best candidate to make progress on this right now.

mcilroyc commented 8 years ago

FBNC. Please review @ktaki

ktaki commented 8 years ago

@iwehrman: You stated "Bonus: If you change the color to be pure black with the slider in the bottom right corner, undo moves the slider to the bottom left corner."

Currently, if RGB is set to (0,0,0), then HSB is forced to be set to (0,0,0) once color picker is closed. Is this the behavior you want? Or, do you want to HSB to be reset only with undo?

iwehrman commented 8 years ago

I don't really care where the slider is when you open the picker with HSB = (0,x,0). The problem is that if I move the slider to, say, (0,50,0) in one step, and then move it to say, red, in a second step, when I undo it goes back to HSB = (0,0,0) instead of (0,50,0). These all mean "black" but the slider goes to a different place in the map. It's relatively a minor issue.

ktaki commented 8 years ago

@iwehrman: I see. I misread your statement. I thought you wanted the feature. You were actually describing a bug instead. Yes, once B is set to 0 and close the color picker, next time you open it, you will see RGB and HSB all set to 0. The slider is at the bottom left corner. This has not been fixed. I also see if S and B values are set to small value (<=4), H value will be reset to 0. I agree this is a relatively minor issue, but I will open a new issue for these behaviors.

ktaki commented 8 years ago

Opened the issue above as #3688.

ktaki commented 8 years ago

Closing this one as fixed.

iwehrman commented 8 years ago

Thanks @ktaki! And sorry for the initial confusion.