[QMS-1166] Improve custom keyboard shortcuts

[robert7k]

What is the linked issue for this pull request:

QMS-1166

What you have done:

Improve custom keyboard shortcuts:

• actions from context menus can now also be assigned keyboard shortcuts
• consolidate some actions which have the same label but are placed on different objects. This allows us to assign a single keyboard shortcut to them
  • Edit
  • Copy to
  • Delete
  • Toggle NoGo-Line
  • Toggle NoGo-Area
• group actions by category so that the user can better identify the correct action

Steps to perform a simple smoke test:

1. Open QMapshack
2. Assign a custom keyboard shortcut to some actions
3. Press the keybord shortcut

Does the code comply to the coding rules and naming conventions Coding Guidelines:

• yes

Is every user facing string in a tr() macro?

• yes

Did you add the ticket number and title into the changelog? Keep the numeric order in each release block.

• yes, I didn't forget to change changelog.txt

[kiozen]

What's the state here? Ready for review? A rebase is necessary for sure.

I would like to do a release. Should this be part?

[robert7k]

What's the state here? Ready for review? A rebase is necessary for sure.

Yes, it's read for review. I resolved the conflicts.

I would like to do a release. Should this be part?

Yes, that would be great :-)

[kiozen]

The branch is still in conflict. And please do not use a merge. Do a rebase as documented in the Wiki.

[robert7k]

The branch is still in conflict. And please do not use a merge. Do a rebase as documented in the Wiki.

Strange, it doesn't show me any conflicts anymore. I'll rebase.

[kiozen]

The branch is still in conflict. And please do not use a merge. Do a rebase as documented in the Wiki.

Strange, it doesn't show me any conflicts anymore. I'll rebase.

This is because your button is not in the "Rebase and merge" mode. It's probably in the "Merge commit".

But having all these merge commits is quite irritating. A linear commit history is much easier to read. And better for bisect searches on regressions.

That said: Cleaning up the commit history by an interactive rebase is also good practice. No one wants to work their way thru a lot of back and forth commits 😉

I start the review...

[kiozen]

Ok here are the findings. I have to apologize I couldn't spend much time into review the findings. As with most AI generated stuff it has to be taken with a grain of salt. Some of the findings may render to be bogus.

As far as I understood the major problem seems to be actions with no keys attached today that can get an assignment. If these actions are used by menus that disable/enable actions depending on the cause (GIS items in the workspace tree widget) it might be possible to trigger the action by key even if that does not make too much sense.

And there are some topics concerning the CShortcutSetupDialog.

qms-1166-punchlist.md

[kiozen]

Any update?

[robert7k]

Thank you for the review!

Any update?

Yes, I tried to address all open points. Here are the most important things I changed:

• fixed the crash when filtering project via keyboard shortcut
• Fixed "reset shortcut" while tree is sorted
• Refresh action states on selection change
  • when an item is selected: project actions are run for the project the item belongs to
  • when mixing selections (between item and project): no context menu will be shown anymore, as there are no common entries.
  • when nothing is selected: keyboard shortcuts are ignored if they depend on a project or item
• make “No” default when confirming delete

[kiozen]

when mixing selections (between item and project): no context menu will be shown anymore, as there are no common entries.

This is a regression.

[kiozen]

The changes in CGisListWks triggered me 😉 They make a bad readable and hard to maintain code even more unreadable and unmaintainable. (not really your fault)

I asked Claude for a review and suggestion. This has not been verified by me, yet.

QMS-1166-review.md

The question is how to proceed. This is a major refactor of the code. A good chance for more regressions.

Whatever the decision will be, I think I do not want to have this in the upcoming release. It's better this is merged after the release and is tested more deeply.

Any preference or plan how to proceed?

[robert7k]

when mixing selections (between item and project): no context menu will be shown anymore, as there are no common entries.

This is a regression.

I'm not sure how to handle this properly: if the user selects both an item and a project and then opens the context menu: what should the context menu show?

I see three variants:

• keep it like it is in this PR: context menu applies to selection, that means no context menu if selection is mixed between project / item
• context menu applies only to project or item the right mouse key clicked on: we can always show a menu
• mixed: apply click to selected entries of same type, i.e. right click actions apply to selection of the same type.

What's your preference?

The question is how to proceed. This is a major refactor of the code. A good chance for more regressions.

Whatever the decision will be, I think I do not want to have this in the upcoming release. It's better this is merged after the release and is tested more deeply.

Any preference or plan how to proceed?

I agree. I'll try to address your review remarks. Let's merge this PR once the release is out.

[kiozen]

The current behavior without shortcuts:

• Iterate over all items selected
• Enable/disable, hide/show menu items according to the item types (trk, rte, wpt, area, project) found.
• Apply action to the selected items matching the type

I think this is the main problem as a key shortcut does not know what action is active. This is derived on the fly when summoning the menu. I think the review summary proposes a solution, but that is quite a huge change in the system. But I have to admit I am not that deep in the topic right now. Too much time since the review and too many other stuff on my mind :)