Skip to content

MVP eyedropper tool for fill colors #300

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

Merged
merged 3 commits into from
Jul 24, 2021
Merged

MVP eyedropper tool for fill colors #300

merged 3 commits into from
Jul 24, 2021

Conversation

henryksloan
Copy link
Contributor

@henryksloan henryksloan commented Jul 24, 2021

Closes #303.

This was marked for Sprint 7, but I realized this MVP would be a quick and useful addition.

  • Enabled eyedropper tool on the tool shelf
  • Added getters to Fill and Stroke
  • Added Lmb => MouseDown input mapping for the eyedropper tool
  • Implemented an MVP for the eyedropper tool
    • Just checks for the layer fill of the topmost layer intersecting the click
    • Updates the primary color

Caveat: The color update is implemented using FrontendMessage::UpdateWorkingColors as opposed to SelectPrimaryColor. That is because using the former doesn't update colors on the frontend. Adding UpdateWorkingColors to the handler for SelectPrimaryColor causes an unresponsive infinite-loop on the frontend. This is because SwatchPairInput registers a response handler for ResponseType.UpdateWorkingColors but also watches primaryColor and secondaryColor, mapping changes to SwatchPairInput.update{Primary | Secondary}Color. Those two functions each the respective WASM functions, which in turn dispatch the color selection messages, causing an infinite loop.

In the future, I think this infinite loop should be avoided by removing the watches and instead add callbacks to ColorPicker. We should only be dispatching the Select...Color messages when the frontend (i.e. the user) updates the colors. In fact, as it currently is, the response handler for UpdateWorkingColors always dispatches redundant calls to SelectPrimaryColor and SelectSecondaryColor.

I could implement that fix, but I think we should leave it for another PR to work out details.


This change is Reviewable

@henryksloan henryksloan requested review from Keavon and TrueDoctor July 24, 2021 20:01
@Keavon
Copy link
Member

Keavon commented Jul 24, 2021

This was marked for Sprint 7, but I realized this MVP would be a quick and useful addition.

Tasks from upcoming sprints are always on the table! I was also debating with myself if we should even include it in 6 or 7. Thanks for grabbing it.

Copy link
Member

@TrueDoctor TrueDoctor left a comment

Choose a reason for hiding this comment

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

LGTM

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jul 24, 2021

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: fb878bd
Status: ✅  Deploy successful!
Preview URL: https://a38ba3b3.graphite-master.pages.dev

View logs

@Keavon
Copy link
Member

Keavon commented Jul 24, 2021

Can you please add right click mapping to the secondary color? So left click samples into the primary color while right click samples into the secondary color.

@henryksloan henryksloan merged commit da84227 into master Jul 24, 2021
@henryksloan henryksloan deleted the eyedropper-mvp branch July 24, 2021 22:29
Keavon pushed a commit that referenced this pull request Jun 16, 2022
* Implement eyedropper for layer fill colors

* Add shortcut for eyedropper

* Add right mouse sampling for secondary color
Keavon pushed a commit that referenced this pull request Jul 30, 2023
* Implement eyedropper for layer fill colors

* Add shortcut for eyedropper

* Add right mouse sampling for secondary color
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.

Eyedropper Tool MVP
3 participants