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

Ability to show/hide menu bar #927

Closed
wants to merge 1,719 commits into from
Closed

Ability to show/hide menu bar #927

wants to merge 1,719 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 20, 2016

Hi! Just added possibility to hide menu bar. In the View menu and F12 shortcut. Tested only in Linux though.

@esbrandt
Copy link
Contributor

On Mac OSX 10.10.5 you can toggle the menu point, but nothing happens.

@daschuer
Copy link
Member

Thank you for the PR.

Please explain why the menu bar should go from you point of view.
I know that GNOME is trying to get rid of any kind of menu, while Ubuntu patches the applications to get it back.

I think it is a usability issue to hide the menu bar via mouse and need to remember that F12 is the magic key to bring it back. It should be more obvious to bring it back. It is too easy to look out.

How about a "gear" menu like old nautilus or a header bar like the new one. Maybe it should be a skin and desktop environment option, if Mixxx has a classic menu bar or some usable replacement.

@ghost
Copy link
Author

ghost commented Apr 22, 2016

Hi!

The main reason is that toolbar looks out of place, especially in fullscreen mode. Mixxx skins just don't look nice with this toolbar, it have custom interface that doesn't look good with standart qt widgets. Toolbar should be with customizable color maybe to look nice. Applying dark Qt theme is not a solution, because so many apps just made to be in light skins.

There's one dark skinned app, the Atom text editor from github. There they have also toggle menu bar option, and when menu bar is hidden you should pres Alt to reveal it again.

@uklotzde
Copy link
Contributor

Hiding the option in full screen mode would free some unused vertical space, I like the idea! I also agree that in the Deere skin it looks misplaced (see screenshot). But it should be obvious on how to re-enable it without remembering some magic key.

deere full screen with menu bar

pToggleMenuBar->setShortcut(
QKeySequence(m_pKbdConfig->getValueString(ConfigKey("[KeyboardShortcuts]",
"ViewMenu_ToggleMenuBar"),
"F12")));
Copy link
Member

Choose a reason for hiding this comment

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

It is common in other Applications like Firefox or Nemo to show the menu bar if the ALT key is pressed.
Mixxx should do the same.

@daschuer
Copy link
Member

In addition to the Alt Key, the Menu should be appear by a new Control Object.
This way the skin designer has the freedom to show the Menu, hide it because of an alternative solution or install a sensitive region on the very top of the skin, or a dedicated gear button.
If this is in place, I have no objections to merge it.

@daschuer
Copy link
Member

@dikouzmine:
This could be implemented in the same way of [PreviewDeck]", "show_previewdeck"

createVisibilityControl(pViewShowPreviewDeck, ConfigKey("[PreviewDeck]", "show_previewdeck"));

If the skin creates the control, the view menu is populated with the Action.

@ghost
Copy link
Author

ghost commented Feb 12, 2017

I've separated actions, signals and slots to separate invisible widget WMenuActionsContainer. Made MenuBar as widget controllable by skin. There is confusion with native menu bar (OS X, Unity), for now i just disabled it. Do we need it native? Also i'm having issue with new MenuBar widget: if i don't put it inside WidgetGroup or some other container, then visibility toggled on main skin container, but not on the MenuBar itself. There is also no straight way to make menu visibility toggle by Alt key, because QActions do not support this key alone. Might be possible to hack into qt message queue, but this solution don't fit into current framework so i don't know do we really need this?

@daschuer
Copy link
Member

This branch does not merge cleanly with the current master.

Maybe you should rebase you changes onto the current master.
(Make a backup first)

# if you have not already have an upstream branch  
git remote add upstream https://github.com/mixxxdj/mixxx.git
git fetch upstream 
git rebase --onto upstream/master 24c22123d25919624454e6aeb15164e60250f2b8
# resolve conflicts 
git mergetool 
git add <conflicting files>
git rebase --continue
# if you are satisfied
git push -f

@daschuer
Copy link
Member

In general we should follow the Firefox way of handling the menu bar.
In unity you cannot hid it, because it takes no extra space and is visible on mouse-over only.
I think the situation is similar for Mac Os

@Be-ing
Copy link
Contributor

Be-ing commented Apr 18, 2018

This pull request has not been updated in over a year so I am closing it to reduce the clutter of open PRs. Also, I am not convinced that this is a great way to solve the problem of having space wasted by the menu bar. I think we should move everything that is only accessible through the menu bar currently into the main UI then remove the menu bar.

@Be-ing Be-ing closed this Apr 18, 2018
@daschuer
Copy link
Member

This is IMHO a candidate for 2.1. We just need to rebase it and change the key to "Alt"

@daschuer daschuer reopened this Apr 18, 2018
@daschuer daschuer added this to the 2.2.0 milestone Apr 18, 2018
@Be-ing
Copy link
Contributor

Be-ing commented Apr 18, 2018

I am opposed to that approach. I find it annoying in Firefox, so much so that I had to search how to disable it because I accidentally hit Alt frequently. I have had Chrome users borrow my computer briefly and get confused by the sudden appearance of the menu bar.

@daschuer
Copy link
Member

Are you against hiding the menu bar or against Alt as a shortcut?

The request of hiding the menu bar comes up from time to time, so there is a valid demand.
"Alt+..." is the key for all menu shortcuts so "Alt" should work intuitive.
And it is used in Firefox and Nemo.

@Be-ing
Copy link
Contributor

Be-ing commented Apr 18, 2018

I would prefer to remove the menu bar entirely. What is the need for it? Why not move the few items that are in the menu bar into the skins? Keeping the menu bar but putting the toggle for it buried in the preferences instead of a single key that is easy to press on accident could work, but IMO there is no point in maintaining it.

@daschuer
Copy link
Member

I prefer the menu bar over the skin menu. Because this way the user find the settings in a common way of all programs. The skin menu is hard to discover for a fist time user and its look changes from skin to skin.

@rryan
Copy link
Member

rryan commented Apr 18, 2018 via email

@Be-ing
Copy link
Contributor

Be-ing commented Apr 18, 2018

this way the user find the settings in a common way of all programs.

This is not the case. More and more programs are adopting designs without menu bars. GNOME and Firefox have both abandoned menu bars.

The skin menu is hard to discover for a fist time user

Really? Have you watched a new user struggle to find it? I think the cog icon is quite clearly related to settings. Zulip uses this and I have not seen anyone complain that they cannot find the preferences.

These are the only items in the menu bar currently that are not accessible another way:

  • Rescan Library: could be moved to the library area
  • Full screen: could be put in top section of skins next to feature show/hide buttons
  • Enable Keyboard Shortcuts: move to Interface pane of preferences?
  • Help menu: maybe move to a new menu that is shown when a "?" icon is clicked

@Be-ing
Copy link
Contributor

Be-ing commented Apr 18, 2018

FWIW menu bars help with accessibility. We do have some vision-impaired users of Mixxx

I think it's a bit too broad of a statement to say that menu bars inherently help with accessibility. They are an accessible way to design a GUI, but that does not mean they are the only way. IIUC our custom skin system is really bad for accesibility.

@rryan
Copy link
Member

rryan commented Apr 19, 2018

FWIW menu bars help with accessibility. We do have some vision-impaired users of Mixxx

I think it's a bit too broad of a statement to say that menu bars inherently help with accessibility. They are an accessible way to design a GUI, but that does not mean they are the only way. IIUC our custom skin system is really bad for accesibility.

Specifically, what I mean is that the OS provides APIs to screen readers to inspect the menu tree. That won't be the case if we move all the options into the GUI (which at the moment is very hard to navigate for vision-impaired folks).

Anyway, good accessibility is often a benefit for non-impaired users too (e.g. curb-cuts). Having a uniform way to get to, e.g. the preferences menu, that matches how other apps work is a benefit.

@ronso0
Copy link
Member

ronso0 commented Apr 19, 2018

These are the only items in the menu bar currently that are not accessible another way:

Rescan Library: could be moved to the library area
Full screen: could be put in top section of skins next to feature show/hide buttons
Enable Keyboard Shortcuts: move to Interface pane of preferences?
Help menu: maybe move to a new menu that is shown when a "?" icon is clicked

and the Preferences menu with Broadcast, Record and Vinyl Control toggles.
I don't think we should squeeze those into the skin or the skin menu which is already quite extensive and lacks the keyboards navigation.

More and more programs are adopting designs without menu bars. GNOME and Firefox have both abandoned menu bars.

yeah and I personally appreciate the ability to bring back a clear menu bar with the press of a (physical) button.

@Be-ing
Copy link
Contributor

Be-ing commented May 21, 2018

Specifically, what I mean is that the OS provides APIs to screen readers to inspect the menu tree. That won't be the case if we move all the options into the GUI (which at the moment is very hard to navigate for vision-impaired folks).

What is the point of having a few menu items that do not control the music accessible to screen readers if the rest of the application is unusable for them? Those menu items do not obviate the need to make the rest of the UI accessible to screen readers. IIUC moving the few functions that are currently only available in the menu bar into the main GUI is not a step forward or backward for screen reader accessibility, it is a step sideways. If anyone wants to work on making Mixxx's skin system accessible, Qt has accessibility documentation.

Plus, to make Mixxx actually usable with a screen reader, we'd need to write documentation on how to configure the OS screen reader to play to the headphone output.

@Be-ing Be-ing removed this from the 2.2.0 milestone Jun 23, 2018
@Be-ing
Copy link
Contributor

Be-ing commented Jun 23, 2018

Re-closing this as there has been no development, there are lots of merge conflicts, and there is no consensus that this should be merged. Let's move further discussion on this topic to Zulip instead of this old pull request.

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.

10 participants