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

Option to use sessionStorage instead of localStorage #72

Open
macguirerintoul opened this issue Dec 29, 2021 · 5 comments · May be fixed by #293
Open

Option to use sessionStorage instead of localStorage #72

macguirerintoul opened this issue Dec 29, 2021 · 5 comments · May be fixed by #293
Labels
enhancement New feature or request v1 Planned for v1

Comments

@macguirerintoul
Copy link

Hello! I noticed after adding this to my project that because it uses localStorage, the theme always persists between sessions.

When someone visits my site, I'd prefer to have it get the default theme first (i.e. system), if for example the system theme has changed from their previous session because they're visiting at a different time of day.

I've implemented this in #71. Please let me know what you think!

@pacocoursey pacocoursey added the enhancement New feature or request label Jan 4, 2022
@pacocoursey pacocoursey added the v1 Planned for v1 label Feb 20, 2022
@trm217
Copy link
Collaborator

trm217 commented Mar 1, 2022

My thoughts on: #71 (comment)

Don't you think having a generic option might be a bit over engineered for the common use cases?

I thought of the following implementation would solve #72 quiet nicely.

  • Add a storage property: the accepted values are : none, sessionStorage, localStorage.

In regards to a 'generic' custom solution :
I think it would be cool if it was possible to hook into the theme changes, to for example allow
the syncing of a users theme-preferences with a data-storage.

  • For custom storage options we could define a simple interface. I believe something like this might make sense:
   interface ExternalThemeStorage extends EventTarget { // EventTarget could be an overkill tbh
      get: () => Promise<string | null>
      set: (value: string) =>  Promise<void>
      delete: () => Promise<void>
   }

An object implementing this interface could then be passed to the customStorage property to be used inside the ThemeProvider.

I think allowing the user to combine both the storage property and customStorage property could be very useful,
since saving a users preferences over multiple devices is fairly common.

@mrjackyliang
Copy link

I don't want to be picky, it is a very nice integration.

But...

I am looking to store the theme info in Redux and have the provider read directly from Redux store instead.

@omarabid
Copy link

I don't want to be picky, it is a very nice integration.

But...

I am looking to store the theme info in Redux and have the provider read directly from Redux store instead.

Have you made any progress on that?

@ciruz
Copy link

ciruz commented May 14, 2024

Hey, any news on this? Having the theme in a session storage if you need this option, sounds like a good use case to me.

@trm217
Copy link
Collaborator

trm217 commented May 14, 2024

@ciruz a pr for this is open and waiting for pacos review.

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

Successfully merging a pull request may close this issue.

6 participants