Skip to content

AlbumMaximizeComponent: Update icon.name for rotate buttons

This MR updates the icon name for AlbumMaximizeComponent's rotate left/right buttons. The rationale for this change is that with the existing image-rotate-*-symbolic, the icons do not display with some icon themes, such as Papirus and Sweet Icons. With various icon sets which are not Breeze, the rotate buttons simply show up with placeholder "gallery"-esque default icons.

This change came about because I noticed this issue in Kasts. Currently on master it utilizes the same icon names as AlbumMaximizeComponent but does not use this component. When updating the rotate button icon names in Kasts to use object-rotate-*, Kasts started displaying the correct rotate icons with the icon themes which did not work with image-rotate-*-symbolic. I chose to use the object-rotate-* names as this is what was noted in the Freedesktop icon specification, however I am by no means an expert.

In that Kasts MR (multimedia/kasts!119 (closed)), the maintainer explained that they were planning to switch to using AlbumMaximizeComponent, which also uses the symbolic icon names, and suggested it would be a better idea to submit an MR here at kirigami-addons to change the icon name upstream. Essentially Kasts was just following the behaviour of AlbumMaximizeComponent, and now that it will switch to using this component, the issue should be taken further upstream. I hope that explanation made sense 😸

The below screenshots are taken from Kasts, and should illustrate the fix.

Before & After (Papirus Dark icon theme)

image

image

Before & After (Breeze Dark icon theme)

image

image

Note that the Breeze icon has changed slightly.


With that out of the way, another reason for opening this MR (as discussed in the linked Kasts MR) is to hopefully get a discussion going on: Is this the correct fix?

I am not entirely sure on how icon names are chosen, or what the significance of -symbolic is for icons. Perhaps this is an issue that needs to be brought to the icon maintainers themselves and it's an issue on there end.

Regardless, it is an issue I found and despite being slightly out of my depth I am willing to help contribute to fix it if the fix can be done on this side :-)

Thanks!

Merge request reports