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

Set circle count on "Ctrl+Scroll Wheel" #2162

Closed
wants to merge 3 commits into from

Conversation

Drarig29
Copy link

@Drarig29 Drarig29 commented Dec 13, 2021

Related to: #1214

This PR implements this idea: #1214 (comment)

I had to have some kind of global variable to keep track of the current circle number, so I added a m_context.circleCount context variable.

Maybe there is a better way, I know that something which is tool-related maybe shouldn't be in the general context.

I moved the "incrementation of the series" logic elsewhere: now the increment is done when adding a new circle.

You are not forced to click outside of the capture area to continue the number series anymore.

And you can go back to:

  • Previous numbers
  • Reset to 1
  • Reset to 0
  • Negative numbers
    • I kept this because maybe there would be a use-case for this
    • If we were to actually bound the value to a minimum, this minimum should be 0. I had this need this morning because I wanted to display IDs, which were starting from 0.

Instead of forcing every TYPE_CIRCLECOUNT tool's count in a loop
Used to display a notifier box
@Drarig29
Copy link
Author

Here is a demo:

demo

@affirVega
Copy link
Contributor

affirVega commented Dec 14, 2021

I like this idea.
Your PR kinda relates to №2108. We agreed that circles must not change their number when their position in the object list is changed. I have implemented circle counter similarly to yours, but your feature is better by adding ability for user to change circle count and a little different, because my commit ensures the sequential order of numbers.


Also, please tell what happens if you

  • open a screenshot
  • place a circle counter
  • press Ctrl-Z to undo circle counter
  • place a circle counter

Does this circle counter start with a 2 after the undo?

@Drarig29
Copy link
Author

Yes, currently it starts with a 2 after the undo. And that's not a good thing for the user...

So your CaptureWidget::restoreCircleCountState is a good add.

@panpuchkov
Copy link
Contributor

I cannot agree with such approach, it breaks the idea of Numbered Circles at all. It should be sequential numbers to be able to numerate steps one by one.
If someone really needs it, it should to be available as an option, not a replacement.
On removing some Numbered Circle in the middle, the rest should be renumbered.
Don't break existing features please.

@Drarig29
Copy link
Author

I cannot agree with such approach, it breaks the idea of Numbered Circles at all. It should be sequential numbers to be able to numerate steps one by one. If someone really needs it, it should to be available as an option, not a replacement. On removing some Numbered Circle in the middle, the rest should be renumbered. Don't break existing features please.

Good point! As I can see I'm not a power user of flameshot, I didn't know you could select a circle and remove it!

You are right then. To be honest, my only need of this PR was to be able to reset the counter to a specific number (#1214), and specifically to 0 because one time I needed to annotate with IDs which started from 0, and I wanted to use the "Numbered circles" because it was easier (I thought) than writing text directly.

I'm closing the PR 😉

@Drarig29 Drarig29 closed this Dec 19, 2021
@Drarig29 Drarig29 deleted the feat_set_circle_count branch July 28, 2024 22:12
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.

3 participants