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

po/colorpicker edit - edit area & point #17051

Merged
merged 3 commits into from
Nov 16, 2024
Merged

po/colorpicker edit - edit area & point #17051

merged 3 commits into from
Nov 16, 2024

Conversation

TurboGit
Copy link
Member

Allow for editing area & point as hinted by the tooltip.

@jenshannoschwalm : This is based on top of your PR but it independent, can you test?

How it works:

  1. click on picker icon
  2. right click on a sample
  3. move it / change size of area
  4. deselect the picker

I'd like to avoid 1 and as soon as we right click we enter in edit mode and maybe right-click again to update the sample. Seems a better user interaction to me, what do you think?

@TurboGit TurboGit added this to the 4.8.1 milestone Jun 24, 2024
@TurboGit TurboGit self-assigned this Jun 24, 2024
@TurboGit TurboGit added bugfix pull request fixing a bug feature: enhancement current features to improve labels Jun 24, 2024
@jenshannoschwalm
Copy link
Collaborator

Yes, indeed a step into a better workflow.

One usability downside, you have to prior-select the picker with the correct "mode" (either click for point or right-click for area) and thus have to know the live samples mode. Could we instead activate the main picker in the correct way via right-click on the live sample?

@TurboGit
Copy link
Member Author

TurboGit commented Jun 24, 2024

Could we instead activate the main picker in the correct way via right-click on the live sample?

Sure, that's what I commented in my first post :)

@jenshannoschwalm
Copy link
Collaborator

Ahh, didn't get that. So show me the way to the next ... :-)

@TurboGit
Copy link
Member Author

No time for 4.8.1 and actually not a fix so moving this for 5.0.

@TurboGit TurboGit modified the milestones: 4.8.1, 5.0 Jul 16, 2024
@jenshannoschwalm
Copy link
Collaborator

@TurboGit let me know if you want a review / test!

@TurboGit
Copy link
Member Author

@TurboGit let me know if you want a review / test!

Fact is that this is still buggy IIRC. I need to resume work on it when I'll have time.

@TurboGit
Copy link
Member Author

@jenshannoschwalm : New version pushed and ready for review.

How it works:

  • Hover over a live sample
  • Right-click on the sample
  • The live sample is now copied to the primary sample
  • Change the size/positon of the sample
  • Hover over a live sample (not necessary the same used above)
  • Right-click on the sample
  • The live sample is receiving the primary sample

@jenshannoschwalm
Copy link
Collaborator

I just checked and for me The live sample is now copied to the primary sample was not working. It was like "primary picker is copied to hover-over live sample" ???

@TurboGit
Copy link
Member Author

It was like "primary picker is copied to hover-over live sample" ???

Hum, the second right-click is to copy back the primary to the live sample. What the steps you did? Maybe there is a state reset to do somewhere.

This Gtk routine can be used to generated a mouse button event
to any widget.
The way to do that:
- Hover over a live sample
- Right-click on the sample
- The live sample is now copied to the primary sample
- Change the size/positon of the sample
- Hover over a live sample (not necessary the same used above)
- Right-click on the sample
- The live sample is receiving the primary sample
@TurboGit
Copy link
Member Author

@jenshannoschwalm : I found 2 cases where a reset of the copy-state should be done. Added and force pushed. Can you test again?

One thing I'd like to add is a status (GUI indicator) somehow that a copy has been initialized so a second right-click would reset the live-sample. But I don't know what to do at the moment GUI wise...

@jenshannoschwalm
Copy link
Collaborator

Will check again tomorrow .

@TurboGit
Copy link
Member Author

@jenshannoschwalm : I've just added an indicator inside the copied-patch. Should be better this way.

@TurboGit
Copy link
Member Author

To help the review, when right-click on a live sample to edit it, a "save" icon is displayed over it:

image

From there, either:

  • click on the picker icon, the edited sample is kept on the primary sample.
  • click to add sample, a new sample is created and the edited sample is kept also in the primary sample.
  • right-click on any live sample, the edited sample is copied back into the clicked live sample (not necessary the one we copied from). the sample is also kept into the primary sample.

Note also that it is not necessary to select the picker before. If one right-click on a live-sample the picker is automatically enabled.

The only behavior that bothers me (but not a regression, it is already the case with current master) is that you need to deselect the picker by using the same action as the one used to enter the picker mode. That is, click for a point and right-click for an area. If one click when editing a box then the picker is switched into the point mode. This is quite annoying to me, if anyone has a proposal to make this module better UX wise let us know.

@jenshannoschwalm
Copy link
Collaborator

Just tested and

  1. the symbol helps and the tooltips reflect what happens.
  2. No unexpected things happened :-)

Codewise: nothing noteworthy. About the debug code, we have -d picker so maybe use that instead of fprint and keep it active?

@TurboGit
Copy link
Member Author

@jenshannoschwalm : I'll add back the debug code at some point. Thanks for the review.

@TurboGit TurboGit merged commit 0badacf into master Nov 16, 2024
7 checks passed
@TurboGit TurboGit deleted the po/colorpicker-edit branch November 16, 2024 15:25
@todd-prior
Copy link

That is, click for a point and right-click for an area. If one click when editing a box then the picker is switched into the point mode. This is quite annoying to me, if anyone has a proposal to make this module better UX wise let us know.

Could we not maybe look at a dedicated button down with color assessment etc for a picker . Selecting that would open the CP module panel. Moving the mouse pointer to the desired spot and left clicking would add a point picker and a modifier or right click would add an area selection instead. Each new selection would populate the panel with samples. Clicking that icon down in the bottom bar would toggle the picker on and off... so set it up a bit like ART and RT.... I am not sure of the implications but working with the samples in the panel etc could be the same and the pipette could also just be a toggle for on off. In addition if the point mode could be added much like the variable nodes of a mask people could add a point but then scroll wheel to make the selection a bit larger. THe same with a selection area ...maybe hovering and the scroll wheel could be used to contract or expand the size of the drawn box in addition to the handles. Finally in the way that we can move those nodes maybe the pickers could be allowed to be dragged on the screen to adjust the location.... just some random thoughts and maybe there are issues I haven't thought of .....

@TurboGit
Copy link
Member Author

Moving the mouse pointer to the desired spot and left clicking would add a point picker and a modifier or right click would add an area selection instead.

This could become very messy as one still need a left-click to edit an area colorpicker (changing position, size...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix pull request fixing a bug documentation-pending a documentation work is required feature: enhancement current features to improve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants