Minor measurement GUI improvements

[marco-langer]

Closes #14

[gavv]

Nice!

I tested it, seems that duration checkbox logic is reverted, when checkbox is enabled, duration field is disabled, and vice versa. Looks counterintuitive.

one year. TODO find a better solution to have infinite measurement

I suggest to add --duration=inf to CLI, or maybe assume infinite measurement if --duration is omitted. I think infinite measurement should be default.

[marco-langer]

I tested it, seems that duration checkbox logic is reverted, when checkbox is enabled, duration field is disabled, and vice versa. Looks counterintuitive.

The checkbox was supposed to get a label with text "infinite measurement" (the PR is still in draft mode). That's why its check state is the opposite of the enabled state of the duration spin box.

But I could also drop the label idea and have the checkbox enable the spin box as you suggested. But then I think it is not very intuitive how to interpret the disabled duration spin box?

one year. TODO find a better solution to have infinite measurement

I suggest to add --duration=inf to CLI, or maybe assume infinite measurement if --duration is omitted. I think infinite measurement should be default.

I think adding --duration=inf is quite difficult to implement if we continue to use a third-party argument parsing library. Currently cxxopts expects this field to be of floating point type. We could make it optional and then use an infinite duration if the field is not supplied. I think this is an easier solution.

I currently had this hacky one-year trick (again, PR was still a draft) because I was not sure if the infinite measurement argument is necessary at all when the GUI starts to use the core classes directly rather than spawning the CLI in another subprocess.

[gavv]

The checkbox was supposed to get a label with text "infinite measurement" (the PR is still in draft mode). That's why its check state is the opposite of the enabled state of the duration spin box.

But I could also drop the label idea and have the checkbox enable the spin box as you suggested. But then I think it is not very intuitive how to interpret the disabled duration spin box?

Ah, got it. Up to you, I'm fine with both approaches.

We could make it optional and then use an infinite duration if the field is not supplied. I think this is an easier solution.

Sounds good to me.

I currently had this hacky one-year trick (again, PR was still a draft) because I was not sure if the infinite measurement argument is necessary at all when the GUI starts to use the core classes directly rather than spawning the CLI in another subprocess.

Actually, infinite measurement is useful feature by itself. I find myself using --duration=99999 most time when I'm using the CLI tool.

On the other hand, it's not necessary implement it in this PR, we can do it later and keep one year hack for now. Up to you as well.

[marco-langer]

What do you think about the latest change which puts the start/stop buttons into the toolbar?

There are other open tickets which made me think that a menu- and toolbar might be a good idea for the future (e.g. load/save of settings, maybe also an 'About' dialog). The start/stop buttons could get start/stop icons in a future refactoring.

Or do you prefer to have the start/stop buttons not in the toolbar and rather somewhere bigger and more central in the GUI?

[gavv]

Interesting idea, but is it possible to make toolbar buttons more noticeable? Here is how they look like on my machine:

"Start" and "Stop" look like labels, not buttons.

I think it would help to add borders and make them bigger or bolder. Icons would be nice too.

[marco-langer]

I added some (dummy) icons just to see how it would look like with icons.

On my machine (Linux Debian):

Unlike in your image (macOS?), the toolbar in my image already has some style which separates it from the other part of the main window. Let's ignore for a second that the icon is not centered. This can be fixed, but not in a very elegant way.

As you can see, the hovered style of the icon (Start button is currently hovered in my image) already has a border, thus I am not sure if the default, non-hovered style also should have a border.

[gavv]

Thanks for update!

Now it looks fine on my machine too, I can now easily understand that they are buttons. Icons are nice as well.

I'm also using Debian, I guess the difference is because we're using different themes. I'm using qtcurve.

BTW where are these icons from? Did you draw them, or otherwise do we need to add license for them?

Let's ignore for a second that the icon is not centered. This can be fixed, but not in a very elegant way.

Up to you whether to fix it, I'm fine with uncentered look too.

As you can see, the hovered style of the icon (Start button is currently hovered in my image) already has a border, thus I am not sure if the default, non-hovered style also should have a border.

I guess it's OK to let theme decide whether to draw borders / lines / etc, i.e. I'm fine to keep it as it's now with the last update.

[marco-langer]

Yes, I draw them by myself. My inkscape skills are limited, but it's just a green triangle and a (rounded) red square. Should I also add the source svg-file to the repo?

I will have a look if the stop button can be made a couple of pixels smaller and also check whether a slightly inner shadow to the buttons will play out.

On your screenshot the icons indeed do look more centered compared to my screenshot.

[gavv]

Yes, I draw them by myself. My inkscape skills are limited, but it's just a green triangle and a (rounded) red square. Should I also add the source svg-file to the repo?

Cool. Yep, please add them.

I will have a look if the stop button can be made a couple of pixels smaller and also check whether a slightly inner shadow to the buttons will play out.

👍

On your screenshot the icons indeed do look more centered compared to my screenshot.

I guess it depends on theme quite a lot.

[gavv]

Please ping me when I should review it.

[marco-langer]

I'll do it, this was just the rebase on top of the latest changes on main

[gavv]

FYI: for infinite measurement, you can now set --duration to 0 or omit it.