Added a Togglable Dark Mode and Refactored Existing CSS Color Variables #535

trevorkw7 · 2022-02-01T02:31:15Z

Feature description

A togglable dark mode switch has been added to the header of the website to make night viewing of the portal easier on the eyes. In addition, colors used across the entire project have now been refactored into CSS color variables defined in vars.less and reset.less, making it easier to tweak colors at a universal level.

Screenshots

Behavior Changes

The original LESS CSS variables that existed in vars.less have now been refactored as Native CSS variables that are defined in reset.less to be accessible anywhere within the body of the website. There are two sets of these variables with the same name but with different color values, one set for when the body class name is "light-theme", and the other set for when the body class name is "dark-theme". Despite this, vars.less continues to exist to define the hex codes for the colors themselves and are referenced in reset.less.

The new color system is weighted, with theme-color-primary-1 having the lightest shade of the primary color and theme-color-primary-6 having the darkest.

Feedback

To test, pull the code and run yarn install and yarn start like normal. Please let me know if you have any feedback or suggestions by replying to this request :0

Dashboard, Profile, About, Admin, Discord Pages Have Dark Mode Ready

…embership-portal-ui into colors-and-dark-mode

steets250

Wow, the dark theme looks great! Couple comments / issues but overall looks super solid. Thanks for taking the time to clean up all the color styling variables we had.

For the portal, we use yarn as our package manager, so we don't want to have a package-lock.json file in the repo. Let's remove this from the PR but make sure yarn install is run to keep things synced correctly.
The code isn't currently passing the linting tests. Before pushing, it's generally a good idea to run the yarn run lint:fix command to automatically clean the code and notify of any issues. If you use Visual Studio Code install the eslint extension, then it runs the linter upon file save.
For dark mode on https://deploy-preview-535--members-nightly.netlify.app/register, the icons don't switch to light colors. Let me know if you have issues fixing this.
For dark mode on https://deploy-preview-535--members-nightly.netlify.app/about, the Facebook logo doesn't display correctly. I'll DM you an updated svg, but I want to make sure to have a note of this as a to-do.

src/admin/components/AdminPage/style.less

src/assets/icons/moon.svg

src/assets/icons/sun.svg

src/index.tsx

src/layout/components/Header/custom.less

src/leaderboard/components/LeaderPage/style.less

src/profile/components/ProfilePage/style.less

src/styles/ThemeContext/themeContext.tsx

duanwilliam

some pretty nice work, still rough around the edges though and doesn't "properly" address dark mode in react per se

not to shill or anything but https://github.com/acmeet/acmeet/tree/main/client/src/context/Theme (honestly id refactor it more now, eg it has an unnecessary usememo for initial theme + handles localstorage stuff on its own instead of custom hook) can be used as a general reference. do note that that uses [data-theme=dark] bound to the html element as opposed to a class or something on body though, and that InitialThemeScript goes in the head, but since portal is CRA the direct html can just be inserted in the index.html template

src/layout/components/DiscordPage/index.tsx

src/styles/ThemeContext/themeProvider.tsx

duanwilliam · 2022-02-01T06:20:47Z

src/styles/ThemeContext/themeProvider.tsx

+    const [theme, setTheme] = useState("light");
+
+    const toggleTheme = () => {
+        // change body className
+        let currentTheme = document.body.className;
+        let newTheme = currentTheme === 'light-theme' ? 'dark-theme' : 'light-theme';
+        document.body.className! = newTheme
+        // update state
+        setTheme(theme === 'light' ? 'dark' : 'light');
+    };
+
+    useEffect(() => {
+        // sync theme state with localStorage on first render
+        const localTheme = localStorage.getItem('theme');
+        if (localTheme) {
+            setTheme(localTheme);
+        }
+        // if dark, toggle theme to light on first render
+        if (localTheme === "dark") {
+            document.body.className! = 'dark-theme';
+        }
+    }, []);
+
+    useEffect(() => {
+        // sync localStorage with theme state when it changes
+        localStorage.setItem('theme', theme);
+    }, [theme]);


can abstract persistent state to a custom hook

If this line is just moved to within toggleTheme, is there an issue? I don't see how a custom hook is necessary

src/styles/ThemeContext/themeProvider.tsx

duanwilliam · 2022-02-01T06:21:44Z

src/styles/reset.less

@@ -1,5 +1,7 @@
+@import "./vars.less";


might be better to define the css variables in a separate file

also, would prob be better to have e.g. light mode variables be default, then override with dark mode selector

src/admin/components/CreateEventPage/style.less

public/index.html

src/admin/components/AddAttendanceForm/style.less

src/admin/components/AddAttendancePage/style.less

src/admin/components/AdminPage/style.less

trevorkw7 · 2022-02-01T07:24:27Z

Thanks for all the feedback guys, I'll take a look at each and every one of your comments and try incorporating them into the branch. This is my first time really contributing to a React project so thanks for being patient with me. I'm learning a lot of valuable information about how to write cleaner and higher spec code which is really exciting 😉

farisashai · 2022-02-27T08:28:38Z

src/styles/reset.less

@@ -50,6 +136,7 @@ video {
  border: 0;
  margin: 0;
  padding: 0;
+  transition: background 0.3s ease;


looking super clean - nice work!

one small nitpick: if you toggle dark mode on /register page, the major and graduation year's dropdown fields don't fade colors in sync with everything else on the page.

this is probably caused by multiple transition css properties on a single element taking incorrect precedence in the stylesheets (some of ant design's dropdown css is being used instead of yours). here's a really good example of the simplest case of this happening https://stackoverflow.com/a/34757242

If you look at the computed style on the right side, we can see that the line transition: background 0.3s ease; is being ignored due to another transition attribute above it.

I was playing around and found that one way to fix this would be to force the precedence with transition: background 0.3s ease !important;. This is a pretty hacky solution though so feel free to try other cleaner approaches

the tiny arrow on the dropdown is also impossible to see in dark mode. would look better in white but it's an ant design component so this might not be possible to fix

StormFireFox1

This looks great! Thanks for all the hard work, and also thanks to everyone who reviewed this PR!

I'm glad this has set the ball for our CSS refactor as well, so props to everyone's inputs!

StormFireFox1 · 2022-03-01T03:11:21Z

@trevorkw7 just a heads up; @paulpan05 decided to work on the last review comments, so no need to worry about that.

StormFireFox1 · 2022-03-31T05:50:16Z

@trevorkw7, thank you so much for your hard work!

We've been doing a lot of work for Portal to revitalize it for some upcoming changes, and we figured we could continue on your work and get this feature pushed in a little bit faster. I'll close this PR, but your changes got merged in with #555.

We'll be announcing Dark Mode in the main ACM Discord soon, we'll make sure to credit you as well!

trevorkw7 and others added 12 commits January 26, 2022 00:43

Added Theme Variables and Refactored Less Styling

44401af

Revert Changes Done For Testing

1c3e808

Applied Dark Mode Theme To Most Elements

e8e63aa

Dark Mode For Main Pages

c709c2f

Dashboard, Profile, About, Admin, Discord Pages Have Dark Mode Ready

Small Dark Mode Tweaks

9012ae2

Added Switch

d855e09

Merge branch 'acmucsd:master' into colors-and-dark-mode

a15baab

Refactored Dark/Light Less Vars to CSS Vars, Switch Works

ce73840

Merge branch 'colors-and-dark-mode' of https://github.com/trevorkw7/m…

f1b3235

…embership-portal-ui into colors-and-dark-mode

Made Theme Persistent On Refresh + Cleaned Color Vars

a91c560

Made Light/Dark animation Smoother

77f0992

Small Style Changes in Store

79b6255

steets250 added the PR: Needs Review This PR needs review label Feb 1, 2022

steets250 self-requested a review February 1, 2022 02:40

steets250 suggested changes Feb 1, 2022

View reviewed changes

steets250 added PR: Reviewed w/ Comments This PR has been reviewed and removed PR: Needs Review This PR needs review labels Feb 1, 2022

duanwilliam reviewed Feb 1, 2022

View reviewed changes

trevorkw7 added 11 commits February 14, 2022 13:40

Remove unneccesary id for index div

e730d39

Remove extraneous g tags from sun and moon.svg

8f0c03b

Remove package-lock and update yarn.lock

aabb170

Remove custom.less

0e2e92a

Remove console.log debug statements

6765e21

Reformated hover css to match convention

3390d95

Updated Facebook Icon for Dark Mode

b6a15d7

Fixed Icons on Register and Login Page

e49f22f

Persistent Theme Uses Blocking Script Instead of useEffect

cb35bba

Removed Uneccessary CSS

9d5effe

Fixed email icon on reset password page.

ef89286

trevorkw7 and others added 2 commits February 22, 2022 01:55

Small Styling Fixes

a349b25

Merge branch 'master' into colors-and-dark-mode

985bcbd

trevorkw7 requested a review from steets250 February 22, 2022 10:03

trevorkw7 changed the title ~~Added a Togglable Dark Mode and Refactored CSS Color Variables~~ Added a Togglable Dark Mode and Refactored Existing CSS Color Variables Feb 24, 2022

farisashai reviewed Feb 27, 2022

View reviewed changes

StormFireFox1 approved these changes Mar 1, 2022

View reviewed changes

StormFireFox1 closed this Mar 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a Togglable Dark Mode and Refactored Existing CSS Color Variables #535

Added a Togglable Dark Mode and Refactored Existing CSS Color Variables #535

trevorkw7 commented Feb 1, 2022

steets250 left a comment

duanwilliam left a comment

duanwilliam Feb 1, 2022

steets250 Feb 15, 2022

duanwilliam Feb 1, 2022

trevorkw7 commented Feb 1, 2022

farisashai Feb 27, 2022

farisashai Feb 27, 2022

StormFireFox1 left a comment

StormFireFox1 commented Mar 1, 2022

StormFireFox1 commented Mar 31, 2022

Added a Togglable Dark Mode and Refactored Existing CSS Color Variables #535

Added a Togglable Dark Mode and Refactored Existing CSS Color Variables #535

Conversation

trevorkw7 commented Feb 1, 2022

Feature description

Screenshots

Behavior Changes

Feedback

steets250 left a comment

Choose a reason for hiding this comment

duanwilliam left a comment

Choose a reason for hiding this comment

duanwilliam Feb 1, 2022

Choose a reason for hiding this comment

steets250 Feb 15, 2022

Choose a reason for hiding this comment

duanwilliam Feb 1, 2022

Choose a reason for hiding this comment

trevorkw7 commented Feb 1, 2022

farisashai Feb 27, 2022

Choose a reason for hiding this comment

farisashai Feb 27, 2022

Choose a reason for hiding this comment

StormFireFox1 left a comment

Choose a reason for hiding this comment

StormFireFox1 commented Mar 1, 2022

StormFireFox1 commented Mar 31, 2022