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

Navigation bar color flickers on page change #138

Open
HenestrosaDev opened this issue Sep 30, 2023 · 6 comments
Open

Navigation bar color flickers on page change #138

HenestrosaDev opened this issue Sep 30, 2023 · 6 comments

Comments

@HenestrosaDev
Copy link

When changing pages on a mobile device, the color of the navigation bar changes from white to the background color of the body, which is annoying and detrimental to the UX. I've tried to fix this with no luck yet, but if I manage to solve it, I'll make a pull request.

The problem might be in this code of the toggle-theme.js file, because the value of the theme-color is only changed if the body exists. That is, when the page is loaded.

function reflectPreference() {
  document.firstElementChild.setAttribute("data-theme", themeValue);

  document.querySelector("#theme-btn")?.setAttribute("aria-label", themeValue);

  // Get a reference to the body element
  const body = document.body;

  // Check if the body element exists before using getComputedStyle
  if (body) {
    // Get the computed styles for the body element
    const computedStyles = window.getComputedStyle(body);

    // Get the background color property
    const bgColor = computedStyles.backgroundColor;

    // Set the background color in <meta theme-color ... />
    document
      .querySelector("meta[name='theme-color']")
      ?.setAttribute("content", bgColor);
  }
}
@HenestrosaDev HenestrosaDev changed the title Navigation bar color flashes on page change Navigation bar color flickers on page change Oct 1, 2023
@HenestrosaDev
Copy link
Author

Not the most elegant solution, but I ended up doing the following:

  1. Instead of leaving the content attribute empty in the theme-color meta tag, I set the predefined value to the light theme color base of my site:
    <meta name="theme-color" content="rgb(246, 238, 225)" />

  2. I added this code to toggle-theme.js. Note that I've only included the changed methods, but you can check the whole file of my website if there is any doubt:

function setPreference(isThemeBtnClicked) {
  localStorage.setItem("theme", themeValue);

  reflectPreference();
  changeThemeColorContent(isThemeBtnClicked);
  toggleGiscusTheme();
}

function changeThemeColorContent(isThemeBtnClicked) {
  const themeColorElement = document.querySelector("meta[name='theme-color']")

  if (themeValue === "dark") {
      themeColorElement?.setAttribute("content", "rgb(16, 23, 42)");
  } else if (isThemeBtnClicked && themeValue === "light") {
    themeColorElement?.setAttribute("content", "rgb(246, 238, 225)");
  }
}

window.onload = () => {
  function setThemeFeature() {
    // set on load so screen readers can get the latest value on the button
    reflectPreference();
    changeThemeColorContent();

    // now this script can find and listen for clicks on the control
    document.querySelector("#theme-btn")?.addEventListener("click", () => {
      themeValue = themeValue === "light" ? "dark" : "light";
      setPreference(isThemeBtnClicked = true);
    });
  }

  setThemeFeature();

  // Runs on view transitions navigation
  document.addEventListener("astro:after-swap", setThemeFeature);
};

It's true that the navigation bar still flickers when loading a new page in dark mode on mobile, but at least now it won't flicker when the light theme is set. Another solution would be to use prefers-color-scheme, but since the theme can be changed independently of the user's preferred color scheme, it's hard to get it to work properly:

<meta name="theme-color" media="(prefers-color-scheme: light)" content="cyan" />
<meta name="theme-color" media="(prefers-color-scheme: dark)" content="black" />

I won't make a pull request with these changes, as I don't think they are the most optimal solution. I wish it would be valid to set the value of the content attribute to currentcolor, as it would change dynamically without requiring any JavaScript code.

@satnaing
Copy link
Owner

satnaing commented Oct 6, 2023

Unfortunately, I cannot reproduce this issue on my browsers.
So, if possible, can you also provide me the browser you tested and the OS.
Maybe getting the color of the document body in toggle-theme.js is somehow blocking and causing the problem.
If it is so, we can update the reflectPreference function inside toggle-theme.js to something like this

function reflectPreference() {
  document.firstElementChild.setAttribute("data-theme", themeValue);

  document.querySelector("#theme-btn")?.setAttribute("aria-label", themeValue);

  const metaThemeColor =
    themeValue === "light" ? `rgb(251, 254, 251)` : `rgb(33, 39, 55)`; // here, you can get these colors from base.css in your project
  document
    .querySelector("meta[name='theme-color']")
    ?.setAttribute("content", metaThemeColor);
}

In the above function, you can hard-code rgb(251, 254, 251) and rgb(33, 39, 55) without any issue. You can get these values from base.css file in your project.

/* file: base.css */
  html[data-theme="light"] {
    --color-fill: 251, 254, 251; /* 👈🏻 get this value for light theme */
    --color-text-base: 40, 39, 40;
    --color-accent: 0, 108, 172;
    --color-card: 230, 230, 230;
    --color-card-muted: 205, 205, 205;
    --color-border: 236, 233, 233;
  }
  html[data-theme="dark"] {
    --color-fill: 33, 39, 55; /* 👈🏻 get this value for dark theme */
    --color-text-base: 234, 237, 243;
    --color-accent: 255, 107, 1;
    --color-card: 52, 63, 96;
    --color-card-muted: 138, 51, 2;
    --color-border: 171, 75, 8;
  }

In AstroPaper, I didn't hard-code those values intentionally. This is because AstroPaper, as a template for various users, has no ability to know which color schemes the user would configure and customize. So, it is better to get the theme-color value from the body using JavaScript. It'd be best if theme-color meta tag supports currentColor, but sadly it doesn't.

If theme-color meta tag bothers you, you can omit that meta from the code; and it would be completely fine without any problem.
You can read more about theme-color meta tag here.

BTW, your updated code would also be fine I guess.

@HenestrosaDev
Copy link
Author

HenestrosaDev commented Oct 6, 2023

You can reproduce the issue using Chrome with the light theme on Android.

EDIT:
Here is a video showing the reported behavior:

video_2023-10-06_10-01-58.mp4

@satnaing
Copy link
Owner

satnaing commented Oct 6, 2023

It's a bit hard to reproduce since I don't find anything like that on iOS.
I'll try and reproduce it using an Android phone.
BTW, did my comment solve this issue? Or does the flickering still exist?

@HenestrosaDev
Copy link
Author

The flickering is still present with the code you provided. Let me know if I can help with anything else.

@saliksik
Copy link

saliksik commented Oct 6, 2023

I can confirm the issue. I tested this on my Samsung A52s 5G phone, and the navigation bar color flickers when pages change.

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

No branches or pull requests

3 participants