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

toggle the menubar with single Alt key press (auto hide) #11526

Merged
merged 12 commits into from
May 13, 2024

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented May 1, 2023

preferred alternative to #11304 which was already reported to work on Windows and Ubuntu.

The menubar is hidden by default and can be toggled with a single Alt key press.
This is available only on Linux and Windows. On macOS there's a permanent global menu, also in fullscreen mode.

This works only with a single Alt press, Alt bindings in custom keyboard mappings don't toggle the menubar.

Users can opt-out via the popup or in Preferences > Interface.

  • same mechanics as toggle the menubar with single Alt key press #11304 (already tested on Linux and Windows):
    setMinimumHeight() to show, setFixedHeight(0) to hide
  • show when a menu hotkey is pressed (e.g. Alt +F)
  • auto-hide when
    • all menus are closed (click action or press Esc)
    • another widget gets focused
    • fullscreen is toggled
  • show a popup after startup presenting this new option (unless the OS has a global menu and Mixxx is not fullscreen, then the dialog is shown when switching to fullscreen mode)
  • add an option in Preferences > Interface
  • add "Auto-hide menu bar" action to the View menu

image
image

The popup introduces this new feature but also allows to change the behaviour. This is supposed to be a safeguard to prevent locking out touch screen users who updated/installed Mixxx but may not have a keyboard attached when starting this new Mixxx version for the first time.

Please test!

@github-actions github-actions bot added the ui label May 1, 2023
@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key_auto-hide branch 3 times, most recently from c64366b to 00d9a3d Compare May 2, 2023 07:52
@ronso0
Copy link
Member Author

ronso0 commented May 2, 2023

I finally fixed all macOS ifndefs, this is ready for review.

If no review was started I might still force-push for polishing the UI.

@ronso0 ronso0 mentioned this pull request May 17, 2023
1 task
@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key_auto-hide branch from 00d9a3d to cbb5328 Compare May 31, 2023 20:20
@github-actions github-actions bot added the build label May 31, 2023
@ronso0 ronso0 changed the base branch from 2.4 to main May 31, 2023 20:50
@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key_auto-hide branch from cbb5328 to 8f88365 Compare June 1, 2023 12:14
@github-actions github-actions bot removed the build label Jun 1, 2023
@ronso0 ronso0 mentioned this pull request Jun 1, 2023
1 task
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

My original review comments went to the wrong PR
#11566

The following findings are pending:

  • Unnecessary popup window on targets with a global menu bar
  • glitch when closing menus after opening the menu bar by a direct shortcut Alt+V
  • Issue opening sub-sub menu when navigating with the right cursor key.

@ronso0
Copy link
Member Author

ronso0 commented Jun 8, 2023

  • Issue opening sub-sub menu when navigating with the right cursor key

This is unrelated to this PR, and expected behaviour when the top item is a submenu.
How should that be fixed anyway?

@daschuer
Copy link
Member

daschuer commented Jun 8, 2023

We may just not open the menu by default like in Firefox or move the menus with sub-menus down.

Directly opening the menu, is IMHO even more usefull if we remember the previous menu. This will remove the need for moving to a diffent menu in most cases.

@daschuer
Copy link
Member

daschuer commented Jun 8, 2023

How about adding "Show menu bar" in the view menu?

@ronso0
Copy link
Member Author

ronso0 commented Jun 8, 2023

There is an other issue, when pressing Alt-V to open the View menu and than alt gain to hide the menu bar, the menu bar disappears and reapers shot after.

Keypress handling depends on the window manager / desktop environment. Some pass the keypress to the app (e.g. Gnome), some don't (e.g. xfwm4, reason for #11566).
With xfce4, open menus don't act on Alt press.
Alt press with hidden menu bar:

  • Gnome (Ubuntu 22.04) opens the first menu
  • xfwm4 does nothing
  • in Firefox (doesn't use Qt) with xfwm4 File is highlighted, but not opened.

I can try to work around the glitch you noticed with an eventFilter() in WMainMenuBar.

Btw #11572 broke this branch. I'll rebase onto main and fix it.

@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key_auto-hide branch from a798907 to f34b29a Compare June 8, 2023 16:38
@daschuer
Copy link
Member

The glitch with Alt+V and Alt is IMHO a minor issue that does not block this PR.

If the showing the menu bar without menu is hard to do, can we than show the last menu instead of "File"?

Did you consider to move the Viny control menu down to not open it when moving right to "Help"?

@ronso0
Copy link
Member Author

ronso0 commented Jun 10, 2023

If the showing the menu bar without menu is hard to do, can we than show the last menu instead of "File"?

I'm pretty sure this behaviour is system dependent, if a menu is highlighted at all, if that is even opened... thus I'm not inclined to touch it.
You tested with Gnome, right? so please check what Alt press does in other apps (that have or allow switching to a local menu, or by temporarily disabling the global menu).

@ronso0
Copy link
Member Author

ronso0 commented Jun 10, 2023

Did you consider to move the Viny control menu down to not open it when moving right to "Help"?

This can happen in another PR. I'd prefer to merge the hide feature first.

@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key_auto-hide branch 2 times, most recently from 9e3d6b1 to d0e0c42 Compare June 10, 2023 23:55
@daschuer
Copy link
Member

Did you try to re-implement QMenuBar focus events?

I just added this with the menu bar always on and it works as if the menu bar where always there:

 void focusInEvent(QFocusEvent *pEv) override {
    	setMinimumHeight(sizeHint().height());
    	qDebug() << "focusInEvent()";
    	QMenuBar::focusInEvent(pEv);
    }

    void focusOutEvent(QFocusEvent *pEv) override {
    	qDebug() << "focusOutEvent()";
    	QMenuBar::focusOutEvent(pEv);
    	setFixedHeight(0);
    }

This solution required a lot less code and be haves natural.
Does it work for you as well?

@ronso0
Copy link
Member Author

ronso0 commented Jun 12, 2023

This is a nice idea, but it doesn't work at all with xfce4, neither when pressing Alt only nor with Alt combos.
I tried some variations but it doesn't even print the debug messages.

As I said, the menu highlight/select behaviour varies across window managers, and with xfwm4 nothing happens when I press Alt. When I press Alt combos the repsective menu is active (note: not focused until the selection is moved with Up/Down keys) but the menubar is not addressed.

@daschuer
Copy link
Member

OK than it looks like this is a bit hacky because of xfce4. Can you put a warning that we must only touch the code again when testing with xfce4?
We may also consider to use the open menu workaround only for xfce4. This will fix the glitch when closing the menu with Alt. But I don't want to put to much extra work on you.

@ronso0
Copy link
Member Author

ronso0 commented Jun 12, 2023

Correct me if I'm wrong, we tested only with Gnome (mutter) and xfce (xfwm4) so far (and Windows 10), but there are a few more window managers out there ;) so I guess testing with xfwm4 revealed issues that may occur anywhere else.
That being said

We may also consider to use the open menu workaround only for xfce4.

I think we may not apply the open window trick on Gnome mutter, though it's difficult to detect the window manager.

Anyhow, I'll test some other distros/WMs, for example KDE Plasma, then we can decide how to proceed.

@daschuer
Copy link
Member

Sounds good. At a bare minimum, we need some comments about the issues and workaround with xfce4 to protect it from removal.

@Russe
Copy link
Contributor

Russe commented Dec 17, 2023

I'm interested in this feature, can I do some help with testing?
Currently I'm using Ubuntu 22.04 with XFCE xfwm4 and most recent Mixxx beta.

@ronso0
Copy link
Member Author

ronso0 commented Dec 17, 2023

Sure, feedback is welcome, everything should work as described in the first post.
Though, I don't expect any surprises since your also using xfce.

@ronso0
Copy link
Member Author

ronso0 commented Apr 18, 2024

I've been testing this for months now, no issues whatsoever (inkl. dev-mod, i.e. skin reload without --developer).

Is anyone interested to get this into 2.5?

@daschuer
Copy link
Member

daschuer commented May 8, 2024

Is anyone interested to get this into 2.5?

Yes, unfortunately a conflict has developed.

@daschuer
Copy link
Member

Thank you. LGTM

@daschuer daschuer merged commit a480e28 into mixxxdj:main May 13, 2024
13 checks passed
@ronso0 ronso0 deleted the menubar-toggle-with-Alt-key_auto-hide branch May 13, 2024 07:38
@ronso0
Copy link
Member Author

ronso0 commented May 13, 2024

Unfortunately it's not quite working anymore as designed:
the auto-hide confirm dialog also pops up when I leave fullscreen (and still have "Ask me again" checked).

I'll take a look.

edit: got it, wrongly resolved the conflicts after #11566 had been merged.
Though I need to wrap my head around what I actually did hear, it's been a while 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hide menu in full screen mode
4 participants