Flicker in default dark theme

[coder12git]

Description

This PR fixes #1451

Notes for Reviewers

Signed commits

• Yes, I signed my commits.

[netlify]

✅ Deploy Preview for mesheryio-preview ready!

Name	Link
🔨 Latest commit	534738d
🔍 Latest deploy log	https://app.netlify.com/sites/mesheryio-preview/deploys/652e7d853cfd8500088484ff
😎 Deploy Preview	https://deploy-preview-1455--mesheryio-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[abhijeetgauravm]

Let's discuss this on website call today at 5:30 PM IST / 7 AM Central time. Please add this as an agenda item in the meeting minutes

[coder12git]

@sudhanshutech please check, and let me know what I am doing wrong, I have done everything similar to docs as you suggested but still it's not working. What, I realize is there is something in css mainly which is causing this, but what css, I still not able to find that?

[iArchitSharma]

@coder12git have you figured out if this is happening due to CSS Rendering or JavaScript Timing?

[coder12git]

@coder12git have you figured out if this is happening due to CSS Rendering or JavaScript Timing?

Yep mainly due to js timing, that's why I added the main logic in starting of the body tag, the main thing is the check whether light mode, dark mode or null should be done in starting, to prevent this which I did, but it doesn't work. I will try again today.

[leecalcote]

Please remove the draft label when ready for another review.

[iArchitSharma]

@coder12git I have not seen what JS logic has been used but I got some idea by seeing the _includes/header.html
The Initial State of Dark mode is not set correctly, the initial state is set to the light mode which i guess is being manipulated by JS
So I guess we have to set the correct initial state in HTML and optimizing JavaScript logic

[coder12git]

@coder12git I have not seen what JS logic has been used but I got some idea by seeing the _includes/header.html The Initial State of Dark mode is not set correctly, the initial state is set to the light mode which i guess is being manipulated by JS So I guess we have to set the correct initial state in HTML and optimizing JavaScript logic

Hmm i agree, can be fix by using dark-mode class in body which I tried but it doesn't work, will see again :)

[iArchitSharma]

@coder12git I have not seen what JS logic has been used but I got some idea by seeing the _includes/header.html The Initial State of Dark mode is not set correctly, the initial state is set to the light mode which i guess is being manipulated by JS So I guess we have to set the correct initial state in HTML and optimizing JavaScript logic

Hmm i agree, can be fix by using dark-mode class in body which I tried but it doesn't work, will see again :)

I know that because you not only have to change the 'dark-mode' class in the body but also all the 'images/logos' to their dark versions. I tried it, and it worked; there is no more flickering. However, there is still a problem: the icon of theme toggler itself is not changing on switching it.
Also, I noticed that by doing this, we are actually flipping the problem, not solving it, as the flickering will start occurring in light mode.

[coder12git]

Hey @iArchitSharma let's discuss this in slack thread if you want, as this bug need to be fix as soon as possible.

[abhijeetgauravm]

Let's discuss this on website call tomorrow at 5:30 PM IST / 7 AM Central time. Please add this as an agenda item in the meeting minutes.

[vishalvivekm]

@coder12git no changes are done here,
will you please make sure to retain the original spacings and indentation here in _includes/home-page.html ?

[vishalvivekm]

why are we changing this layout ?

[vishalvivekm]

@coder12git @iArchitSharma
Good work, here.

Good that you've tried to split the code into components and then accordingly putting them in the over-all flow.
I, however, wonder just this one change ( the logic for checking the current set theme and applying effects acc to it) below in _includes/header.html should fix the issue:

Current

meshery.io/_includes/header.html

Line 55 in 6afdd37

<body style="overflow-x: hidden">

After the change:

  <body style="overflow-x: hidden" class="dark-mode">
    <script>
      const body = document.body;
  
      if (localStorage.getItem("mode") === "light-mode") {
        body.classList.remove("dark-mode");
  
        let allLogos = document.querySelectorAll("#logo-dark-light");
        allLogos.forEach(e => e.src = e.dataset.logoForLight);
      }
  
      // Automatically set logos based on initial theme
      window.addEventListener('load', () => {
        let allLogos = document.querySelectorAll("#logo-dark-light");
        allLogos.forEach(e => {
          e.src = body.classList.contains("dark-mode") ? e.dataset.logoForDark : e.dataset.logoForLight;
        });
      });
    </script>
    

[coder12git]

@coder12git @iArchitSharma
Good work, here.

Good that you've tried to split the code into components and then accordingly putting them in the over-all flow.
I, however, wonder just this one change ( the logic for checking the current set theme and applying effects acc to it) below in _includes/header.html should fix the issue:

Current

meshery.io/_includes/header.html

Line 55 in 6afdd37

<body style="overflow-x: hidden">

After the change:

  <body style="overflow-x: hidden" class="dark-mode">
    <script>
      const body = document.body;
  
      if (localStorage.getItem("mode") === "light-mode") {
        body.classList.remove("dark-mode");
  
        let allLogos = document.querySelectorAll("#logo-dark-light");
        allLogos.forEach(e => e.src = e.dataset.logoForLight);
      }
  
      // Automatically set logos based on initial theme
      window.addEventListener('load', () => {
        let allLogos = document.querySelectorAll("#logo-dark-light");
        allLogos.forEach(e => {
          e.src = body.classList.contains("dark-mode") ? e.dataset.logoForDark : e.dataset.logoForLight;
        });
      });
    </script>
    

@vishalvivekm , so should we revert our pr and only change that one file or the current one is fine?

[vishalvivekm]

@vishalvivekm , so should we revert our pr and only change that one file or the current one is fine?

I remember, on websites call y'day @iArchitSharma saying he'll be PRing the changes he has done locally

[iArchitSharma]

@vishalvivekm, done. I hope everything is fine now

[vishalvivekm]

Thanks @coder12git @iArchitSharma 🚀
LGTM. // @sudhanshutech

[welcome]

Thanks for your contribution to the Layer5 and Meshery community! 🎉

        ⭐ Please leave a star on the project. 😄