kiwix / kiwix-js

Fully portable & lightweight ZIM reader in Javascript
https://www.kiwix.org/
GNU General Public License v3.0
298 stars 125 forks source link

Mitigate against excessive flashing between light and dark themes on startup #787

Open Jaifroid opened 2 years ago

Jaifroid commented 2 years ago

This issue exists in our current code, but it will be exacerbated by #764 / #771 and will particularly affect the Firefox extension. It is also an accessibility issue (flashing can be dangerous for users with certain conditions).

The issue is that if the user has started the app with the default light theme, and then switches to SW mode and turns on dark theme, then (in the Firefox extension) the app will initially load with the light theme, and will only switch to dark theme when it reboots into SW mode. This causes a very visible flashing between themes. In our existing code, in any mode, the app initially loads with a light background (the default for index.html) and only inverts once the app is fully loaded.

Possible mitigations:

Most of these would not fully eliminate flashing (possibly the use of CSS custom properties could).

Jaifroid commented 2 years ago

This is called Flash Of Incorrect Theme (FOIT), and is a common problem. Some resources:

n1thun commented 2 years ago

Hey @Jaifroid do you need help fixing this or will you be taking it on yourself?

Jaifroid commented 2 years ago

@n1thun Thank you for the offer. Is this something you have experience with (UI in HTML/JS/CSS)? Looking at the resources I've linked above, it looks fairly complicated to optimize properly. If you are comfortable with the techniques outlined especially in https://webcloud.se/blog/2020-04-06-flash-of-unstyled-dark-theme/ then help would be appreciated.

You should note that working on this issue would need to wait until #771 is merged, because the issue is exposed particularly in that PR, in a Firefox Extension set to run in Service Worker mode, due to the fact that the browser does a redirect to the PWA version of this app in that mode.

n1thun commented 2 years ago

Hey jaiford, I have worked with the preferred color scheme css query before, so think I should be able to handle it. Thanks for the heads up on #771, is there an eta on when that would be merged.

Wr, Nithun


From: Jaifroid @.> Sent: Monday, January 3, 2022 12:12:57 PM To: kiwix/kiwix-js @.> Cc: Nithun Harikrishnan @.>; Mention @.> Subject: Re: [kiwix/kiwix-js] Mitigate against excessive flashing between light and dark themes on startup (Issue #787)

@n1thunhttps://github.com/n1thun Thank you for the offer. Is this something you have experience with (UI in HTML/JS/CSS)? Looking at the resources I've linked above, it looks fairly complicated to optimize properly. If you are comfortable with the techniques outlined especially in https://webcloud.se/blog/2020-04-06-flash-of-unstyled-dark-theme/ then help would be appreciated.

You should note that working on this issue would need to wait until #771https://github.com/kiwix/kiwix-js/pull/771 is merged, because the issue is exposed particularly in that PR, in a Firefox Extension set to run in Service Worker mode, due to the fact that the browser does a redirect to the PWA version of this app in that mode.

— Reply to this email directly, view it on GitHubhttps://github.com/kiwix/kiwix-js/issues/787#issuecomment-1004266375, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAWCEPIGRTQIQB3TXH6GW6TUUHRKTANCNFSM5LEXAJSA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.Message ID: @.***>

Jaifroid commented 2 years ago

OK, thanks! I'm hoping to merge in the next day or two. I'll post here when it's done.

Jaifroid commented 2 years ago

@n1thun Just to let you know, as promised, that #771 has been merged to master now. Please pull latest code. Would you like me to assign this issue to you, if you're still interested?

n1thun commented 2 years ago

@Jaifroid hey can you provide me the directions to reproduce the issue. From what I see the Firefox plugin does seem to switch to/work in SW mode but the issue seems to say that this occurs when we are in SW mode.

Jaifroid commented 2 years ago

@Jaifroid hey can you provide me the directions to reproduce the issue. From what I see the Firefox plugin does seem to switch to/work in SW mode but the issue seems to say that this occurs when we are in SW mode.

@n1thun The extension you download from the Store (or any released version) doesn't have the workaround that allows booting to SW mode. This is in master (it's only just been merged, not released). You could work with one of the nightly builds of the Extension, but this would require using Firefox Developer Edition. What I would suggest is simply working on the Flash Of Incorrect Style in a simple Firefox or Chromium browser pointed at the development implementation here:

https://kiwix.github.io/kiwix-js/

This contains the latest code in master and will stay up to date with the latest code as it is merged.

For heavier-duty development, I'd recommend serving your code from your own machine using (for example) node/npm's http-server. Then you'd serve the root of the repo and point browser to http://localhost:8080 .

Please note this comment about the current need to clear the appCache (Storage tab in Firefox, Application tab in Chromium) before reloading the app, or you won't see changes. I'm working on a way to make this easier for devs.

n1thun commented 2 years ago

Cool, thanks for the details. You can assign this to me.

Wr, Nithun


From: Jaifroid @.> Sent: Tuesday, January 11, 2022 11:18:16 AM To: kiwix/kiwix-js @.> Cc: Nithun Harikrishnan @.>; Mention @.> Subject: Re: [kiwix/kiwix-js] Mitigate against excessive flashing between light and dark themes on startup (Issue #787)

@Jaifroidhttps://github.com/Jaifroid hey can you provide me the directions to reproduce the issue. From what I see the Firefox plugin does seem to switch to/work in SW mode but the issue seems to say that this occurs when we are in SW mode.

@n1thunhttps://github.com/n1thun The extension you download from the Store (or any released version) doesn't have the workaround that allows booting to SW mode. This is in master (it's only just been merged, not released). You could work with one of the nightly builds of the Extension, but this would require using Firefox Developer Edition. What I would suggest is simply working on the Flash Of Incorrect Style in a simple Firefox or Chromium browser pointed at the development implementation here:

https://kiwix.github.io/kiwix-js/

This contains the latest code in master and will stay up to date with the latest code as it is merged.

For heavier-duty development, I'd recommend serving your code from your own machine using (for example) node/npm's http-server. Then you'd serve the root of the repo and point browser to http://localhost:8080 .

Please note this commenthttps://github.com/kiwix/kiwix-js/issues/790#issuecomment-1008669638 about the current need to clear the appCache (Storage tab in Firefox, Application tab in Chromium) before reloading the app, or you won't see changes. I'm working on a way to make this easier for devs.

— Reply to this email directly, view it on GitHubhttps://github.com/kiwix/kiwix-js/issues/787#issuecomment-1010185472, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAWCEPK3E7S3E7U7L3ZGQGLUVRQ5RANCNFSM5LEXAJSA. You are receiving this because you were mentioned.Message ID: @.***>

Jaifroid commented 2 years ago

Thanks, done!

Jaifroid commented 2 years ago

Setting a background_color key in the manifest.json might help a bit with this issue. See https://developer.mozilla.org/en-US/docs/Web/Manifest/background_color.

Niharika-kakumanu-22 commented 1 year ago

Hi there , i know how to do this , can u assign this to me .

Jaifroid commented 1 year ago

@Niharika-kakumanu-22 Thanks for the offer! Could you expand a little on how you propose to fix this issue?

Niharika-kakumanu-22 commented 1 year ago

i will set a hard-coded background in Html and change it, CSS custom properties to force the browser to display a theme based on the prefers-color-scheme media query . or seperate script above html which will initialize theme to localstorage .

Niharika-kakumanu-22 commented 1 year ago

Can u assign this to me .

Jaifroid commented 1 year ago

OK, thanks you @Niharika-kakumanu-22. Assigning you.

Jaifroid commented 1 year ago

Please follow the guidelines in https://github.com/kiwix/kiwix-js/blob/main/CONTRIBUTING.md. Note in particular that the app caches its own code when run from localhost, and this can make it difficult to debug. You need to turn on the option in Expert Settings to bypass the app cache, and you also need to disable browser caching in Dev Tools Network tab. Then, when you do Ctrl-Shift-R while Dev Tools is open, it should fully refresh its code, incorporating any changes you've made.

Niharika-kakumanu-22 commented 1 year ago

heyy, i couldnt see any flashing between light and dark themes but still changed few things which are mentioned in mitigations above and background_color key in the manifest.json and i did npm test where all 44 are success.

Jaifroid commented 1 year ago

@Niharika-kakumanu-22 Thank you! I think that since this issue was created browser vendors themselves may have implemented some internal mitigations against such flashing, because I don't think we implemented anything. Would you like to make a PR with the changes, so I can test?

Niharika-kakumanu-22 commented 1 year ago

yeah sure

Niharika-kakumanu-22 commented 1 year ago

i did PR and can u see and give approval review

Niharika-kakumanu-22 commented 1 year ago

can u see my PR