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 #11304

Closed
wants to merge 10 commits into from

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Feb 24, 2023

I decided to make another attempt to hide the menubar, this time by filtering a single Alt keypress + the essentials from #3184, but without the ControlObject and the skin button.
This is not available on macOS since there's a permanent global menu, also in fullcreen mode

I decided to require a single Alt press so Alt bindings in custom keyboard mappings don't toggle the menubar.

TODO

  • single-press Alt to show or hide the menubar (press duration doesn't matter, only that there's no other key pressed between Alt press and release)
  • hide the menu when going fullscreen 1
  • unhide the menubar if a menu hotkey (like Alt+F) is pressed while the menu is hidden 2
  • add a label / dummy action to View menu "Toggle menu bar with Alt key"
    that action is not clickable (or does nothing respectively), so users have to press Alt themselves
  • show a hint when going fullscreen
    image
    I wonder if that should be omitted on macOS altogether?
  • style hint for all skins
  • test
  • test
  • test

TESTING (only done on *ubuntu so far)
I added debug output for the keystroke filtering, so in case something is not working as expected please run from the command line with --developer and share the relevant output snippets so we can debug it.

Closed #8902

Footnotes

  1. for me this didn't work when I used the fullscreen hotkey (at least in debug builds I tested so far) because the window manager (xfwm4) hotkey and the Mixxx hotkey were identical, and the keystroke was consumed by the window manager.
    I have a workaround 167c569, though that's not applicable to linux distros with a global menu that require the current menubar hack, but for those hiding the menubar isn't available anyway 🤷 Maybe if my workaround is considered mergable I'll try to polish it so it replaces the hook to slotViewFullScreen on demand.

  2. I tested with Ubuntu Studio 20.04 (xfce desktop) (menus open immediately) and Ubuntu 22.04 (Gnome) (a second hotkey press is required to unroll the menu)

@github-actions github-actions bot added the ui label Feb 24, 2023
@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key branch from 0c74aec to 801c55c Compare February 24, 2023 22:35
@ronso0 ronso0 marked this pull request as draft February 24, 2023 22:43
@ronso0 ronso0 marked this pull request as ready for review February 27, 2023 22:33
@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key branch from 801c55c to 075b7aa Compare February 28, 2023 21:50
@github-actions github-actions bot added the skins label Feb 28, 2023
@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key branch from 075b7aa to 8eee847 Compare February 28, 2023 22:03
@ronso0 ronso0 marked this pull request as draft February 28, 2023 23:39
@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key branch from 8eee847 to c5c992f Compare February 28, 2023 23:45
@ronso0
Copy link
Member Author

ronso0 commented Mar 2, 2023

I tested with Ubuntu Studio 20.04 (xfce desktop) (menus open immediately) and Ubuntu 22.04 (Gnome) (a second hotkey press is required to unroll the menu) leftwards_arrow_with_hook

Somehow this broke after a recent update #11321
Fix is included in #11313 1a4d335

@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key branch 6 times, most recently from 67ef039 to a8f2138 Compare March 9, 2023 00:12
@ronso0
Copy link
Member Author

ronso0 commented Mar 9, 2023

unhide the menubar if a menu hotkey (like Alt+F) is pressed while the menu is hidden

Now this works flawlessly on both 20.04 with xfce and 22.04 with Unity.

@ronso0
Copy link
Member Author

ronso0 commented Mar 9, 2023

show a hint when going fullscreen

Automatically focusing the Okay button does not work on all platforms, yet.

@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key branch from a8f2138 to 4e74af1 Compare March 9, 2023 21:58
@ronso0 ronso0 marked this pull request as ready for review March 12, 2023 00:13
@ronso0
Copy link
Member Author

ronso0 commented Mar 12, 2023

I've been using this + #11313 without and have any issues (Ubuntu 20.04 with vala global menu).
I'd appreciate if someone on Windows could give this a spin, before I find time to reactivate/fix my Win10 partition. @JoergAtGithub maybe?

edit macOS testing would be nice, too, but the changes don't shouldn't affect macOS at all.

@daschuer
Copy link
Member

Works good here on Ubuntu Focal. The popup window can be improved for keyboard interactions.
I think it should be "Ok" instead of "Okay" like in other dialogs. You may also consider to enable Alt Shortcuts "Alt+o"

@JoergAtGithub
Copy link
Member

I did a short test on Windows 11 and it works as expected!
My personal favorite would be to show a hamburger button for the menu as in #3189, but this is of cause beyond the scope of this PR.

@ronso0
Copy link
Member Author

ronso0 commented Mar 13, 2023

Great, thanks for testing!

The popup window can be improved for keyboard interactions.
I think it should be "Ok" instead of "Okay" like in other dialogs.

Yeah, I still have to polish the dialog.

You may also consider to enable Alt Shortcuts "Alt+o"

I don't understand... Alt+F is just an example, here all keys work and unhide the menu bar.

My personal favorite would be to show a hamburger button for the menu as in #3189, but this is of cause beyond the scope of this PR.

Yes, entirely.
Also, I intend to get this merged into 2.4, not 4.2 ; )

@daschuer
Copy link
Member

You may also consider to enable Alt Shortcuts "Alt+o"

I don't understand... Alt+F is just an example, here all keys work and unhide the menu bar.

It is just that in case of a standard QMessageBox, the O if OK is underlined for pressing the button via Alt+O.

@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key branch from 4e74af1 to 140e581 Compare March 13, 2023 21:56
@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key branch from 140e581 to 232b8c1 Compare March 14, 2023 00:22
@ronso0 ronso0 linked an issue Mar 23, 2023 that may be closed by this pull request
@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key branch 2 times, most recently from 23fc9ce to 74751ec Compare April 5, 2023 19:27
@ronso0
Copy link
Member Author

ronso0 commented Apr 8, 2023

(responding to @daschuer's review comment in #11313 )

I have just tested this a bit, with some results:

  • The Menu bar disappears when going to fullscreen, which is OK, but it does not com back on window mode, which was my expectation. Maybe we should store the menu bar visible state for both independently. Default: Windowed = menu bar visible; Full-screen = menu bar hidden.

Users are notified how to toggle the menubar when going fullscreen, so they should know how to get it back in windowed mode. But indeed this is inconsistent with showing the menubar on each (windowed) start.
So yes, if someone always wants the menubar in windowed or fullscreen mode we could store the state.

Anyway, allowing to toggle the menubar is a significant change and we may make a step backwards and discuss the implementation options and how to let as many users as possible benefit from it.

  1. We could also hide the menubar by default on each start and only show it on demand (state is not stored).
    This is actually my preferred solution, though it's not discoverable how to show the menubar. We may expect some users know this from Firefox / Thunderbird / SublimeText (actually I'm only aware of these Mozilla apps) / ... , but I suspect there'll be many support request "Where's the menubar???"
    So the question is how to make the toggle discoverable. A popup after upgrade would be okay for me (with the same 'Remind me again' checkbox).
  2. Show it by default, show the Alt key hint when going fullscreen + the Alt hint in the View menu
    I think that way many users will miss it, e.g. if they are upgrading and are already somewhat used to the interface they won't dicscover the hint in the View menu. If they never use Mixxx in fullscreen mode (FullHD screens don't really require it) they'll also never see the popup. Then hiding the menubar will be like a hidden feature, even though many users may enjoy it.
  • "Enter" is still not equal pressing Ok

So the Okay button is not focused when the hint pops up?
Should Enter close the dialog even if the checkbox has focus?

@ronso0
Copy link
Member Author

ronso0 commented Apr 9, 2023

The more I think about it I notice I prefer the way it's done in Firefox:

  • menubar always hidden by default (fullscreen and window)
  • press Alt to show the menubar and focus the first menu
  • press menu hotkey to show the menubar and focus the requested menu
  • hide menubar when it loses focus (menu action clicked || escape pressed || focused something else)

+

  • show a popup after start to explain the new feature, incl. "Remind me again" checkbox

I'll test this in another branch so the different approaches can be tested easily.

@ronso0 ronso0 marked this pull request as draft May 1, 2023 16:18
@ronso0 ronso0 mentioned this pull request May 17, 2023
1 task
@ronso0 ronso0 force-pushed the menubar-toggle-with-Alt-key branch from 74751ec to af2cfcb 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:51
@ronso0
Copy link
Member Author

ronso0 commented Jun 1, 2023

I'll close this one in favor of #11526
I messed something up (or rather wrong git rerere) during rebase onto main, and I prefer auto-hide anyway.

@ronso0 ronso0 closed this Jun 1, 2023
@ronso0 ronso0 deleted the menubar-toggle-with-Alt-key branch August 21, 2023 22:31
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.

3 participants