-
Notifications
You must be signed in to change notification settings - Fork 50
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
Extension Icons for Light/Dark/Custom Theme #229
Comments
I fully support it. However, it seems to me that it should be implemented as a browser-based JS API available for websites as well. The point is that all the problems from the list above are relevant for Favicons. |
I think website favicon and extension icon are not exactly the same problem. Here is a stackoverflow question for website favicon. There are several solution for it. For example, svg favicon with media query, But these solutions do not apply to extensions. Extension icons are declared in manifest.json. And there is no |
Chromium is tracking a similar feature request in issue 893175. A Chromium engineer shared a link to a design doc we were considering in comment 19, but that effort is currently on hold. @xeenon, @zombie, and @mukul-p, it might be worth taking a look at this and sharing feedback either on the doc or on a related issue. |
I see the design doc. It is a static(declarative) way in manifest file. I think it solves 90% of the problems. I think a runtime API should also be provided to solve dynamic problems, because developers can change icon by
|
We can draw some inspiration from ongoing discussion on how to handle this in PWA manifests, see: In general, going for a list of icons versus an object, like Mozilla did with We can go for a syntax like this:
Or adding a |
Firefox implementation can be seen at e.g. https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/action |
For best backwards compatibility, we can consider implementing the
We can then move forward with more breaking changes in regard to icons for Manifest v4. |
I didn't know Firefox's theme_icons before. For me, it's counterintuitive. I prefer dark for icons used in dark theme and light for icons used in light theme. |
@hanguokai This would then be something we can fix once drafting MV4. As for the API to update
Calling this method would overwrite the Currently in Firefox, setIcon will overwrite what is defined in |
The |
Discussion on this issue has been narrowed to the |
For reference, 5 years ago I proposed |
We (Mozilla) are going to add support for As for |
any updates on chrome extension support for dark mode icon? |
The title of this issue is specific to browser toolbar icons. Should another issue be created for A single context menu item by default uses the Certainly dark menu background may require inverted colors for icons. |
This issue actually focuses on all the places where the icon appears, including:
So I change the title of this issue (remove toolbar) |
I know that browser vendors have a lot of backlogs to do and need to understand the priorities of different issues. Many developers have recently asked this question (link-1, link-2, link-3). In other words, whenever you develop a new extension, you need to design an icon, then you may encounter this problem. Supporting light and dark icons is very fundamental, it should get higher priority. There is currently no ideal workaround. Except toolbar action icon, developers cannot control icons in other places. Even with the icons on the toolbar, developers can't listen to theme changes very well. According to my observation, developers use the following workaround to solve it:
This method is not the best effect, nor can it achieve effects like browser built-in icons. But this is the most common way to workaround, so the entire extension store is flooded with icons of this design. |
We (Chrome) are currently working on an implementation of the
My current thinking is:
|
Here is my opinion.
The browser should give a warning, and use it for both dark and light. To use the dark/light keys, developers should provide two sets of icons, which is developers' obligation.
The dark/light keys have higher priority than top-level size keys, so the browser should ignore top-level size keys.
If old versions of browser can ignore the unknown dark/light keys, developers could provides both dark/light and top-level size keys. Otherwise, developers should only provide one of them, and set "minimum_chrome_version" for the dark/light keys.
I have no clear idea. Can the browser automatically determine that the current theme is more inclined to the light or dark? |
As mentioned during our 2023-12-07 meeting. An extension with Something to take into consideration is the difference between OS dark mode and the need for an icon which works on a dark or light background. The browser could have a custom theming which would mean the icon needed does not equivalent the OS Dark Mode. This is I believe why the |
At present, this issue is mainly about developers' feedback on functional requirements of extension icons on Dark/Light theme, rather than detailed proposal. And all browsers are supportive of this feature. So this issue has achieved its goal. Next, I suggest that browsers submit a formal PR as the specification (like other browsers' proposals) to |
While considering a dark/light icon setting in
Just an idea... {
"action": {
"theme": {
"light": {
"icons": {
"16": "image/icon.svg",
"32": "image/icon.svg"
},
"badge_background_color": "#ffffff",
"badge_text_color": "#000000"
},
"dark": {
"icons": {
"16": "image/icon-dark.svg",
"32": "image/icon-dark.svg"
},
"badge_background_color": "#000000",
"badge_text_color": "#ffffff"
}
}
}
} |
Agreed on the noise in this issue @hanguokai - continuing here for now but agree it might be good to have a new issue once we have a concrete proposal. One thing that came up in the last meeting was that unfortunately We haven't settled on anything yet, but with that in mind we've been discussing a few different options. One that has come up is a new
Does this seem like a reasonable way to have a cross-browser key which is backwards compatible? There would be a few things to decide about precedence but I think we can figure those out if this seems agreeable. |
@oliverdunk that is exactly the API I was expecting to come from this issue. Assuming older browser versions can simply ignore the icon variants without it throwing an error I think this would be the best option. In terms of precedence I'd suggest the following:
I would like to see a way to determine which of the icons was selected in JavaScript so that programmatically defined icons can also follow the theme setting. It would perhaps be prudent to also add a high contrast option to the icon variants for improved accessibility. |
@oliverdunk one suggestion is to work together with the Web Applications WG on how to best specify this in the manifest. They have some interesting discussions about this: In general it is good not to forget the need for icons which work well on a dark / light background independent of the operating system dark mode due to custom browser theme requirements. My suggestion is to prioritise a way to set action icons for dark backgrounds programmatically. @JamesCoyle Instead of allowing the extension to detect which mode is used (dark/light), I would propose setting both when calling browser.action.setIcon({
imageData: {
16: image16,
32: image32,
},
darkBackground: {
imageData: {
16: image16,
32: image32,
}
}
}); Either using Since the action button is the main area which has this need. We could allow changing badgeBackgroundColor and badgeTextColor in the calls as well. Think of something like this: browser.action.setThemeData({
"dark": {
"badgeBackgroundColor": "lightgreen",
"badgeTextColor": "black",
"icon": "dark.svg",
},
"light": {
"badgeBackgroundColor": "green",
"badgeTextColor": "white",
"icon": "light.svg",
}
}); |
@carlosjeurissen extending setIcon to allow for the icon variants would work for me. I can't currently think of a use-case that wouldn't work with that. |
It would be nice if SVG icons simply supported Edit: this currently works thanks to Edit2: w3c/csswg-drafts#9872 |
@phaux I agree, that should work in Safari. |
@phaux how about using an env variable for this to provide authors a way to have a fallback in case the browser does not support this feature? |
For my use case, I am dynamically creating the browser icon at runtime. In order to accommodate dark and light modes, I draw different styles to a canvas and then set the icon to the canvas contents. An API design which takes as input both images would be better than needing to detect the theme in a offscreen document, but would still require wasted work painting the unused theme's respective canvas. Preferably media queries would be exposed to service workers, but something like Also, notifications are another surface to consider. |
The tradeoff here is that it means the icons wouldn't be available when the theme first changes, and we couldn't update them at the same time as everything else. I don't think exposing media queries to service workers is off the table but I think we'd be hesitant to encourage doing reactive work based on theme changes, at least in the most common cases :)
For sure! |
@oliverdunk Very much agree with having both dark and light icons available so a theme switch can be made is a good one. However I believe exposing additional metadata for the action icon would be very useful in reducing the icons needed to generate and action icons are as pixel perfect as possible. For example, devicePixelRatio is currently not available in serviceWorkers sadly. Think about a method like {
"size": 16,
"dpi": 2,
"actualSize": 32,
"color_schemes": ["light", "dark"]
}
|
On the topic of DPR, I just realized that the multiple monitor use case is also something to consider. There can be multiple screens with different icon sizes. So for a hypothetical function to get the icon size, you would need to return a set or array of icon sizes. You should then be able to use that with the existing size based As far as I can tell calculating icon sizes for multiple screens is currently not trivally implemented. Should this icon size retrieval topic be filed as a separate issue? |
A new issue would be best. |
A little related to issue-216. Besides the text color and background color of the BadgeText, the extension icon itself is also a problem to adapt to various themes (Light/Dark/Custom).
Current state, developers can set static extension icon or dynamic change it by
setIcon()
. But:The text was updated successfully, but these errors were encountered: