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

[Feature] Use Dark Theme by Default #149

Closed
Char2sGu opened this issue Apr 3, 2024 · 7 comments
Closed

[Feature] Use Dark Theme by Default #149

Char2sGu opened this issue Apr 3, 2024 · 7 comments
Assignees
Labels
Type: Enhancement New feature or request

Comments

@Char2sGu
Copy link
Contributor

Char2sGu commented Apr 3, 2024

Description

When using SSR, the rendered page is initially displayed in the default light theme. If the user's OS uses a dark palette and the theme switching is based on the OS settings, this would cause a dazzling white flash on page load.

Imagine this: A developer is sitting in a dark room focusing on learning a library. When he clicks into the documentation website powered by NgDoc, the screen suddenly turns into a flashbang.

Proposed solution

I believe rendering a dark theme by default would harm less, since starting from a dark theme and then turn into a light theme could be less "surprising" than the opposite way.

Alternatives considered

Or let's change the definition of a "theme". A theme must include two color schemes: light and dark. By default, the color scheme follows the prefers-color-scheme query, and after the hydration is finished, the application can apply additional classes on the <html> or <body> element to force a color scheme.

@Char2sGu Char2sGu added the Type: Enhancement New feature or request label Apr 3, 2024
@skoropadas
Copy link
Member

This is a known bug that appeared with the transition to SSR. It will be addressed within another ticket (#126 which includes a discussion of this issue), so I'll close this one as a duplicate.

As for the default theme, I think I'll leave everything as is to avoid sudden breaking changes. In any case, with the bug fix, there shouldn't be any more issues with the dark theme.

@Char2sGu
Copy link
Contributor Author

Char2sGu commented Apr 6, 2024

Glad to know the default theme fix is already in progress.

However, what do you think of the first screen before hydration?

It is theoretically not possible to know the user's prefer-color-scheme at the server side. If we render one theme by default and then change to a dark theme based on the user setting after hydration, there will definitely be a flash.

My proposal is to combine the light scheme and dark scheme into one theme. The scheme to display is determined by prefer-color-scheme by default, but can be explicitly specified through html classes after hydration.

In other words, we split the concept of color scheme and theme. A theme consists of two color schemes: light and dark.

By doing so, we can make sure the color scheme of the first screen matches the user's system setting. If the user has explicitly specified a color scheme that doesn't match the system setting, we can then change the theme after the hydration. There will still be a flash in this case, but at least the first screen matches the system setting.

@skoropadas
Copy link
Member

Switching to the dark theme by default still doesn't solve the problem. Before transitioning to SSR, the necessary theme was loaded via HTTP during the application initialization stage, and there was no flash. Now, it occurs more because the browser first renders what the server sends and then executes scripts. Therefore, if, for example, you move this logic into a separate script and include it in index.html, there won't be any flash.

So I think the only solution could be integrating a script inside index.html that sets the required theme, similar to what angular.dev does. But anyways, I'll consider this and look for solutions that would fix this flash.

@Char2sGu
Copy link
Contributor Author

Char2sGu commented Apr 6, 2024

I think you misunderstood what I meant.

Using dark theme by default is just a temporary fix that reduces the impact, because a dark flash within a white screen is less harmful than a light flash within a dark screen.

I understand how it is difficult to load the expected theme at the server side, which is exactly what I explained before. Therefore, my proposal is to change the concept of a theme.

Previously, themes can be:

- default-light.theme
- default-dark.theme
- custom-light.theme
- custom-dark.theme

I suggest to distinguish the concept of "color scheme" and "theme". A "theme" must include two color schemes: light and dark:

- default.theme
  - light.scheme
  - dark.scheme
- custom.theme
  - light.scheme
  - dark.scheme

In other words, the light and dark schemes comes naturally with a theme's CSS files. The two schemes are always loaded together.

Within a theme's stylesheet, it could be something like this.

default.theme.scss:

@media (prefer-color-scheme: light) {
  :root {
    @include light-scheme();
  }
}
@media (prefer-color-scheme: dark) {
  :root {
    @include dark-scheme();
  }
}
:root[data-scheme=light] {
    @include light-scheme();
}
:root[data-scheme=dark] {
    @include dark-scheme();
}
  • Before hydration (before the script are loaded), the color scheme follows the prefer-color-scheme media query, so we can always ensure that it matches the user's operating system's default setting.
  • After hydration (after the scripts are executed), the color scheme can be programmatically determined by the data-scheme attribute of the <html> element.

In most cases, the user would expect to see the color scheme to match their OS settings, so this solution works perfectly fine.
In rare cases, the user might specify a different color scheme from their OS setting, e.g. use a light scheme when the OS setting is dark. This would cause a flash, but it wouldn't harm the user's eyes, because the initial color scheme always matches the OS setting.

@skoropadas
Copy link
Member

Thank you for the explanation. I like this solution. I think I'll do something similar. I'll need to think a little more because previously I also wanted to add a layout setting to the theme, so that it would be possible to make fullscreen themes, which requires preconfiguring the sidebar. But now it seems to me that this should not be part of the theme but rather just separate layout settings.

@Char2sGu
Copy link
Contributor Author

Char2sGu commented Apr 7, 2024

Would you like me to create a new ticket for that?

@skoropadas
Copy link
Member

Would you like me to create a new ticket for that?

There's no need for that, I'll do everything when I start working on the #126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants