Rishabh-malhotraa / caucus

Realtime Collaborate Editor with Embedded Compiler
https://caucus.rishabhmalhotra.in/
MIT License
295 stars 70 forks source link

Added: Light Theme Mode #63

Closed GaganpreetKaurKalsi closed 1 year ago

GaganpreetKaurKalsi commented 2 years ago

44 : Successfully added Light Mode to caucus

I have added classes such as tone1, tone2, tone3 to the components. In future, if you would like to add other themes to the components, you just need to specify it in the Theme.style.css file. Below are some screenshots for your reference. image image image image image

Kindly review and let me know if I need to do something on my part. Thanks!

Rishabh-malhotraa commented 2 years ago

@all-contributors please add @GaganpreetKaurKalsi for code and design

allcontributors[bot] commented 2 years ago

@Rishabh-malhotraa

I've put up a pull request to add @GaganpreetKaurKalsi! :tada:

Rishabh-malhotraa commented 2 years ago

I am not a big fan of how CSS modules way of switching themes. I am tempted to switch to styled-components for all things CSS, but that would be a big restructure. What do you think?

GaganpreetKaurKalsi commented 2 years ago

I am not a big fan of how CSS modules way of switching themes. I am tempted to switch to styled-components for all things CSS, but that would be a big restructure. What do you think?

Definitely it would be a big restructure. The problem is that many classes are not defined by the project builder. They are inbuilt classes from modules. If you would like to switch to styled-components, no problem. It will also help removing the inline styles and then one doesn't have to quote "!important" everywhere. I am also not a big fan of normal css, especially when working with react/typescript.

Rishabh-malhotraa commented 2 years ago

I am not a big fan of how CSS modules way of switching themes. I am tempted to switch to styled-components for all things CSS, but that would be a big restructure. What do you think?

Definitely it would be a big restructure. The problem is that many classes are not defined by the project builder. They are inbuilt classes from modules. If you would like to switch to styled-components, no problem. It will also help removing the inline styles and then one doesn't have to quote "!important" everywhere. I am also not a big fan of normal css, especially when working with react/typescript.

Let me do some research about the pros and cons of styled-components, because CSS modules are really annoying when you require to write some complex CSS. @GaganpreetKaurKalsi Have you used any CSS in JS solution before like JSS, emotion or styled components?

GaganpreetKaurKalsi commented 2 years ago

I am not a big fan of how CSS modules way of switching themes. I am tempted to switch to styled-components for all things CSS, but that would be a big restructure. What do you think?

Definitely it would be a big restructure. The problem is that many classes are not defined by the project builder. They are inbuilt classes from modules. If you would like to switch to styled-components, no problem. It will also help removing the inline styles and then one doesn't have to quote "!important" everywhere. I am also not a big fan of normal css, especially when working with react/typescript.

Let me do some research about the pros and cons of styled-components, because CSS modules are really annoying when you require to write some complex CSS. @GaganpreetKaurKalsi Have you used any CSS in JS solution before like JSS, emotion or styled components?

I have worked with styled-components.

GaganpreetKaurKalsi commented 2 years ago

I have made all the requested changes. You can have a look now!

Rishabh-malhotraa commented 2 years ago

@GaganpreetKaurKalsi After doing some research about how to implement different themes, I think CSS modules would work well too, we just need to do some workaround.

If we do this we can also remove all the CSS-in-JS we have used at different places, like we do here

const lightTheme = {
  "--primary": "#31e981",
  "--seconday": "#000000",
  };

//This is our dark mode
const darkTheme = {
  "--primary": "#286843",
  "--seconday": "#ffffff",
};
export { lightTheme, darkTheme };
const ApplyTheme = ({ theme, children }) => {
  //Update the CSS Variables
  const updateCSSVariables = theme => {
    const arrayOfVariableKeys = Object.keys(theme);
    const arrayOfVariableValues = Object.values(theme);

    //Loop through each array key and set the CSS Variables
    arrayOfVariableKeys.forEach((cssVariableKey, index) => {
      //Based on our snippet from MDN
      document.documentElement.style.setProperty(
        cssVariableKey,
        arrayOfVariableValues[index]
      );
    });
  };

Here is an article which explains theming in details

GaganpreetKaurKalsi commented 2 years ago

@GaganpreetKaurKalsi After doing some research about how to implement different themes, I think CSS modules would work well too, we just need to do some workaround.

  • Since MUI injects the CSS at the end of the HTML it takes precedence over all the CSS we write in our css.modules file and we have to write !important everywhere, we can fix this by using
<StyledEngineProvider injectFirst>
  {/* Your component tree. Now you can override MUI's styles. */}
</StyledEngineProvider>

MUI Docs Explaining the same

If we do this we can also remove all the CSS-in-JS we have used at different places, like we do here

  • the way I want to implement different themes is by using a JSON object and change the CSS-variable values using JS
const lightTheme = {
  "--primary": "#31e981",
  "--seconday": "#000000",
  };

//This is our dark mode
const darkTheme = {
  "--primary": "#286843",
  "--seconday": "#ffffff",
};
export { lightTheme, darkTheme };
const ApplyTheme = ({ theme, children }) => {
  //Update the CSS Variables
  const updateCSSVariables = theme => {
    const arrayOfVariableKeys = Object.keys(theme);
    const arrayOfVariableValues = Object.values(theme);

    //Loop through each array key and set the CSS Variables
    arrayOfVariableKeys.forEach((cssVariableKey, index) => {
      //Based on our snippet from MDN
      document.documentElement.style.setProperty(
        cssVariableKey,
        arrayOfVariableValues[index]
      );
    });
  };

Here is an article which explains theming in details

Okay! I got it. But this workaround is quite huge. I would suggest you merge this pull request first and open up another issue and assign that to me. I will work on it then. Also it would be great if you could merge the others as well, as I am participating in Hacktoberfest for the first time and it means a lot to me. Thanks!