Skip to content

Refactor Global Menu and unify ActionToolBar and GlobalMenu Actions

Eamonn Rea requested to merge eamonnrea/plasma-systemmonitor:use-about-action into master

This MR attempts to "unify" the Global Menu and the ActionToolBar actions. This MR borrows a QML component from Filelight, let me know if this either needs further credit, or is out-right unacceptable. I am not sure what the policy is around code; to be clear, I am not trying to pass that component off as my own work, it just allows to solve the problem in a convenient way, as I don't think there is a built-in solution.

Right now they are defined separately, which means code duplication and also the opportunity for these to become de-synced. This has happened before; the "About" action has broken in the Global Menu a couple of times (!272 (merged), !307 (merged)). I made MRs to fix it both times, but never really stopped to think if there was a better way to handle this.

When working on an MR for another KDE application, Filelight, I noticed they took an approach to resolve this, by making the Global Menu actions trigger existing actions. Instead of these being re-defined for the Global Menu, a custom QML component called ActionMenuItem was used. This takes the ID of a Kirigami.Action and will use the information from that -- The name, icon, and the onTriggered logic -- to create the MenuItem for the Global Menu.

In doing this, we can make sure items are defined with behaviour in one place for both menu types, which should prevent issues like the "About" action becoming de-synced. It also removes duplicated code from the Global Menu QML. It also makes the icons chosen for the actions consistent as well.

As well as the above, I also made two other changes in this MR:

  • I added an "About KDE" action to the Global Menu's "Help" menu. This was present on the ActionToolBar, but not on the Global Menu.
    • I chose to put this at the bottom of the "Help" menu based on the order in the ActionToolBar, where the order is "About System Monitor" -> "About KDE", and based on what I saw in other KDE applications (KDevelop, Konsole), where again "About KDE" tends to be below "About ".
  • A couple of separators were added.
    • One separator is at the bottom of the "File" menu, to separate the "Quit" action from the other actions.
    • One separator is between "Report Bug" and the "About KDE", "About System Monitor" options.

Before

image image

After (This MR)

image image

This is probably one of the more significant MRs I have ever submitted, with some absolutely subjective choices. Please let me know if they are not desired or should be changed! Additionally, I have tested the actions and have not found any regressions, but let me know if I accidentally introduced any problems and I will be happy to investigate.

Thanks!

Merge request reports