abhijeetnishal / URLShortener

URL Shortener Deployed Link 👇🏻
https://urlsrtner.vercel.app
MIT License
37 stars 58 forks source link

fixed theme issue #147

Closed AMS003010 closed 5 months ago

AMS003010 commented 6 months ago

Revert PR https://github.com/abhijeetnishal/URLShortener/pull/143 The theme issue has been solved

Please do check the Link ↗️ to the demo of the application

Please do assign the gssoc label Please do assign the necessary label

vercel[bot] commented 6 months ago

@AMS003010 is attempting to deploy a commit to the Abhijeet's projects Team on Vercel.

A member of the Team first needs to authorize it.

AMS003010 commented 6 months ago

@abhijeetnishal please do review This PR is for the Issue #132

abhijeetnishal commented 6 months ago

Hey @AMS003010, use state management library (zustand) to implement this toggle. The current code is not efficient. Also it will be valid for every page.

  1. Install zustand as this library is light weight and efficient.
  2. Create a folder named store in src directory and inside that create a folder name themeStore and create index.tsx and add your logic.
  3. Finally use this everywhere to toggle it.

The themeStore code looks like:

import {create} from 'zustand';

interface ThemeState {
  theme: 'light' | 'dark';
  toggleTheme: () => void;
}

const useThemeStore = create<ThemeState>((set) => ({
  theme: 'light', 
  toggleTheme: () => set((state) => ({ theme: state.theme === 'light' ? 'dark' : 'light' })),
}));

export default useThemeStore;

Use this store to get and set the theme like this: const theme = useThemeStore((state) => state.theme);

AMS003010 commented 5 months ago

Hey @abhijeetnishal, needed some help 😅 Are we using zustand to manage the theme state here instead of next-themes here because if zustand is being used along with next-themes then that would simply make it redundant.

Thank you

AMS003010 commented 5 months ago

So should we remove next-themes completely

abhijeetnishal commented 5 months ago

Hey @abhijeetnishal, needed some help 😅 Are we using zustand to manage the theme state here instead of next-themes here because if zustand is being used along with next-themes then that would simply make it redundant.

Thank you

Hey @AMS003010, you are right. We will remove next-theme and we will use zustand only, as we need state management later also so it's better to use this.

AMS003010 commented 5 months ago

@abhijeetnishal I have made a PR. Please do review.

The changes made are: