Theme Switcher App

Posted on

Problem

I am a student who wrote a theme switcher, but I don’t know if everything is ok.

I want professionals to review my code and point out mistakes. I accept all recommendations and criticism.

gitHub: https://github.com/Alexander-Korniichuk/React_Theme_Switcher/commit/f70f3cceb81c484d4dab0bd7799cd234ee39d4fe

src/components/ThemeSwitcherApp/

import React, {useEffect, useState, useMemo, useCallback} from "react";
import DarkStyles from "../DarkStyles/DarkStyles";

export const ThemeSwitcherApp = () => {
    const key = "DARK_THEME";
    const saveJSON = (key, data) => {
        if (window[`localStorage`] === null) return
        return localStorage.setItem(key, JSON.stringify(data));
    };
    const loadJSON = key => {
        if (window[`localStorage`] === null) return
        return key && JSON.parse(localStorage.getItem(key));
    };

    const [themeIsActive, setThemeIsActive] = useState(loadJSON(key) ?
        loadJSON(key).darkMode : false);
    const [button_text_value, setButton_text_value] = useState(themeIsActive ? "Turn off" : "Turn on");

    const theme_data_false = useMemo(() => ({darkMode: false}), []);
    const theme_data_true = useMemo(() => ({darkMode: true}), []);

    const insertDarkStyles = (where) => {
        const style = document.createElement(`style`);
        style.setAttribute(`id`, `dark-mode`);
        style.innerHTML = DarkStyles;
        where.append(style);
    };

    const ActiveDarkTheme = useCallback(
        () => {
            insertDarkStyles(document.getElementsByTagName(`head`)[0]);
            setButton_text_value("Turn off");
            saveJSON(key, theme_data_true);
        }, [theme_data_true]
    )
    const DeactivateDarkTheme = useCallback(
        () => {
            const isDarkModeExist = !!document.getElementById(`dark-mode`);
            if (isDarkModeExist) {
                document.getElementById(`dark-mode`).remove();
                setButton_text_value("Turn on");
                saveJSON(key, theme_data_false);
            }
        }, [theme_data_false]
    )

    const ThemeStorageLoading = useCallback(
        (key, data) => {
            if (!localStorage.getItem(key)) {
                saveJSON(key, data);
            } else {
                if (loadJSON(key).darkMode) {
                    ActiveDarkTheme();
                } else {
                    DeactivateDarkTheme();
                }
            }
        }, [ActiveDarkTheme, DeactivateDarkTheme]);

    useEffect(() => { // Once Loading
        ThemeStorageLoading(key, theme_data_false);
    }, [theme_data_false, ThemeStorageLoading]);

    const ButtonAction = (bool_value) => {
        setThemeIsActive(bool_value);
        if (bool_value) ActiveDarkTheme();
        else DeactivateDarkTheme();
    };

    return (
        <div>
            <input type="button"
                   onClick={() => ButtonAction(!themeIsActive)}
                   defaultValue={button_text_value}
                   style={{cursor: "pointer"}}
            />
        </div>
    );
}
```

Solution

  • variable key can relate to anything. I’d just call this variable theme
  • I’d also avoid naming the function parameter where
  • If You are using an arrow function and it has one parameter then You can avoid adding () around it
  • Why can’t You just use button instead of <input type="button">?
  • style={{cursor: "pointer"}} I’d remove that from the JSX and place that in the css document
  • Sometimes you name things with camelCase and sometimes You use such_a_thing. Try to stay consistent
  • Function names inside Your component should start with lowercase
  • It’s a good practice to use {} to wrap the blocks of code in Your if/else statements
  • Additionally, I’d change the approach a little bit. If I had the task to do the theme switching functionality, I’d create the toggle which will change the isDarkMode flag to true/false. If the flag is true, then I would add the dark-mode-active on the body element of the page. After that I would create the proper CSS file and put the dark mode related styles to the class we’ve just attached to the body element.
  • Once loading this comment is not necessary. Use comments to describe some workarounds / hacks / code tricks but such a fundamental functionality like useEffect does not require any comments 🙂

I hope that You will find the above feedback useful. Good job with the above and good luck with the improvements

Leave a Reply

Your email address will not be published. Required fields are marked *