pacocoursey / next-themes

Perfect Next.js dark mode in 2 lines of code. Support System preference and any other theme with no flashing
https://next-themes-example.vercel.app/
MIT License
5.16k stars 188 forks source link

Option to use sessionStorage instead of localStorage #72

Open macguirerintoul opened 2 years ago

macguirerintoul commented 2 years ago

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 https://github.com/pacocoursey/next-themes/pull/71. Please let me know what you think!

trm217 commented 2 years ago

My thoughts on: https://github.com/pacocoursey/next-themes/pull/71#issuecomment-1046267203

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.

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.

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 commented 1 year ago

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 commented 1 year ago

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 commented 5 months ago

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 commented 5 months ago

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