ubuntu / yaru

All Ubuntu Yaru GNOME themes
https://community.ubuntu.com/c/desktop/theme-refresh
GNU General Public License v3.0
1.34k stars 180 forks source link

Icons for "New", "Open" and "Save" are the same in KeepassXC #1294

Closed marcbone closed 4 years ago

marcbone commented 5 years ago

I think this issue actually belongs here.

Expected Behavior

In KeepassXC the icons for New, Open and Save should be different like in the adwaita theme

Actual Behavior

The icons are the same

Specifications

name: communitheme summary: The next Ubuntu theme built by the community. publisher: Didier Roche (didrocks) license: unset description: | Yaru, formerly known as Communitheme, is the new Ubuntu theme built by the community. Yaru will become the default Ubuntu theme in Ubuntu 18.10. This package allows you to try out the theme on Ubuntu 18.04 LTS.

To try out the theme, install this package on Ubuntu 18.04 LTS, restart your computer and select the "Ubuntu with communitheme snap" session from the login screen.

More information is available at https://community.ubuntu.com/t/faq-ubuntu-new-theme/1930. snap-id: Yd6CISPIf6tEf3ZEJ0cqSoEg9rG2VkRi tracking: stable refresh-date: 13 days ago, at 19:41 CET channels: stable: 0.1 2019-03-13 (1768) 16MB - candidate: ↑
beta: ↑
edge: 0.1 2019-03-22 (1775) 16MB - installed: 0.1 (1768) 16MB -

CDrummond commented 5 years ago

Yeah, I noticed this and mentioned in issue #831 As stated there, I believe the issue is that Yaru has a document icon, but no document-new and document-save icons. So, when Qt looks for document-new it can't find it and fallsback to document

clobrano commented 5 years ago

Qt looks for document-new it can't find it and fallsback to document

how does this work? Yaru iconset inherits from Humanity and hicolor, so if an icon is not provided directly it should be given by one of the above icon sets

EDIT: now we also have document-new, but it's a symbolic icon

CDrummond commented 5 years ago

To be honest, I don't really know. But seeing as the Yaru icon set has document I think Qt just uses that - ie. finds the closest matching icon within the current set. But, I could be wrong :-)

clobrano commented 5 years ago

I looked at the code. The application actually looks for document-new while we have document-new-symbolic. If I change the name, removing the -symbolic part, it works. Preparing a fix, but maybe it'd be good to infor Kepassxc that the naming convention is changing in adwaita as well

clobrano commented 5 years ago

Transmission has the same use case, but it works fine

image

chrisjbillington commented 5 years ago

Also seeing this in tortoisehg - it displays a document icon instead of a save icon, because it is looking for 'document-save'.

chrisjbillington commented 5 years ago

Qt doesn't seem to know about fallback themes in this case:

import sys
from PyQt5 import QtWidgets, QtGui

app = QtWidgets.QApplication(sys.argv)

icon = QtGui.QIcon.fromTheme('document-save')

print("actual icon name:", repr(icon.name()))
print("theme name:", repr(icon.themeName()))
print("fallback theme name:", repr(icon.fallbackThemeName()))

button = QtWidgets.QToolButton()
button.resize(200, 200)
button.setIcon(icon)
button.show()
app.exec()

prints:

actual icon name: 'document'
theme name: 'Yaru'
fallback theme name: ''

and shows the document icon: image

CDrummond commented 5 years ago

@chrisjbillington is this not how the icon themes are supposed to work? I thought the convention was to strip off the trailing "-something" until a match was found. So that an icon set could have (e.g.) "input-mouse" which would be used if "input-mouse-usb" was not found.

See https://standards.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html this mentions:

The dash “-” character is used to separate levels of specificity in icon names, for all contexts other than MimeTypes. For instance, we use “input-mouse” as the generic item for all mouse devices, and we use “input-mouse-usb” for a USB mouse device. However, if the more specific item does not exist in the current theme, and does exist in a parent theme, the generic icon from the current theme is preferred, in order to keep consistent style.

Seeing as "document" exists, but not "document-new", then "document" is being used - as its the more generic term, and in the current icon theme.

chrisjbillington commented 5 years ago

@CDrummond, ah, I see! That makes sense. I had no clue how it was supposed to work and was just reported how it appears to be working. Good to know Qt appears to be following spec (even though the outcome might be not ideal in this case).

leoheck commented 5 years ago

Hi guys, what is the status of this issue? Do you have any workaround for that? Just let me know cause I want to apply here, please. I am really interested in fix this https://github.com/ubuntu/yaru/issues/1361

clobrano commented 5 years ago

Hi @leoheck,

we didn't go deeper on this actually. I should be able to try again today with a VM

leoheck commented 5 years ago

@clobrano what do you want to try? Why you need a VM? What should I do to help to move this forward? I thought it was just creating some symbolic links, isn't it?

clobrano commented 5 years ago

See this for example

Transmission has the same use case, but it works fine

image

Icon sets have a "fallback" theme, for providing a wider range of icons, Yaru uses Humanity which should have document-new, and according to @chrisjbillington prototype above, the API QtGui.QIcon.fromTheme is expected to look for the fallback theme https://srinikom.github.io/pyside-docs/PySide/QtGui/QIcon.html#PySide.QtGui.PySide.QtGui.QIcon.fromTheme

I still wonder why it could not find it and of course what we can do to support it. If you have any idea of solution or testing, I will be glad to discuss it :)

clobrano commented 5 years ago

Qt doesn't seem to know about fallback themes in this case:

import sys
from PyQt5 import QtWidgets, QtGui

app = QtWidgets.QApplication(sys.argv)

icon = QtGui.QIcon.fromTheme('document-save')

print("actual icon name:", repr(icon.name()))
print("theme name:", repr(icon.themeName()))
print("fallback theme name:", repr(icon.fallbackThemeName()))

button = QtWidgets.QToolButton()
button.resize(200, 200)
button.setIcon(icon)
button.show()
app.exec()

prints:

actual icon name: 'document'
theme name: 'Yaru'
fallback theme name: ''

and shows the document icon: image

hi @chrisjbillington, with which pyqt5 version you tested this code? I installed the latest from pip (version 5.13.0) but it gives me errors like QIcon does not have fallbackThemeName attribute and the exec function is actually exec_

EDIT: nevermind, it actually works fine in 19.04, sorry

leoheck commented 5 years ago

I see. I tried many things here to test if how to fix this fallback issue but none of them was good. So, I fixed it with this workaround since now since I dont want to wait more time for a fix.

sudo ln -fs /usr/share/icons/Humanity/actions/48/document-new.svg  /usr/share/icons/Yaru/scalable/actions/
sudo ln -fs /usr/share/icons/Humanity/actions/48/document-open.svg /usr/share/icons/Yaru/scalable/actions/
sudo ln -fs /usr/share/icons/Humanity/actions/48/document-save.svg /usr/share/icons/Yaru/scalable/actions/
sudo ln -fs /usr/share/icons/Humanity/actions/48/document-print.svg /usr/share/icons/Yaru/scalable/actions/
Feichtmeier commented 5 years ago

Maybe we have an icon that is symlinked to all these icons document-new.svg document-open.svg document-save.svg document-print.svg ?

Feichtmeier commented 5 years ago

Or some error in the index https://github.com/ubuntu/yaru/blob/master/icons/Suru/index.theme#L8

clobrano commented 5 years ago

The actual icon name found is "document", so Qt actually selects the "document" image before looking for an alternative in the fallback themes (which are used only if no match is found). I think that, as per CDrummond comment, we are forced to provide the icons for:

document-new.svg document-open.svg document-save.svg document-print.svg

EDIT: I am not sure we can safely symlink icons from the fallback theme, so @leoheck solution is valid only if made manually in its own system.

\cc @ubuntujaggers :)

leoheck commented 5 years ago

I guess numix does the same with symlinks, I found some there but I am not sure if they were related with these files. We just have to find an icon them with this working so we can ask/or see what are the differences.

I didn't have time to test hundred things this morning, but Freecad works with the default Adwaita theme. It is just a matter of comparing folders/files. The only issue I can see is the folder structure that slows down this process.

https://github.com/numixproject/numix-icon-theme

Feichtmeier commented 5 years ago

No, it's exactly what @clobrano said. We need that icons . The thing is: full color action icons where never intended to be part of Suru. Since we no longer follow the development of Suru, we can for sure pic up that shitload of work. But for modern gnome apps, those full color action icons are not necessary. Yet I understand that for those applications which use that legacy toolbars it's relevant. But those 3 icons are just a tiny potion of the real work, yet stuart could for sure make those 3 icons and we could close this issue... But KDE Apps are really not high on our priority list

Amr-Ibra commented 4 years ago

Any news here? I'm also affected by this, using the Qt apps: Kid3-qt and Octave.

Amr-Ibra commented 4 years ago

Many Qt apps appear broken in Yaru because of this issue.

ubuntujaggers commented 4 years ago

There's about a trillion of these icons, isn't there? But maybe I could do just the "main ones" and see how we get along with that?

clobrano commented 4 years ago

Indeed, we can start from the small list here

ubuntujaggers commented 4 years ago

image

??

Amr-Ibra commented 4 years ago

OK, the first two show "New" and "Open", good, but unfortunately, the third doesn't show "Save".

clobrano commented 4 years ago

Adwaita adds an arrow to underline that it's an icon to "save", maybe it can help. What about using a green dot in the "new" icon, instead that blue?

Amr-Ibra commented 4 years ago

I suggest the "save" icon to be the same as the "new" icon but replacing the dot with a downward arrow.

ubuntujaggers commented 4 years ago

I can certainly make the dot green - I just used blue because that's what Adwaita did :)

Document with a down arrow would be very easy to draw, so that gets my vote!

Amr-Ibra commented 4 years ago

Or follow Adwaita and use the "file-manager" icon with a downward arrow as a "save" icon, as seen in the first comment. It's your choice.

ubuntujaggers commented 4 years ago

image

They also show up in Inkscape, with the document-print icon too:

image

Problem: if I create a new folder in the icons structure for 22x22 icons (which these are) they don't get picked up. I got it to work by dropping them in the 24x24 full colour apps folder. Bit messy but it works - does anyone know how to get it to recognise a completely new folder for these legacy full colour icons? Alternatively: does anyone object to them staying in an existing folder?

clobrano commented 4 years ago

Nice work @ubuntujaggers!

I got it to work by dropping them in the 24x24 full colour apps folder. Bit messy but it works

is it messy, because the icons are not 24px?

ubuntujaggers commented 4 years ago

@clobrano Yes, and they're not apps either. I don't mind, but it's maybe not very organised.

clobrano commented 4 years ago

I see. I am not really against leaving it this way, since it's the only way to make it work, but I'll try to find the reason first

Amr-Ibra commented 4 years ago

Did you try sizing them 24×24 and put them in a legacy folder like Adwaita? https://gitlab.gnome.org/GNOME/adwaita-icon-theme/tree/master/Adwaita/24x24/legacy

And excuse my ignorance, why not sizing the icons 48×48 for them to scale better?

ubuntujaggers commented 4 years ago

Hi @Amr-Ibn, the icons render at 22px, so a multiple of 24 wouldn't scale sharply to 22px (which is important for a small button with details like a little "+"). If the icons can be displayed at 44px then we do need to export it at double size. If the icons can be rendered at multiple different sizes then we need multiple icons.

It may be the new folder didn't follow the right naming convention - I'll copy Adwaita exactly and see if that works, thanks for the tip :)

ubuntujaggers commented 4 years ago

Okay, looks like Adwaita has these icons at 22px plus all the usual sizes.

Step one: I'll see if I can get the folders to work.

Step two: I'll draw the other sizes.

ubuntujaggers commented 4 years ago

Okay, copying the 22x22/legacy folder structure doesn't work :(

Changing the icon size in Ubuntu doesn't seem to make these buttons render at different sizes. Nor does using the accessibility zoom feature (it just zooms in on the existing image assets, rather than adapting the UI to use any higher res assets that might be available).

Given that these are legacy icons, my suggestion is to only target the default 22px button size (presuming this is the same regardless of monitor size - but we can test that when I push). However, I will export the icons at 176px ( = 8x size) so they don't become blocky if there are edge cases where the UI renders them larger than 22px. They won't be 100% sharp in these cases (if they even exist) but at least there won't be visible pixellation.

ubuntujaggers commented 4 years ago

Okay, frustratingly, that won't work. If I export a high res png for these buttons, Keepass scales it down to 22px (which is the desired behaviour) but Inkscape shows it at 100%, leading to a comically enormous button. So, really the only safe thing to do is to have a 22px button at 22px and then create the other sizes if they're ever used.

clobrano commented 4 years ago

Thanks @ubuntujaggers, it seems good to me :) I can't test right now, but I kinda remember to have found this icons also in some "action" folder, is that possible?

ubuntujaggers commented 4 years ago

That was my first attempt, but there was no actions folder under full colour so I had to make it :(

Basically: if I make any new folders in the Yaru structure, the icons in them don't get used. But if I put them in existing folders they do.

clobrano commented 4 years ago

Sorry for the silly question, but is there any list of "folders to be installed" that might need to be updated when you create a new one?

ubuntujaggers commented 4 years ago

Good point, I'll see if I can find one :+1:

chrisjbillington commented 4 years ago

For what it's worth here in tortoisehg we've got document-save in 16x16:

image

ubuntujaggers commented 4 years ago

Aha, good tip. Then we need to work out how to make new folders and do this properly.

Feichtmeier commented 4 years ago

Should be fixed by #1600 (even though #1600 produced #1610 )

Could anyone confirm that it is fixed?

chrisjbillington commented 4 years ago

I can confirm the icon in tortoisehg has been fixed.

clobrano commented 4 years ago

Thanks!

amavlyanov commented 4 years ago

Any chance it'll be ported to 19.04 / 19.10?

clobrano commented 4 years ago

19.10 for sure