Skip to content

[ENH]: Distinguish between built-in and user-provided images for toolbar icons #22164

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

Open
daniilS opened this issue Jan 8, 2022 · 2 comments

Comments

@daniilS
Copy link
Contributor

daniilS commented Jan 8, 2022

Problem

Currently, the backends treat icon images for the toolbar the same whether they're built-in, or provided by a user. This extends to replacing image.png image_large.png if it exists (#22135), and changing the black pixels in the icon on a dark theme (#22150). Both of these could be unwanted - in the first case, mpl can load a different file from the one requested, and in the second case, it modifies the image.

Proposed solution

Should we consider treating built-in and user-provided image files differently? Maybe by adding function arguments that control these two behaviours, having separate public and private functions for adding tools, or checking in the code whether the icon is being loaded from inside /mpl-data or a different location?

@timhoffm
Copy link
Member

timhoffm commented Jan 9, 2022

I agree that our internal processing can conflict with user icons. This should be avoided and we have two ways to do this

  1. Distinguishing between internal and user icons (your proposal)
  2. Defining a set of rules how icons should be provided and may be post-processed.

I'd prefer to have a closer look at 2 first. Users might want to opt into our features like choosing high-res images and adapting color according to the theme. In general, it would be a good idea to explicitly document what users should provide as icon. I suppose we don't have more than saying you can add an icon file. https://matplotlib.org/devdocs/api/backend_bases_api.html#matplotlib.backend_bases.ToolContainerBase.add_toolitem

We should specify

  • size(s)
  • formats (at least explicitly mention common ones, not necessary list every format supported by every GUI framework)
  • Color adjustment; To be discussed: Maybe limit this to the case of only black-on-transparent icons? - We should be able to check this.
  • ...

Only if we then think that we still need an explicit flag for user icons, we should follow up on that.

@QuLogic
Copy link
Member

QuLogic commented Jan 11, 2022

I need to experiment a bit more, but I believe GTK4 doesn't correctly theme icons because we are not following the icon theme spec directory layout. If that is the case and I can't figure out a way around that, then I think we should a) move icons to corresponding directories, and b) say that is how we expect user-provided icons to work.

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

No branches or pull requests

3 participants