Qt: Overhaul menubar #5794

spycrab · 2017-07-16T00:39:55Z

Improves the menubar in several ways:

Add ... to items when appropriate
Add missing keyboard shortcuts (mainly for Open... and Exit)

spycrab · 2017-07-16T00:40:20Z

@ligfx @MayImilae @Starsam80

MayImilae · 2017-07-16T00:40:36Z

Adds icons to entries?

spycrab · 2017-07-16T00:41:36Z

@MayImilae
e.g. a little file icon next to Open...

MayImilae · 2017-07-16T00:43:55Z

That seems a bit unnecessary, imo. The menu bar isn't really conducive to icons, because of how small it is. (It's designed around text)

To be clear, there's nothing wrong with it, just, it's not a good space for icons, and it's more icons that have to be made!

Starsam80 · 2017-07-16T03:16:25Z

The icons don't seem to appear on windows:

Also, while you are at it, can you also add separators to the Emulation menu? This is what WX looks like:

and Qt looks like this:

ligfx

Keyboard shortcuts are good!

On the topic of icons: they don't work on macOS (possibly intentional by Qt, possibly because the icons don't exist), which is good because the HIG discourages it. Do they show up on Windows?

Source/Core/DolphinQt2/MenuBar.cpp

-  m_perform_online_update_menu = tools_menu->addMenu(tr("Perform Online System Update"));
+  m_perform_online_update_menu =
+      tools_menu->addMenu(QIcon::fromTheme(QStringLiteral("system-software-update")),
+                          tr("Perform Online System Update..."));


ligfx · 2017-07-16T03:20:11Z

@Starsam80 you answered my question before I asked it! If the icons only work on Linux, I'd say it's probably not worth the extra code.

Starsam80 · 2017-07-16T03:20:22Z

@ligfx No, they do not appear on windows. Quoting the Qt docs:

https://doc.qt.io/qt-5/qicon.html#fromTheme

Note: By default, only X11 will support themed icons. In order to use themed icons on Mac and Windows, you will have to bundle a compliant theme in one of your themeSearchPaths() and set the appropriate themeName().

I don't think the icons are worth it

spycrab · 2017-07-16T11:46:13Z

@Starsam80 Done.

Now using Dolphin's icons only. Also reduced the amount of icons used due to the fact that our iconset is rather limited.

ligfx · 2017-07-16T16:20:44Z

Ah, sorry, I wasn't clear before: menu icons should not be displayed on macOS.

ligfx

Okay, I think the icons shouldn't be shown on Windows, either. I poked around a Windows 10 install, and I couldn't find any applications that used icons in their menus. First-party apps and large third-party applications like Spotify and Atom don't use them. Dolphin should be consistent.

Also, the fullscreen icon metaphor isn't understandable at such a small size, and it's unclear to me whether the screenshot one is. We all know it's a camera, but, with the loss of higher-definition details, would a new user recognize what a rectangle with a ring in the middle and a dot in one corner means?

Source/Core/DolphinQt2/MenuBar.cpp

@@ -9,6 +9,8 @@
 #include <QMessageBox>
 #include <QUrl>

+#include <iostream>


MayImilae · 2017-07-19T10:28:40Z

Are the icons still in the menu bar on the current version of this?

spycrab · 2017-07-19T10:29:34Z

@MayImilae No, I removed all icons.

MayImilae

In that case, everything's good for me!

Source/Core/DolphinQt2/MenuBar.cpp

@@ -17,6 +17,7 @@
 #include "DolphinQt2/AboutDialog.h"
 #include "DolphinQt2/GameList/GameFile.h"
 #include "DolphinQt2/MenuBar.h"
+#include "DolphinQt2/Resources.h"


Source/Core/DolphinQt2/MenuBar.cpp

-  m_open_action = file_menu->addAction(tr("Open"), this, SIGNAL(Open()));
-  m_exit_action = file_menu->addAction(tr("Exit"), this, SIGNAL(Exit()));
+  m_open_action = file_menu->addAction(tr("Open..."), this, SIGNAL(Open()),
+                                       QKeySequence(QStringLiteral("Ctrl+O")));


Source/Core/DolphinQt2/MenuBar.cpp

-  m_state_save_menu->addAction(tr("Save State to Oldest Slot"), this, &MenuBar::StateSaveOldest);
+  m_state_save_menu->addAction(tr("Save State to File..."), this, SIGNAL(StateSave()));
+  m_state_save_menu->addAction(tr("Save State to Selected Slot"), this, SIGNAL(StateSaveSlot()));
+  m_state_save_menu->addAction(tr("Save State to Oldest Slot"), this, SIGNAL(StateSaveOldest()));


ligfx reviewed Jul 16, 2017

View reviewed changes

spycrab force-pushed the qt_menubar branch from 06c60a4 to 3fb99cc Compare July 16, 2017 11:40

spycrab force-pushed the qt_menubar branch from 3fb99cc to 8f4dff3 Compare July 16, 2017 16:26

ligfx reviewed Jul 16, 2017

View reviewed changes

Source/Core/DolphinQt2/MenuBar.cpp Outdated

@@ -9,6 +9,8 @@

#include <QMessageBox>

#include <QUrl>

#include <iostream>

This comment was marked as off-topic.

Sign in to view

spycrab force-pushed the qt_menubar branch from 8f4dff3 to 71debab Compare July 17, 2017 11:19

spycrab force-pushed the qt_menubar branch from 71debab to 7c56331 Compare July 19, 2017 10:30

MayImilae approved these changes Jul 19, 2017

View reviewed changes

Starsam80 approved these changes Jul 21, 2017

View reviewed changes

ligfx reviewed Jul 21, 2017

View reviewed changes

Source/Core/DolphinQt2/MenuBar.cpp Outdated

@@ -17,6 +17,7 @@

#include "DolphinQt2/AboutDialog.h"

#include "DolphinQt2/GameList/GameFile.h"

#include "DolphinQt2/MenuBar.h"

#include "DolphinQt2/Resources.h"

This comment was marked as off-topic.

Sign in to view

spycrab force-pushed the qt_menubar branch 2 times, most recently from fc81610 to 886b327 Compare July 23, 2017 08:40

ligfx reviewed Jul 25, 2017

View reviewed changes

spycrab force-pushed the qt_menubar branch from 886b327 to 2ea5648 Compare July 26, 2017 20:01

ligfx suggested changes Jul 26, 2017

View reviewed changes

spycrab force-pushed the qt_menubar branch 4 times, most recently from 43d3df4 to 6ddb8b9 Compare July 29, 2017 18:35

Qt: Overhaul menubar

b3e93c5

spycrab force-pushed the qt_menubar branch from 6ddb8b9 to b3e93c5 Compare July 29, 2017 19:19

leoetlino merged commit 9bab7ff into dolphin-emu:master Aug 4, 2017

leoetlino modified the milestone: Qt Sep 13, 2017

spycrab deleted the qt_menubar branch April 14, 2018 11:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Qt: Overhaul menubar #5794

Qt: Overhaul menubar #5794

spycrab commented Jul 16, 2017 •

edited

spycrab commented Jul 16, 2017

MayImilae commented Jul 16, 2017

spycrab commented Jul 16, 2017

MayImilae commented Jul 16, 2017 •

edited

Starsam80 commented Jul 16, 2017

ligfx left a comment

This comment was marked as off-topic.

ligfx commented Jul 16, 2017

Starsam80 commented Jul 16, 2017

spycrab commented Jul 16, 2017

ligfx commented Jul 16, 2017

ligfx left a comment

This comment was marked as off-topic.

MayImilae commented Jul 19, 2017

spycrab commented Jul 19, 2017

MayImilae left a comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

Qt: Overhaul menubar #5794

Qt: Overhaul menubar #5794

Conversation

spycrab commented Jul 16, 2017 • edited

spycrab commented Jul 16, 2017

MayImilae commented Jul 16, 2017

spycrab commented Jul 16, 2017

MayImilae commented Jul 16, 2017 • edited

Starsam80 commented Jul 16, 2017

ligfx left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

ligfx commented Jul 16, 2017

Starsam80 commented Jul 16, 2017

spycrab commented Jul 16, 2017

ligfx commented Jul 16, 2017

ligfx left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

MayImilae commented Jul 19, 2017

spycrab commented Jul 19, 2017

MayImilae left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

spycrab commented Jul 16, 2017 •

edited

MayImilae commented Jul 16, 2017 •

edited