Skip to content
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.

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

allow to hide the menu bar with [Controls],show_menubar #3184

Closed
wants to merge 2 commits into from

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Oct 17, 2020

supersede #927
https://bugs.launchpad.net/mixxx/+bug/1703777

Allows to hide the main menubar with [Controls],show_menubar.
I added a toolbar button to all skins except Shade. State is not persistent across restarts.
image

All keyboard shortcuts work regardless of the show/hide state.
Menus can be opened as before (like Alt + F for File menu).

ToDo

  • ifdef APPLE: what happens to macOS global menu when hiding?
  • hide the skin pushbutton via stylesheet if hiding isn't allowed in OS

Nice to have

  • toggle menubar by pressing Alt
  • bind to fullscreen?

src/widget/wmainmenubar.cpp Outdated Show resolved Hide resolved

void WMainMenuBar::showHideMenuBar(double v) {
if (v > 0) {
int minHeight = sizeHint().height();
Copy link
Member Author

Choose a reason for hiding this comment

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

Does it make sense to read this after initialization, or keep it here in case fonts or decoration or whatever is changed by OS measures while Mixxx runs?

@ronso0 ronso0 requested a review from daschuer October 18, 2020 01:04
@poelzi
Copy link
Contributor

poelzi commented Oct 19, 2020

I would incoperate this changes into #3189 if this approach gets greenlight, since I need this here next

@ronso0
Copy link
Member Author

ronso0 commented Oct 19, 2020

I would incoperate this changes into #3189 if this approach gets greenlight, since I need this here next

We need testing on all supported OS first, especially macOS with the global menu.

src/widget/wmainmenubar.cpp Outdated Show resolved Hide resolved
src/widget/wmainmenubar.h Outdated Show resolved Hide resolved
@ronso0
Copy link
Member Author

ronso0 commented Oct 19, 2020

I'll address the comments.

On the scenario:
Do we want to make the menubar always optional or automatically hide it (and show the button) when in fullscreen mode?

@Holzhaus
Copy link
Member

I would say always optional (maybe add a skin flag, so that old skins that don't have the button keep working).

@ronso0
Copy link
Member Author

ronso0 commented Oct 19, 2020

Thinking about it, always optional is better as the user has to toggle the button to hide the menu and therefore implicitely knows how to recall it.

@ronso0
Copy link
Member Author

ronso0 commented Oct 19, 2020

rebased with comments addressed, m_bShowMenuBar is now a parented pointer.

The button is alway in the same place, also close to where the menu items are be shown.
When switching to Shade the menubar is enforced.
image

@poelzi
Copy link
Contributor

poelzi commented Oct 20, 2020

@ronso0 I'm for #3189 instead of adding a button to show/hide the menubar. There is only one bug left that toggeling fullsceen recreates the menubar and therefor destroys the old association.

@daschuer
Copy link
Member

daschuer commented Oct 20, 2020

We now have two similar menu buttons in the skins one for the application menu and one for the skin menu.

That is OK looking at the legacy, but feels unintuitive. Do we have an idea to improve it?

We may fix the shrink issue that makes the "skin preview" during setting the skin option void in turn.

I have in mind to integrate the skin menu as pop up widget into the view menu of the menu bar.
This way we have two clicks more to reach it on the down side, but screen reader support and a "real" preview on the up side.

What do you think? What are alternatives?

@daschuer
Copy link
Member

On Ubuntu Bionic + Gnome, the Menu button stops working in full-screen mode and does not work again after leaving it.

Some issues are logged:

Warning [Controller]: USB permissions problem (or device error.) Your account needs write access to USB HID controllers.
Critical [Main]: DEBUG ASSERT: "!pCreatorCO" in function static QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl(const ConfigKey&, ControlFlags, ControlObject*, bool, bool, bool, double) at /home/daniel/workspace/advanced_autodj_2/src/control/control.cpp:118
Warning [Main]: ControlObject "[Controls]" "show_menubar" already created
Critical [Main]: DEBUG ASSERT: "!pCreatorCO" in function static QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl(const ConfigKey&, ControlFlags, ControlObject*, bool, bool, bool, double) at /home/daniel/workspace/advanced_autodj_2/src/control/control.cpp:118
Warning [Main]: ControlObject "[Controls]" "show_menubar" already created
Warning [LibraryScanner 1]: QSqlDatabasePrivate::removeDatabase: connection 'MIXXX-2' is still in use, all queries will cease to work.

@daschuer
Copy link
Member

All Gnome based desktops suffer the issue of the broken button after full screen.
Unity works as expected. The Menu button is always out of work.
We may consider to make it work in full screen and hid it in windowed mode, where the menu has been moved to the global menu like in Mac OS.

@ronso0
Copy link
Member Author

ronso0 commented Oct 21, 2020

We may consider to make it work in full screen and hid it in windowed mode, where the menu has been moved to the global menu like in Mac OS.

Okay, thanks for testing.
Do the shortcuts work there and can you access the menus (for example Ctr+F) when the menu is hidden?
That what be good to know for whether I continue following this route or not.

I'll try to hide it in fullscreen only, regardless of the skin. Maybe I also get an eventFilter working that catches single-pressing Alt to show it again.

@daschuer
Copy link
Member

In Unity, I have the issue to have no Menu bar when starting if full screen the first time.
In windowed mode ALT+O and Crtl+P works.
In Full screen mode ALT+O does not work but Crtl+P still works.

@daschuer
Copy link
Member

In Full screen mode ALT+O does not work but Crtl+P still works.
This is the case if there is no menu bar visible when starting if fullscreen mode and when the menu is visible.

@daschuer
Copy link
Member

With Gnome, unlike Unity, I see a menu bar starting in full screen mode.
Unfortunately ALT+O is still broken, Ctrl+P works.

@Be-ing Be-ing changed the base branch from master to main October 23, 2020 23:05
@ronso0
Copy link
Member Author

ronso0 commented Nov 5, 2020

Closing this. Happy to have it in my personal branch until #3189 is merged :)

@ronso0 ronso0 closed this Nov 5, 2020
@ronso0 ronso0 deleted the hide-menubar branch November 5, 2020 01:12
@ronso0 ronso0 restored the hide-menubar branch November 10, 2021 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants