#868: add apply-button for Key Signature menu

[serval2412]

No description provided.

[guiv42]

thanks for your contribution, just one minor remark.

[guiv42]

Keeping "OK" as the default button seems more logical

[serval2412]

Thank you for the review.

I thought "Apply" as default would be more logical since it allows to test quickly different key signatures.
I mean with the mouse you select the key signature then just type "enter" key so it applies without closing the dialog (1 hand for each action). If you want to test several key signatures, you won't have to loop between going with the mouse to select another key signature then going to the "Apply" button until you find the right one.
Now, perhaps I missed the keyboard shortcut to select "Apply" button?

[guiv42]

I understand your point, but "OK" has always been the default button, and many users will not use "Apply" at all, so I prefer not to change that

[serval2412]

Ok from my point of view not putting "Apply" as by default action is counterintuitive and it decreases the interest of this patch, I prefer abandoning this PR.

[guiv42]

I'm sorry but I don't understand.
Your PR is clearly valuable, I just want to avoid confusing users who are used to dialogs without "apply" option.
Edit: I just asked the original author of associated issue

[serval2412]

I don't want to put my name on a patch I partly disagree. I mean, in our jobs, we must do compromises. Here it's not my job just a opensource project where I'm just providing a casual and straightforward patch and don't want to make some compromise.
I agree with you some people would complain but perhaps other people would be satisfied. Now even if 99% would be against but this change, I prefer not pushing this with OK by default.

Now you can obviously copy paste the code, put the default on OK button and push with your name, no pb for me.

Since you reopened this PR, I'll let you accept it with "Apply" by default or close it when you feel like.

[guiv42]

OK, as you want.
I'll try to get some other users opinions before a decision.

[ElektroVirus]

Hey, thanks @serval2412 for the PR. This is a tough decision, didn't believe we would get a discussion about this, but this is of course a good thing to have.

I agree with both of you. @serval2412 has a good point that this Key Signature dialog would serve my purposes for iterating through key signatures easiest if the Apply-button would be the default button. I could scroll down the list with arrow keys, press enter on each item and see the results. Because enter already applied the last selection, I could get away from the dialog by pressing ESC.

But I appreciate it more if the whole UI of a program is consistent. If this would be the only dialog with Apply and every other dialog would have only Ok which is default, this dialog should behave the same. Also the point of @guiv42 is good, it's always better not to distract old users by unnecessary workflow changes. Why should they now learn to press ESC instead of enter.

Now that I thought about this and looked at other dialogs in TuxGuitar, it also came to my mind that this same issue theoretically applies to the Clef-dialog as well (and maybe Time Signature as well). I think TuxGuitar should concentrate on a consistent UI and thus for this question I'll go on @guiv42 side although @serval2412 is closer on my personal needs.

---

In my original issue I wrote
Another option would be to implement some kind of auto discovery system which counts, which key signature has the least accidentials, but the approach with an apply-button on the dialog definitely has the least work involved.

A third option came to my mind now that I was playing with this dialog. We don't need an apply-button if the key signature would activate as soon as an item has been selected from the drop-down menu. Of course then the cancel-button would have to bring the old key signature back. This sounds genious on paper, but when I started testing this out myself [1], I quickly discovered some issues.

First of all, undoing changes by pressing cancel. Now that each menu item on activation will result in this key change action, the cancel button can't anymore only dispose the dialog, it has to undo the change. In the current TuxGuitar version the two checkboxes which allow different scenarious, apply to end or apply to selection caused probelms and I took the liberty to mend the workflow.

1. User has some bars selected: they key signature is done to those bars. The user can't select apply to end, or only for current measure.
2. User doesn't have selected any bars. Now the user may choose between setting the key signature for only the current bar or till the end.

In my opinion this change simplifies the program logic quite a lot as well as the workflow logic. I also like the idea of seeing the results as soon as I change the drop-down menu. I can do it with the arrow keys and it's fast! No need to press an Apply-button. Well, I'm no UX expert so I'd like to know what you think.

But this approach has still two flaws. First, disposing the dialog using ESC does not do the undo-stuff, that is to reset the key change to the original. The second flaw is with TuxGuitar's undo controller. Now that each item selection causes an action to change the key, each of these actions will be stored in the undo controller. It feels unnatural. I'd like TuxGuitar only to store the last key change. That is, I'm in the menu, have iterated through numerous key changes, then I either cancel or ok the selection. Now if I press ctrl-z, I expect the key change be the one before I opened up the dialog. I don't want to iterate all steps with ctrl-z which I did while being in the dialog.

[1] https://github.com/ElektroVirus/tuxguitar/tree/feature/key-signature-iteration

[guiv42]

Thank you @ElektroVirus for this very detailed answer.
I had not thought of this idea to apply immediately the key signature when an item is selected in drop-down list, that seems an interesting concept.
As you mentioned, the undo controller might be a little tricky to manage, but a priori it seems possible. One solution could be to create an additional action "change key signature without undo controller". In the dialog, first action on the drop-down list would trigger the usual "change key signature" action and load initial state of track in the undo controller, and all subsequent selections would call "change key signature without undo", to avoid messing with this initial state. After closing dialog, pressing Ctrl+z should then restore the state of the track as it was before dialog was opened (no matter how many selections were done)
The "Cancel" button and "Esc" keys might be then tricky to handle, but it doesn't look impossible.
With this, it may be interesting to add an indicator of the number of accidentals in track when one key signature is selected (or even to add a "Hint" button)

I still think the "apply" button is the safest option (whatever the default button). But I'll take some time to experiment with this new concept.

[ElektroVirus]

Sounds awesome! The Apply-button is for sure the easiest patch for the original issue. I would definitely prefer to keep the UX united across different dialogs or parts of the program, there are too many open source projects out there where the idiom too many cooks spoil the broth can be observed as different workflows.

I however really like now this idea where selecting the drop-down menu item directly gives feedback and shows the results. The counter for accidentials and thus a suggestion for key would be a good idea too - probably even better - saves the user the time for iterating through all the possibilities.

But I'm not aware enough of music theory that I could even say, what pitfalls there could be to encounter.

Happy expreimenting! :)

[guiv42]

Playing around with the undo controller is theoretically possible, but it really looks like a dirty hack. The original structure and independence between action/undo controller/update controller has proven to be quite robust, and I don't feel comfortable modifying this.
Proposal: merge this PR as it is, publish the result in a pre-release, and see how it is received by the community of users.
We can still change the default button afterwards if there are user reactions. (Note: to be done after 2.0.0 is released)

[guiv42]

To be merged after tuxguitar-next branch is merged in master