nuxt-modules / color-mode

Dark and Light mode with auto detection made easy with Nuxt 🌗
https://color-mode.nuxtjs.org
MIT License
1.08k stars 99 forks source link

Feat/sync cookie #188

Closed Hossein-Mirazimi closed 1 day ago

Hossein-Mirazimi commented 1 year ago

this MR about sync localStorage with cookie

netlify[bot] commented 1 year ago

Deploy Preview for nuxt-color-mode canceled.

Name Link
Latest commit 9ee19d9511daf4ef2c16410ee27ce7ba92a2d2a1
Latest deploy log https://app.netlify.com/sites/nuxt-color-mode/deploys/64fc45595db70e00087dd5d2
atinux commented 1 year ago

Why adding the cookie sync @Hossein-Mirazimi ?

Hossein-Mirazimi commented 1 year ago

Why adding the cookie sync @Hossein-Mirazimi ?

because in some cases may need to get preference value from cookies instead of local storage

186

tasiotas commented 1 year ago

Yep, for subdomain

When I visit example.com and set dark mode, it won't get carried over to subdomain.example.com without cookies

atinux commented 1 year ago

What about GPRD if we use cookies then? For EU websites they will have to display the cookies banner?

tasiotas commented 1 year ago

As far as I understand GDPR, it does not specify technology used for storage at all. It never uses term "cookie" or "local storage", it's about storing data for given reason.

So even with current method of using local storage, GDPR does apply.

The exceptions are outlined here, but bit hard to understand.

I would like to believe that color theme saved in cookie/storage is necessary to deliver what visitor has asked for. But is that accurate interpretation of EU law, I don't know.

wokalek commented 1 year ago

Really need this feature. But aware of GPRD must will be included when enable this setting.

Hossein-Mirazimi commented 1 year ago

please help me add the GPRD rule for this

I don't know how to add this rule in the color-mode module

because this repo is just for managing the theme

If we should policy to set cookies for the theme

developers must do it in their project

atinux commented 1 year ago

It would be nice to have a GPRD expert to know what we should do about this

hacknug commented 1 year ago

According to GitHub it is not necessary: https://github.blog/2020-12-17-no-cookie-for-you/

EDIT: Also, both localStorage and sessionStorage are considered cookies from a legal stand point so I don't think this PR introduces any new legal liabilities.

What we should probably discuss is whether or not this module should store any preferences before the user interacts with an element that changes $colorMode.preference.

I also think it might be better to add a mode option to the module that defaults to localStorage and allows 'localStorage'|'sessionStorage'|'cookies'. This way anyone can decide where to store these preferences and we don't need to touch multiple stores (not sure if this change would affect other modules that could depend on this though).

Hossein-Mirazimi commented 1 year ago

please review my changes

Jordan-Ellis commented 12 months ago

Since this pull request adds the ability to choose the storage type, it would be great if you could also specify a custom storage provider. For example, I'm using ionic and would prefer to store this in the native settings instead of local storage.

Something along the lines of this. The only thing I'm not sure of is where you would place the provider.

// Custom settings store. Could be pinia store, native settings library, etc.
const mySettings = useMySettings() 

// Settings provider passed to color mode
const colorModeSettingProvider = {
    getPreference() {
        return mySettings.get("theme")
    },
    setPreference(value) {
        mySettings.set("theme", value)
    }
}
atinux commented 12 months ago

It is a nice idea @Jordan-Ellis but the biggest issue is that it is written inside the inject script: https://github.com/nuxt-modules/color-mode/blob/961a4ab96a1a0e63bee34a4b86041558a6f0519c/src/script.ts#L9

This code is added in the <head> of the project in order to properly handle static site generation to avoid having a color switch on hydration. This script can only be JavaScript code that run in the browser and don't have access to any of the Vue runtime.

Jordan-Ellis commented 12 months ago

Ah, I didn’t realize that! Should that be moved to a separate issue, or is that beyond the scope of the project right now?

manniL commented 9 months ago

Ah, I didn’t realize that! Should that be moved to a separate issue, or is that beyond the scope of the project right now?

@Jordan-Ellis yup 👍

manniL commented 9 months ago

Another missing feature would be be adding cookie reading to plugin.server.ts to already have the info on the server (if a cookie is set).

Created a follow-up in #221 (have no permissions to edit this)

Nisthar commented 6 months ago

I got this error when using subdomains in my nuxt app. The color scheme was working fine on the main domain but not on the subdomains. Are there any workarounds for this issue or should i stop using nuxtui atm?

atinux commented 1 day ago

Closing in favour of #221