Skip to content

Fix #15473: No platform menu bar support on Linux #15826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Jan 20, 2023
Merged

Fix #15473: No platform menu bar support on Linux #15826

merged 8 commits into from
Jan 20, 2023

Conversation

adazem009
Copy link
Contributor

@adazem009 adazem009 commented Jan 11, 2023 •
edited
Loading

Resolves: #15473

Native menu bar support is only implemented for macOS. This pull request contains changes that will enable it on Linux-based systems as well (for example the global menu bar in KDE Plasma).
Uses D-Bus (com.canonical.AppMenu.Registrar) to check whether global menu is available.

I've tested this in KDE Plasma and it properly detects whether global menu is enabled.

Global menu bar

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

cbjeukendrup
cbjeukendrup reviewed Jan 11, 2023
View reviewed changes
Copy link
Member

@cbjeukendrup cbjeukendrup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for you contribution! A few comments.

@adazem009 adazem009 force-pushed the master branch 2 times, most recently from e0e676a to 2a153f5 Compare January 12, 2023 20:04
cbjeukendrup
cbjeukendrup reviewed Jan 13, 2023
View reviewed changes
igorkorsukov
igorkorsukov reviewed Jan 13, 2023
View reviewed changes
@adazem009 adazem009 marked this pull request as draft January 13, 2023 22:11
@adazem009
fix musescore#15473: Register AppMenuModel as PlatformAppMenuModel on…
161adda 
… Linux

Both AppMenuModel and NavigableAppMenuModel has to be exposed to QML on Linux
to allow GUI menu bar and platform menu bar support.
@adazem009
fix musescore#15473: Add Qt D-Bus module (on Linux)
f21df85 
Required for checking if global menu bar is available.
@adazem009
fix musescore#15473: Add method for global menu check to platform theme
dd429ee
@adazem009
fix musescore#15473: Add Linux platform theme
f3de883 
Currently provides isGlobalMenuAvailable() method that uses DBus to check whether it is enabled
@adazem009
fix musescore#15473: Add global menu check method to AppMenuModel
ef60a3d
@adazem009
fix musescore#15473: Move macOS platform menu to separate component
56e2154
@adazem009
fix musescore#15473: Add support for platform menu bar on Linux
e9d1043 
Uses Loader components to make only one of the menu bars active.
@adazem009 adazem009 marked this pull request as ready for review January 13, 2023 22:59
@adazem009 adazem009 requested review from cbjeukendrup and igorkorsukov and removed request for cbjeukendrup January 13, 2023 22:59
@igorkorsukov
Copy link
Contributor

@adazem009 Great!

In order for the unit tests to be successful, could you please update with the master

@cbjeukendrup
Copy link
Member

cbjeukendrup commented Jan 14, 2023 •
edited
Loading

The unit test failure is also on the latest master branch, but only sometimes. I still need to clean that up...

I've restarted the check for now, let's see whether it passes this time :)

@adazem009
fix musescore#15473: Register AppMenuModel as PlatformAppMenuModel on…
47315f0 
… macOS

Using AppMenuModel for GUI menu bar and PlatformAppMenuModel for platform menu bar makes more sense when both menu bar types are possible on a platform
@adazem009
Copy link
Contributor Author

@adazem009 Great!

In order for the unit tests to be successful, could you please update with the master

Unit tests are successful now.

@igorkorsukov
Copy link
Contributor

@cbjeukendrup do you have any other suggestions? or is it ready for merge? :)

@cbjeukendrup cbjeukendrup self-requested a review January 16, 2023 08:51
@adazem009
Copy link
Contributor Author

@cbjeukendrup do you have any other suggestions? or is it ready for merge? :)

It's ready for merge. I've already tested it on Windows and macOS just to be sure that it doesn't break anything.

@cbjeukendrup
Copy link
Member

It's probably okay, but I'll quickly check it later today!

@DmitryArefiev
Copy link
Contributor

@cbjeukendrup I don't have KDE Plasma. I have only Ubuntu and Mint and as I know it's not easy to enable Global menu there..

I've tested this PR on Win10,Mac13 and Linux Mint and didn't found any differences with master. So, if it's safe we can merge it to master.

@cbjeukendrup
Copy link
Member

Thanks for checking! In that case I'll merge it.

@cbjeukendrup cbjeukendrup merged commit 0bd5c09 into musescore:master Jan 20, 2023
@DmitryArefiev
Copy link
Contributor

Great. Thanks!

@DmitryArefiev DmitryArefiev mentioned this pull request Jan 27, 2023
Closed
@Eism Eism mentioned this pull request Jul 17, 2023
Merged
8 tasks
This was referenced Aug 19, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igorkorsukov igorkorsukov igorkorsukov approved these changes

@cbjeukendrup cbjeukendrup cbjeukendrup approved these changes

Assignees

@DmitryArefiev DmitryArefiev

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[MU4 Issue] Menu bar Doesn't behave like a normal menu bar
4 participants
@adazem009 @igorkorsukov @cbjeukendrup @DmitryArefiev