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

Add ${qx.iconset} environment variable and replace hardcoded icontheme #9485

Open
cboulanger opened this Issue Jan 19, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@cboulanger
Contributor

cboulanger commented Jan 19, 2018

@johnspackman has looked at the framework code and found that the iconthme "Tango" is often hardcoded. This needs to be replaced by ${qx.icontheme}. Better yet, we should add ${qx.iconset} defined as qx/icon/${qx.icontheme} and then change references in the framework to just use @asset(${qx.iconset}/22/etc/etc.png).

@cboulanger

This comment has been minimized.

Contributor

cboulanger commented Jan 24, 2018

To take this a step further: It seems a bad idea to use hardcoded icon paths at all. To make theming possible that is independent of the (arbitrary) naming and ordering of the Tango and Oxygen icon sets that are shipped with the framework (side note: they look very 1990-ish), all icon paths in the framework should be aliases which are resolved by the theme used.

@cboulanger

This comment has been minimized.

Contributor

cboulanger commented Feb 20, 2018

Sorry for just having opinions and not contributing any code ATM (just no time) - just jotting down some conceptual thoughts here: We should think of icons not in terms of their actual paths and what they actually represent, but in terms of their function. For example, I use the (partly aliases) path "icon/16/apps/utilities-network-manager.png" from Oxygen in a tree to represent the root node of a access control list, even though it has nothing to do with a network. If I change the iconset, I might use a completely different icon from Tango (or whatever other iconset). That is why it is so important to establish a "best practice" in which users define icon aliases and declare assets in the icon theme class itself and NOT in the widget classes. There, only the symbolic aliases should be used (in my case,for example, 'appicons/aclroot'). This might mean a bit more work upfront but prevents enormous headaches later. Qooxdoo already has all the tools for this approach and we should use it in the framework itself....

@hkollmann

This comment has been minimized.

Member

hkollmann commented Feb 20, 2018

You are right. On the other side this is a lot of work. Therefore we should fix the bug as a first step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment