MuseScore
MuseScore copied to clipboard
Fix #18014: Cloud score missing label in open recent menu
Resolves: #18014
- [x] I signed the CLA
- [x] The title of the PR describes the problem it addresses
- [x] Each commit's message describes its purpose and effects, and references the issue it resolves
- [x] If changes are extensive, there is a sequence of easily reviewable commits
- [x] The code in the PR follows the coding rules
- [x] There are no unnecessary changes
- [x] The code compiles and runs on my machine, preferably after each commit individually
- [x] I created a unit test or vtest to verify the changes I made (if applicable)
@shoogle Hi Peter, updated the PR with the requested changes
@nasehim7, thanks! I'm not seeing any difference on Windows, but that could just mean that the menu font doesn't have a glyph for the emoji so it falls back to using the text character glyph instead.
It would be interesting to see what these look like on other platforms.
I see, on MacOS it looks like below
Ok, thanks! The emoji on macOS looks much better, which good because we can't control the fonts in the macOS menus.
On Windows, the cloud looks rubbish, but we control the fonts in the menus on Windows, so we could use a different font for the icon, or modify the current font to include the emoji glyph.
Could you take a screenshot of the menus in macOS Light Theme?
Ah I see. So if we decide on the font change for windows that change should also go in with this PR, right?
Yes, here is the light theme screenshot:
On non-Mac, why not just using the icon from the icon font, by setting the iconCode
of the action
?
The solution with unicode characters was only invented as a workaround for macOS, which uses the native menu bar and it's too much effort to show icons from the icon font there.
@cbjeukendrup, good point!
Can someone with macOS take a screenshot of the emoji when the menu text is dark, like in this one. You might need to put MuseScore in its dark theme or change your desktop background in order to change the menu text colour.
Here it is
I suspect It is inconsistent due to the variant selector
@cbjeukendrup What you suggested would loosely boil down to something like this:
if (isCloud) {
#if defined(Q_OS_WIN) || defined(Q_OS_LINUX)
action.iconCode = IconCode::Code::CLOUD;
#elif defined(Q_OS_MAC)
action.title = TranslatableString::untranslatable("\u2601\uFE0F " + file.displayName(/*includingExtension*/ true));
#endif
}
is it?
@nasehim7 it's the right approach but no need to detect Windows and Linux explicitly. You can do #if defined(Q_OS_MAC) ... #else ... #endif
. And you'll still need to set the action.title
on non-macOS platforms, just exclude the emoji character.
I suspect It is inconsistent due to the variant selector
Adobe was able to make the nicely shaped cloud black when the text is black, so it looks like we still haven't found the right approach yet for macOS.
Please add all of these characters with the current code layout so we can see how these look in the macOS menu:
☁\u2601
cloudy weather
🌧\u1F327
cloud with rain
⛅\u26C5
sun behind cloud
⛈\u26C8
thunder cloud
🌩\u1F329
cloud with lightning
Also add the line action.iconCode = IconCode::Code::CLOUD;
so we can compare to our own cloud icon. It won't do anything on macOS but will affect the other platforms.
if (isCloud) {
action.iconCode = IconCode::Code::CLOUD;
action.title = TranslatableString::untranslatable("\u2601\uFE0F\u1F327\uFE0F\u26C5\uFE0F\u26C8\uFE0F\u1F329\uFE0F " + file.displayName(/*includingExtension*/ true));
} else {
action.title = TranslatableString::untranslatable("\u2601\uFE0E\u1F327\uFE0E\u26C5\uFE0E\u26C8\uFE0E\u1F329\uFE0E " + file.displayName(/*includingExtension*/ true));
}
@shoogle Yes that makes sense. I updated the code and checked the output, 2nd and 5th text versions are not getting rendered properly. Here is the screenshot:
P.S.: while we are finalising for MacOS, I added completed code for other platforms so that they can be tested as well.
Adobe was able to make the nicely shaped cloud black when the text is black, so it looks like we still haven't found the right approach yet for macOS.
Adobe probably doesn't use a text character, but uses the proper macOS APIs to add an icon to a menu item. Either they use these APIs directly, or they use them via whatever UI framework they use.
It seems we could also achieve that using the icon
property of QML's MenuItem: https://doc.qt.io/qt-6.2/qml-qt-labs-platform-menuitem.html#icon-prop
But that's still not trivial, because QML uses a very different format for icons than we do. In MuseScore, all icons are actually characters from a font, and can be accessed using the IconCode
enum. But Qt expects a URL where a SVG or PNG or so can be found.
I'm not sure yet if there is an elegant way around that. The non-elegant way would be to put a SVG version of all icons in some folder, but that's basically out of question.
What may or may not make the challenge bigger is that on macOS we would only want cloud icons for the 'open recent' list, and not all action icons that can be seen in the menus on Windows and Linux.
It sounds like we just need an SVG version of the cloud icon, or has someone requested that we show other icons too? If they have, there are tools that can convert fonts to SVG (and then to PNG if necessary), but we should probably verify the method works with the cloud SVG first before we go about converting an entire font.
For now, it has indeed only been requested for the cloud icons. But having exceptions does not necessarily make things easier. Anyway, afaics, the most straightforward solution would be to add yet another field to MenuItem
, namely nativeMenuBarIconPath
, which we'll use in this situation only.
Adobe was able to make the nicely shaped cloud black when the text is black, so it looks like we still haven't found the right approach yet for macOS.
Adobe probably doesn't use a text character, but uses the proper macOS APIs to add an icon to a menu item. Either they use these APIs directly, or they use them via whatever UI framework they use.
It seems we could also achieve that using the
icon
property of QML's MenuItem: https://doc.qt.io/qt-6.2/qml-qt-labs-platform-menuitem.html#icon-prop But that's still not trivial, because QML uses a very different format for icons than we do. In MuseScore, all icons are actually characters from a font, and can be accessed using theIconCode
enum. But Qt expects a URL where a SVG or PNG or so can be found. I'm not sure yet if there is an elegant way around that. The non-elegant way would be to put a SVG version of all icons in some folder, but that's basically out of question.What may or may not make the challenge bigger is that on macOS we would only want cloud icons for the 'open recent' list, and not all action icons that can be seen in the menus on Windows and Linux.
Incase you didn't see i commented about Adobe using the same icon on Windows version. If we need a SVG for a cloud icon for MS4, lemme know, I am happy to provide easily.
@jessjwilliamson On Windows we would indeed use the same icon but there it can be done trivially via the icons font, because there we have our custom menu bar. In order to get the icon into the macOS menu bar too (and also into the native menubar on Linux OSs that support it), it looks indeed like having an SVG version would be great.
@jessjwilliamson On Windows we would indeed use the same icon but there it can be done trivially via the icons font, because there we have our custom menu bar. In order to get the icon into the macOS menu bar too (and also into the native menubar on Linux OSs that support it), it looks indeed like having an SVG version would be great.
That's not a problem, I'll get a SVG done soon :)
I thought i had to design one but there's a perfect one in our font. Just made it an SVG.
@cbjeukendrup Here I am summarising the potential approach that can be taken to fix this:
- Add
nativeMenuBarIconPath
member to MenuItem with default value as empty String. Add it's getter, setters. - We can put the
cloud_icon.svg
in appshell ->qrc:/qml/resources/cloud_icon.svg
(needs confirmation) - Need to add resource path to appshell.qrc, I suppose.
- In the
appmenumodel.cpp
we should do something like this -
if (isCloud) {
#ifdef Q_OS_MAC
item->setNativeMenuBarIconPath("qrc:/qml/resources/cloud_icon.svg");
#else
action.iconCode = IconCode::Code::CLOUD;
#endif
}
the action.title
will set afterwards
5. In PlatformMenuBar.qml
of MacOS, we will update the makeMenuItem()
method to have -
if (itemInfo.nativeMenuBarIconPath !== "") { menuItem.icon.source = itemInfo.nativeMenuBarIconPath; }
let me know if I missed on something crucial
Sounds good! A few more thoughts:
- I'd put the icon in
qrc:/resources/cloud_icon.svg
(i.e. drop/qml
from the path) - You could remove the
#ifdef Q_OS_MAC
/#else
checks, and just let both lines run on all OSs. This way, Linux platforms that have a systemwide menubar (i.e. similar to macOS) will also benefit. - In order for light/dark mode to work properly, you might need to set
menuItem.icon.mask
totrue
.
I did the discussed approach + changes but the cloud_icon.svg
is not being rendered on the UI. I did console.log()
in the QML to check if the nativeMenuBarIconPath
being set correctly and it is indeed qrc:/resources/cloud_icon.svg
whenever I open the menu. I double checked at the file path and also the path has been added in the appshell.qrc
file, they seem correct. Any thoughts? I am using Qt 5.15 so it supports the SVG icon source.
@nasehim7, can you push the new code? The Unicode characters aren't going to work unfortunately, so no need to keep that code.
Also, here's a PNG version of the icon you can use, just to try to see something in the menu:
@shoogle yes sure! let me try and get back
@shoogle I tried to incorporate the png but still a challenge in rendering it on the UI. I have updated the code as well here for any reference, incase I am missing anything from my side
@nasehim7 please rebase
@cbjeukendrup @Eism I tried to check it with both the svg and png and it didn't work on the Mac. I think this PR was still open due to that. I will check/test it again today and get back with the results and suggested changes.
Restored PlatformMenuBar.qml
. Currently, kept only the second approach discussed on this thread, installing the assets and using them. The icons still aren’t showing up when I set the icon.source. I also tried the first method, adding the file to appshell.qrc and using the direct path, but no luck there either. I explored and got to know .icns format is compatible with macOS. Also, the appicon is in .icns format. Should we try converting the SVG to ICNS to see if it resolves the issue? Would that be a good way to go? I’m all ears for any suggestions or ideas
Should we try converting the SVG to ICNS to see if it resolves the issue?
That's a good idea, but before you do that, try displaying the app icon in the menu. The app icon is already ICNS.