Skip to content
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

feat/question: usage of .svg sources directly as Actions icons and how to customize Action's icon color? #217

Closed
dalthviz opened this issue Aug 14, 2024 · 4 comments · Fixed by #219

Comments

@dalthviz
Copy link
Contributor

dalthviz commented Aug 14, 2024

Description

Currently, over napari, some .svg sources are used to generated themed/colored icons + QSS to use the generated files/icons over Qt elements. Could it be worthy to be able to allow to pass as an Action's icon definition a path to a .svg besides iconify/supertqt.fonticon identifiers?

If that doesn't seem like the right approach, what would be the proper way to provide/set colored Action's icons? I saw #132 (although seems like there only a dark and a light color could be configured?) and checking superqt fonticon utilities seems like you can create font plugins, right? Should napari eventually try to create a font including the .svg files currently being used? 🤔

Edit: This is part of the elements that started to be discussed over an exploration to use QModelToolBars for buttons over napari. For more details on that you can see napari/napari#7133

@tlambert03
Copy link
Member

i can certainly see the usefulness of accepting svg paths. We just need to be careful not to overload the interpretation of the string too much, so if the icon is a literal path, perhaps it should be prefaced by file://.
It's also worth pointing out that that works fine for your internal use, but the benefit of an icon namespace (whether it be via iconify or a font icon) is that it becomes a bit easier for you to offer up icons for others to use.

Should napari eventually try to create a font including the .svg files currently being used? 🤔

I've definitely gone back and forth between preferring fonts and svgs... fonts are much easier to style in a theme-aware way (since they can be re-colored purely via QSS), but there's no doubt that svgs are incredibly flexible, particularly via iconify. As you probably saw, I once started considering a big overhaul of napari's icon delivery methods (it's what prompted the creation of superqt.fonticon in the first place), but i no longer have a strong opinion on what you all should do there. When I proposed making a font library for napari icons a while back there was some pushback, so that's a discussion you should have internally. If you want to stick with the builtin svgs, and if there is a feature that you need added to app-model to support it, then as long as it seems like a "generally" useful feature I'm happy to add it here.

I saw #132 (although seems like there only a dark and a light color could be configured?)

note that the light/dark point of #132 is to acknowledge the fact that one color rarely works for both a light and dark colored theme. So it lets you pick not one, but two colors, to be chosen based on whether the background is generally light or generally dark. when you say "only" a dark and light color, what limitation are you imagining there? did you interpret that to mean you can only pick from one of two literal colors?

@dalthviz
Copy link
Contributor Author

dalthviz commented Aug 15, 2024

i can certainly see the usefulness of accepting svg paths. We just need to be careful not to overload the interpretation of the string too much, so if the icon is a literal path, perhaps it should be prefaced by file://.

Makes sense to me 👍 Will try to implement that but if you have any further pointers on how things should work let me know!

what limitation are you imagining there? did you interpret that to mean you can only pick from one of two literal colors?

I interpreted it like you have only one color when the mode is dark and one color when the mode is light. Maybe is something different? 🤔 And, about limitations, I guess what my interpretation made me think about is how to support a case where you have multiple darkish or lightish palletes/themes. So for example, I can think on a dark palette/theme that shows icons with a color like imagen but another dark palette/theme which could show icons with a color like
imagen but with #132 you will be able to only set one of those as the dark mode color, no?

@tlambert03
Copy link
Member

tlambert03 commented Aug 15, 2024

ah, you're getting to the concepts of themes then. which is not something that app-model currently tracks. I would expect that to go in napari's Theme.icon.

The icon color in that PR lives more in a world where app-model makes no particular assumptions about the "theme" per se, other than to acknowledge that light and dark themes generally have different needs. And it gives you an icon-specific color, (not a theme-specific icon color). If the user doesn't declare an icon color, it will just use an appropriate grayish color given the background. If the user does declare a color (imagine a special icon, like your trash icon, that needs to be red to indicate some semantics) then app-model allows them to say how that special color should show in both light and dark themes. However, if you want to have the full concept of different themes and palletes, and how all icons should be colored basded on the theme, then I would say that should probably continue to live in napari's Theme object. would that work for you?

@dalthviz
Copy link
Contributor Author

Thank you for the explanations! I think I have now a better grasp of the scope of #132 and in general app-model handing of icon colors. I think I will need to further check the logic over napari but seems like as long as I'm able to set a .svg as an action icon I should be able to customize the icon over the napari side 👍

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

Successfully merging a pull request may close this issue.

2 participants