styled-components' Global Theme only changes theme after F5 on desktop, but works on mobile-CodePudding

good afternoon! I have a problem using styled-components, precisely the Global Theme, on desktop for my portfolio. When I request to change theme, it works on mobile page, but I have to refresh the page to work in the desktop version. I want to know where's my problem, please.

const SetTheme = () => {

    const [theme, setTheme] = useState('light');

    window.addEventListener("load", () => {
        localStorage.getItem('theme') === null ? setTheme('light') : setTheme(localStorage.getItem('theme'));
    })

    var newTheme;

    const themeToggler = () => {
        theme === 'light' ? newTheme = 'dark' : newTheme = 'light';
        setTheme(newTheme);
        localStorage.setItem('theme', newTheme);
    }

    return ( 
        <ThemeProvider theme={theme === 'dark' ? darkTheme : lightTheme}>
        <GlobalStyles/>
        <div className="setTheme-button">
            <img src={ halfSunMoon } className='halfSunMoon' alt='halfSunMoon' onClick={ themeToggler } />
        </div>
        </ThemeProvider>
    );
}

The complete code is here

Thanks for the time!

CodePudding user response：

You shouldn't listen to a windows event directly in a render function, it should be in a useEffect and also call removeEventListener in the return. You also don't need to listen for load to read localStorage anyway, your state can do it sync ... = useState(() => localStorage.getItem('theme') || 'light').

Your toggle function is also bad practice, there is no need for an intermediary variable and it will be forgotten every render. If you needed a persistent variable you should use useRef but you don't need one.

const SetTheme = () => {
    const [theme, setTheme] = useState(() => localStorage.getItem('theme') || 'light');

    const themeToggler = () => setTheme(theme => {
      const nextTheme = theme === 'light' ? 'dark' : 'light';
      localStorage.setItem('theme', nextTheme)
      return nextTheme;
    })

    return ( 
        <ThemeProvider theme={theme === 'dark' ? darkTheme : lightTheme}>
        <GlobalStyles/>
        <div className="setTheme-button">
            <img src={ halfSunMoon } className='halfSunMoon' alt='halfSunMoon' onClick={ themeToggler } />
        </div>
        </ThemeProvider>
    );
}

CodePudding user response：

The problem, most likely, is with your newTheme variable, that, at the end, is not even needed if you refactor your code to make it more readable, change the code to something like (just a personal preference, more to give you an idea, than something strictly needed):

const [theme, setTheme] = useState(undefined);

const updateTheme = (updatedTheme = undefined) => {
  const localStorageTheme = localStorage.getItem('theme');
  const currentTheme = updatedTheme || localStorageTheme || 'light';
  setTheme(currentTheme);
  localStorage.setItem('theme', currentTheme);
}

useEffect(() => {
  updateTheme()
}, []);

const themeToggler = () => {
  updateTheme(theme === 'light' ? 'dark' : 'light');
}

This way you will delegate all the logic behind theming to a single function, that can be, eventually, moved outside the component to make tests easier (with small changes, of course).

It is preferable to avoid using native event listeners to let React handle its data (state and props) properly.