search

jamulussoftware / jamulus

Jamulus enables musicians to perform real-time jam sessions over the internet.
https://jamulus.io
Other
997 stars 222 forks source link

Same ALT shortcut used for Menu and item on screen in Dutch language #1556

Closed henkdegroot closed 3 years ago

henkdegroot commented 3 years ago

A minor issue exists in the Dutch language. The menu item for file is named: Bestand And the ALT combination used for this is ALT + s

The Settings item (displayed on screen, not the menu) is named: Instellingen And the ALT combindation used for this is ALT + s

Meaning both options are using the same combination.

Expected behavior There should not be 2 items available at the same time using the same ALT combination. In most applications Bestand will use the shortcut ALT + B, so that menu item should be updated. In the view menu, there is also an option to get to the settings. This is using ALT + I. I would propose to use the ALT + I on the main screen as well for the Instellingen item.

Screenshots jamulus-alt-keys

Operating system Windows 10

Version of Jamulus 3.7.0

henkdegroot commented 3 years ago

Please assign this issue to me, so I can resolve this with a PR.

hoffie commented 3 years ago

Please assign this issue to me, so I can resolve this with a PR.

Did that :)

Cc'ing @softins as he had worked on such accelerator keys recently as well.

hoffie commented 3 years ago

We'll just have to make sure that the replacement does not conflict with anything else (haven't checked).

henkdegroot commented 3 years ago

We'll just have to make sure that the replacement does not conflict with anything else (haven't checked).

Of course! I will be doing a local build first with the change and test the result before creating the PR. Thank for the reminding the terminoly (accelerator keys)...forgot about that.

softins commented 3 years ago

I'm not sure how I missed this. There is not enough information within the translation files to arrange the items in a hierarchy, so the tool I wrote just lists candidate conflicts for manual checking. I guess this one got overlooked.

softins commented 3 years ago

The tool I used is a perl script, which is available at https://github.com/jamulussoftware/jamulus/blob/master/tools/checkkeys.pl

henkdegroot commented 3 years ago

While I am working on this, I notice the language updates for the added functions in the latest master. Should I expand this issue and resolve this while I am updating the Dutch language? Or would/should that be a different issue?

As a note on the side, I also found the Chat option in the menu not to match with the onscreen Chat option. One is using ALT + C, the other ALT + h. There is no need for this as well, so should be updating these.

If have tested on my local build and did not find conflicting keys/behaviour now. I did noticed another behaviour. When you start jamulus, the accelerator keys on screen are not active. However after pressing ALT the first time, they remain active. Is that something we should look into changing?

The tool I used is a perl script, which is available at https://github.com/jamulussoftware/jamulus/blob/master/tools/checkkeys.pl

I found this....really nice and easy insight in the configured keys. And it seems you had detected the duplicates however in the translation/update they got assigned another key that was in use.

henkdegroot commented 3 years ago

Should I expand this issue and resolve this while I am updating the Dutch language? Or would/should that be a different issue?

Looks like it should be a separate, as the .ts files in master have not been updated yet to reflect/include the new strings (or dialog reference update for the string)