gyng/save-in

[Feature Request] White SVG usage - theme detection or manual option

irvinm opened this issue · 4 comments

I see that you have white versions of the "update" and "archive" SVG files, but it doesn't look like the code detects the theme or provides an option to allow the user to manually select either the dark or light versions.

When using the dark theme for Firefox, the SVGs are hard to see.

image

Just as a workaround if anyone wants to use it, you can manually apply an inversion filter to the SVG files via userChrome.css. I might have been able to simplify the CSS (Got from exploring "Browser Toolbox") but this worked for me.

html#main-window body popupset#mainPopupSet menupopup#contentAreaContextMenu menu#_72d92df5-2aa0-4b06-b807-aa21767545cd_-menuitem-_save-in-_-_-root.menu-iconic hbox.menu-iconic-left image.menu-iconic-icon {
	filter: invert();
}

image

gyng commented

There was a minor update in 3.7.0 to use a white icon in dark mode for Last Used, but there's a limitation in Chrome for the main app icon. You might have to update your userChrome.css files, sorry!

@gyng thanks for the update. I am running Firefox (FF109.0.1 - v3.7.1), but when I removed my userChrome.css change, I still see the dark SVG.

When I debug live, I see this:

image

When I look into the code, I see you added the white SVG files and also test for dark mode:

save-in/src/menu.js

Lines 153 to 168 in bc0b7b1

// Chrome, FF < 57 crash when icons is supplied
// There is no easy way to detect support, so use a try/catch
try {
browser.contextMenus.create(
Object.assign({}, lastUsedMenuOptions, {
icons: {
16: OptionsManagement.isDarkMode()
? "icons/ic_update_light_24px.svg"
: "icons/ic_update_black_24px.svg",
},
})
);
} catch (e) {
browser.contextMenus.create(lastUsedMenuOptions);
}
},

I am wondering if the "isDarkMode()" is always returning "false" in that check. I tried setting both the Firefox theme to "auto" with Windows in dark mode and also just setting the Firefox theme to "dark". In each case, I could not get the icon to display as white.

@gyng, I spent a little time on this tonight and looked at what TST does to handle dark\light and few articles and it seems like the recommended method is to use media queries.

https://developer.mozilla.org/en-US/docs/Web/API/Window/matchMedia
https://developer.mozilla.org/en-US/docs/Web/CSS/Media_Queries/Testing_media_queries
https://ultimatecourses.com/blog/detecting-dark-mode-in-javascript

TST Implementation:

https://github.com/piroor/treestyletab/blob/0d746115eaa4db30abc0e5a60ce94eebd9169d1f/webextensions/options/init.js#L137

https://github.com/piroor/treestyletab/blob/0d746115eaa4db30abc0e5a60ce94eebd9169d1f/webextensions/options/init.js#L301-L306

https://github.com/piroor/treestyletab/blob/0d746115eaa4db30abc0e5a60ce94eebd9169d1f/webextensions/options/init.js#L995-L997

So, I think at the end of the day, the quickest way to do this (without updating real-time if it changes) would be to change:

save-in/src/menu.js

Lines 159 to 161 in bc0b7b1

16: OptionsManagement.isDarkMode()
? "icons/ic_update_light_24px.svg"
: "icons/ic_update_black_24px.svg",

To:

 16: window.matchMedia('(prefers-color-scheme: dark)').matches
   ? "icons/ic_update_light_24px.svg" 
   : "icons/ic_update_black_24px.svg",