-
Notifications
You must be signed in to change notification settings - Fork 112
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
Feat/sync cookie #188
Feat/sync cookie #188
Conversation
✅ Deploy Preview for nuxt-color-mode canceled.
|
Why adding the cookie sync @Hossein-Mirazimi ? |
because in some cases may need to get preference value from cookies instead of local storage |
Yep, for subdomain When I visit |
What about GPRD if we use cookies then? For EU websites they will have to display the cookies banner? |
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. |
Really need this feature. But aware of GPRD must will be included when enable this setting. |
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 |
It would be nice to have a GPRD expert to know what we should do about this |
According to GitHub it is not necessary: https://github.blog/2020-12-17-no-cookie-for-you/ EDIT: Also, both What we should probably discuss is whether or not this module should store any preferences before the user interacts with an element that changes I also think it might be better to add a |
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…mode into feat/sync-cookie
please review my changes |
function setPreferenceToStorage (storageType: typeof storage, preference: string) { | ||
switch (storageType) { | ||
case 'cookie': | ||
window.document.cookie = storageKey + '=' + preference |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need ability to pass extra cookie options such as expires
, max-age
, path
, samesite
, secure
, etc.
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)
}
} |
It is a nice idea @Jordan-Ellis but the biggest issue is that it is written inside the inject script: Line 9 in 961a4ab
This code is added in the |
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 👍 |
|
||
// @ts-ignore | ||
function getStorageValue(storageType, storageKey) { | ||
if (typeof window === 'undefined') return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (typeof window === 'undefined') return null; | |
if (window?.document === 'undefined') return null; |
The other code will break on build.
Another missing feature would be be adding cookie reading to Created a follow-up in #221 (have no permissions to edit this) |
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? |
Closing in favour of #221 |
this MR about sync localStorage with cookie