Skip to content

refactor: annotation size factor editing into toolbar#549

Open
robertwidfen wants to merge 1 commit into
Satty-org:mainfrom
robertwidfen:refactor/annotation_size_factor_editing_in_toolbar
Open

refactor: annotation size factor editing into toolbar#549
robertwidfen wants to merge 1 commit into
Satty-org:mainfrom
robertwidfen:refactor/annotation_size_factor_editing_in_toolbar

Conversation

@robertwidfen

@robertwidfen robertwidfen commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

This was inspired by PR #548 by FedotCompot.

The dialog is now gone and editing is directly possible in the
toolbar. Default step sizes are adjusted to 0.1 and 1.0 and
minimum to 0.01 and maximum to 99.99.

The GTK default bindings are:

  • left mouse button, mouse wheel and up/down step size is 0.1
  • middle mouse button and page up/down step size is 1.0
  • right mouse button jumps to minimum/maximum
  • holding Shift will switch to 0.01 step size

Leave the input with Escape, Return or by clicking somewhere in the
screenshot.

@robertwidfen

Copy link
Copy Markdown
Collaborator Author

Direct comparison to PR#548

I also wonder if we really need the reset.

image

@robertwidfen robertwidfen force-pushed the refactor/annotation_size_factor_editing_in_toolbar branch 2 times, most recently from 3779029 to c322ba9 Compare June 19, 2026 20:33
@robertwidfen

Copy link
Copy Markdown
Collaborator Author

In PR #548 FedotCompot asked: At this point remove the S M L buttons and just use the number as a scale factor directly, no?

I am kind of uncertain - I like the S-M-L for quick switching between defined sizes.
But on the other hand I also like to get the toolbar less crowded.

For the moment I would keep S-M-L (size) as it is wired into Satty. All the tools use it and blur-tool uses the size but not the annotation-size-factor. So removing size would touch many files ...

pub struct Style {
    pub color: Color,
    pub size: Size,
    pub fill: bool,
    pub annotation_size_factor: f32,
}

pub enum Size {
    Small = 0,
    #[default]
    Medium = 1,
    Large = 2,
}

What do others think?

@RobertMueller2

Copy link
Copy Markdown
Member

Regarding reset: I don't think the reset button is needed. When I implemented the dialog, the setting was available as config parm and command line parm only, and it was the first time we exposed it via GUI. It felt right at the time, because it's a technical value and I wasn't sure if it was confusing, so I wanted to provide a way to go back to the value that was configured. It has been around for a bit more than a year and there haven't been many complaints about it. I think it's safe to say the reset button was overcautious.

Regarding S/M/L: I think ONE setting for size would be good. But personally I'm not yet convinced the spin button on its own is intuitive enough. Let's get this merged for now, it's a big improvement as it is.

In the long run, I'd like to pick up discussion again what might be an intuitive control, as has been discussed in #178. Maybe we can take that as a basis too, although it would need some heavy rebasing now, though.,

@robertwidfen robertwidfen force-pushed the refactor/annotation_size_factor_editing_in_toolbar branch 4 times, most recently from 9b482fa to 1e7be55 Compare June 19, 2026 22:23
@robertwidfen

Copy link
Copy Markdown
Collaborator Author

It is not ready to merge, there is still a potential racing condition with updates.

@robertwidfen robertwidfen force-pushed the refactor/annotation_size_factor_editing_in_toolbar branch 2 times, most recently from dd5d03c to 35e4e02 Compare June 20, 2026 18:51
This was inspired by PR Satty-org#548 by FedotCompot.

The dialog is now gone and editing is directly possible in the
toolbar. Default step sizes are adjusted to 0.1 and 1.0 and
minimum to 0.01 and maximum to 99.99.

The GTK default bindings are:
 - left mouse button, mouse wheel and up/down step size is 0.1
 - middle mouse button and page up/down step size is 1.0
 - right mouse button jumps to minimum/maximum
 - holding Shift will switch to 0.01 step size

Leave the input with Escape, Return or by clicking somewhere in the
screenshot.
@robertwidfen robertwidfen force-pushed the refactor/annotation_size_factor_editing_in_toolbar branch from 35e4e02 to f3199ce Compare June 20, 2026 18:55
@robertwidfen

Copy link
Copy Markdown
Collaborator Author

IMHO it is now ready for another review and test by others.

To fix the racing condition I removed the

#[watch]
                set_label: &model.annotation_size_formatted,

and annotation_size* fields - they were actually not necessary anymore.

The start value is set from config at creation.

@RobertMueller2

Copy link
Copy Markdown
Member

IMHO it is now ready for another review and test by others.

To fix the racing condition I removed the

#[watch]
                set_label: &model.annotation_size_formatted,

and annotation_size* fields - they were actually not necessary anymore.

The start value is set from config at creation.

good catch, I guess that was needed for the interaction of the value with both the label and the dialog.

@RobertMueller2 RobertMueller2 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

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.

2 participants