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

Flashing seen when using dark theme. #227

Closed
rohit-nair opened this issue Feb 17, 2019 · 4 comments
Closed

Flashing seen when using dark theme. #227

rohit-nair opened this issue Feb 17, 2019 · 4 comments

Comments

@rohit-nair
Copy link
Contributor

rohit-nair commented Feb 17, 2019

First off, @idank thanks for setting up this great resource and thanks to @anestv for the dark theme implementation.

One of the things I noticed while using dark theme is that when navigating to pages, we see flashing (shows up in light mode then switches to dark mode). This is happening as the logic to trigger dark mode happens client side on document.ready event.

Fix for this would be to do this server side while generating initial page response. For this the server needs to know about the user's theme setting. This can be achieved by setting it in the cookie when user selects dark mode for the first time and reading that cookie value for subsequent responses.

I've the changes working locally and will setup a PR for this.

Let me know your thoughts.

rohit-nair added a commit to rohit-nair/explainshell that referenced this issue Feb 17, 2019
…e by writing and reading cookie. Also renamed 'default' theme as Light consistent with other implementations.
@idank
Copy link
Owner

idank commented Feb 17, 2019

Thanks for the pull request, looks good in general. I wonder if there's a client side way to prevent this though. Right now we're setting the theme at $(document).ready, is there an event that triggers before that we can use such that the flashing isn't visible?

@rohit-nair
Copy link
Contributor Author

Perhaps the logic could be part of the initial page markup that gets sent and gets executed on parsing which would reduce the duration of flash a bit but won't completely remove it imo.

@idank
Copy link
Owner

idank commented Feb 18, 2019

Sadly I don't have time to look into other approaches myself. As long as you're confident that this is how others are solving this then I'm fine with taking it as is.

@rohit-nair
Copy link
Contributor Author

Maybe @anestv or other contributors could pitch in. I'm ok with keeping the PR open for a bit.

@idank idank closed this as completed in bcad627 Aug 14, 2019
MichaelBergquistSuarez added a commit to MichaelBergquistSuarez/explainshell that referenced this issue Nov 17, 2019
* master:
  web/ui: setting theme server side to prevent flashing (fixes idank#227)
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

2 participants