8000 feat(nuxt): SSR-friendly color mode preference implementation by ahmedrangel · Pull Request #5662 · scalar/scalar · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(nuxt): SSR-friendly color mode preference implementation #5662

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 16, 2025
Merged

feat(nuxt): SSR-friendly color mode preference implementation #5662

merged 2 commits into from
May 16, 2025

Conversation

ahmedrangel
Copy link
Contributor

Problem

Currently, the nuxt module does not save the user's color mode preference when using the toggle switch, it is tied to starting with the one defined in the config.

Solution

With this PR we'll add support for saving the user's color mode preference and handle their states.

I look forward to your feedback.

Checklist

I’ve gone through the following:

  • I’ve added an explanation why this change is needed.
  • I’ve added a changeset (pnpm changeset).
  • I’ve added tests for the regression or new feature.
  • I’ve updated the documentation.

marclave
marclave reviewed May 15, 2025
View reviewed changes
Copy link
Member
@marclave marclave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this PR @ahmedrangel ✨

going to add @amritk to take a look at this as they did a ton of the nuxt work <3

@amritk
Copy link
Member
amritk commented May 16, 2025

So this does work as long as you don't mind the flash of incorrect color in the beginning as its coming from local storage. Since we are on nuxt, ideally we could set a cookie then read it on the server to get the correct color from the beginning.

If you don't mind the flash we can merge this then add the cookie solution when we do SSR.

hanspagel
hanspagel previously approved these changes May 16, 2025
View reviewed changes
Copy link
Member
@hanspagel hanspagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sick PR

I’d tend to avoid the color flash (flashing colors aren’t great for some people), but not a super strong opinion here.

@hanspagel hanspagel self-requested a review May 16, 2025 09:08
@hanspagel hanspagel changed the title feat(nuxt): ssr-friendly color mode preference implementation feat(nuxt): SSR-friendly color mode preference implementation May 16, 2025
@ahmedrangel
Copy link
Contributor Author
ahmedrangel commented May 16, 2025 •
edited
Loading

Do you see a flash while testing the PR? In my end that does not happen.

If so, I thought this part would avoid it

scalar/integrations/nuxt/src/runtime/components/ScalarApiReference.vue

Lines 50 to 68 in d0b7a6f

useHead({
script: [
{
// Inject dark / light detection that runs before loading Nuxt to avoid flicker
// This is a bit of a hack inspired by @nuxtjs/color-mode, but it works
id: 'scalar-color-mode-script',
tagPosition: 'bodyClose',
innerHTML: `((isDark, forced) => {
try {
const stored = window.localStorage.getItem('colorMode');
const useDark = forced === 'dark' || !forced && (stored === 'dark' || !stored && isDark);
window.document.body.classList.add(useDark ? 'dark-mode' : 'light-mode');
} catch {}
})(${isDark.value}, ${JSON.stringify(forcedMode)});`
.replace(/[\n\r]/g, '')
.replace(/ +/g, ' '),
},
],
})

In any case, I agree that a cookie should be used as the ideal SSR solution.

If this solution is not convincing, we can close this PR. <3

@amritk
Copy link
Member
amritk commented May 16, 2025

I actually did not test it, but I know that we cannot use localStorage for SSR. That being said I don't think the SSR is quite working anymore anyway so maybe its fine. I'll test it and see 👍🏾

amritk
amritk approved these changes May 16, 2025
View reviewed changes
Copy link
Member
@amritk amritk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep just tested and there is no flash because its not doing SSR. We'll take it for now thanks for the PR! I'll do a proper cookie fix when we fix SSR

@amritk amritk merged commit 3113a01 into scalar:main May 16, 2025
27 checks passed
@ahmedrangel
Copy link
Contributor Author

Good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marclave marclave marclave left review comments

@amritk amritk amritk approved these changes

@geoffgscott geoffgscott Awaiting requested review from geoffgscott geoffgscott is a code owner

@hanspagel hanspagel Awaiting requested review from hanspagel hanspagel is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ahmedrangel @amritk @hanspagel @marclave
0