MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.84k stars 4.83k forks source link

[Bug]: we should never render a theme different from the user's selected theme #26545

Open davidmurdoch opened 3 weeks ago

davidmurdoch commented 3 weeks ago

Our loading screens always use the system default theme, but the user may have selected a different theme.

Instead of only storing the users theme setting in (browser|chrome).storage.local, we could sync it to localStorage, which has a synchronous interface that can be used to set the page's theme correctly before it the first render, eliminating a flash of the incorrect theme.

Because this setting is cleared whenever the browser starts up we'd need to occasionally sync it from storage.local to localStorage (like when the background starts up and on each initial page render).

Note: service-workers don't have access to localStorage, so syncing the value will have to be proxied through an interface that can.

davidmurdoch commented 3 weeks ago

Note: this is how https://ganache.pages.dev/ selects the system theme by default, but allows the user to toggle to the opposite theme and when refreshing/revisiting the page the system theme doesn't cause a flash of the incorrect theme.

Unrelated fun fact: theme switching (but not theme persistence, as that is not possible on a static page AFAIK) works even if JavaScript is disabled 🤯 .

davidmurdoch commented 3 weeks ago

Example:

function getUserColorTheme() {
  const localTheme = localStorage.getItem("theme");
  if (localTheme) {
    return localTheme;
  } else if (
    window.matchMedia &&
    window.matchMedia("(prefers-color-scheme: light)").matches
  ) {
    return "light";
  } else {
    return "dark";
  }
}

const userColorTheme = getUserColorTheme();
document.documentElement.setAttribute("data-theme", userColorTheme);
davidmurdoch commented 3 weeks ago

A better solution may be top use https://developer.chrome.com/docs/extensions/reference/api/action#method-setPopup:

This feature may allow us to set the user's popup to one that matches the user's theme:

chrome.action.setPopup({popup: `popup-${state.metamask.theme}.html`});

Which should eliminate the flash of the wrong theme. Neat!