Add dark mode feature #26

sumanbiswas7 · 2023-02-18T10:48:22Z

Added a button to toggle between light and dark modes.
Added toggle between dark mode and light mode feature
Feature working on pages /, /news, news/[post], /donate
Made feature persistent using localStorage
Added moon and sun icon for toggle theme button
Requested Changes

Fixes #1

netlify · 2023-02-18T10:48:26Z

✅ Deploy Preview for makedeb ready!

Name	Link
🔨 Latest commit	`a1697c1`
🔍 Latest deploy log	https://app.netlify.com/sites/makedeb/deploys/6479a2cc1ea4e6000810d2b1
😎 Deploy Preview	https://deploy-preview-26--makedeb.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Heus-Sueh · 2023-02-18T14:28:49Z

would it be possible for you to change the button ''light mode'' to an icon similar to these:

Heus-Sueh · 2023-02-18T14:35:28Z

I think it would be good to also change the gray color of the command to install makedeb

something similar to this would be fine in my opinion

hwittenborn · 2023-02-18T15:30:08Z

Thanks a ton for this! I've been wanting this done for a while, I just never got around to doing it.

I'm gonna be busy for a bit today, but I'll get this checked out by the end of the day with some review comments. There's a few things I'd like changed, I agree with the comments @Heus-Sueh made, but I can give some when I get a chance to look at everything more later today.

sumanbiswas7 · 2023-02-18T16:02:36Z

@Heus-Sueh I agree with you. I'll be working on the button and the background of the code 🤓

Heus-Sueh · 2023-02-18T16:24:22Z

I think I'm asking a lot but can you make the change that this issue proposes?
makedeb/mprweb#112
since you have knowledge in html and css

sumanbiswas7 · 2023-02-18T17:40:40Z

@Heus-Sueh Yeah, I'll look into it.

hwittenborn

Most of my comments are visual, though one placement thing is under a separate comment below;

Is there any CDN to get the two icons you used from? Currently Jam icons is used for most things, but the icons you got look quite good with the text next to it in the header bar (the lines being close to the same width as the text next to it makes it work really good imo). Just because the Jam icons are loaded directly from the website though, I'd also like to load these two icons in the same manner.
The clipboard icon on the homepage (visible if you hover over the codeblock under Install makedeb) isn't changing colors with the rest of the codeblock.
The border color under the post boxes on the News page looks a bit too light under the dark mode. #464646 looked pretty good in my local testing.
With the border color change above, I also think the date text on post boxes could be a bit lighter on dark mode, I think #bfbfbf works pretty good.
The <hr> line elements on individual news post pages looks a bit too bright for me, #6a6a6a was a nice color when I tried changing it.
I think the theme changer icon could have no border on it, and then just have a background that blends in with the rest of the header. The line color for it could just change like the links in the header bar too, thus making it all look unified.

And then I also had some other issues, but I'm not quite sure how they should be solved yet:

The icon doesn't look quite in place on the mobile view - it feels like it should be separated from the links, but putting it on it's own line feels a bit off too.
The Authelia and Prism Launcher logos under Ready for Production don't stand out enough on the dark theme. I'll probably have to reach out to the two teams to see if I could get some dark-theme friendly icons.

I'll probably have to think about at least that first one, if you got any thoughts feel free to throw them out too.

With all of that though, thanks again for getting support for this going! It's definitely long overdue, and I'm really looking forward to getting it merged in :)

hwittenborn · 2023-02-18T21:16:43Z

themes/makedeb/layouts/partials/header.html

+
+<script>
+	initialThemeLoad()
+
+	const toggleThemeBtn = document.getElementById("toggle-theme-button")
+	toggleThemeBtn?.addEventListener("click",function(){
+		const body = document.body
+		if (body.className=="dark")
+		{
+			setBodyClass("")
+			toggleThemeBtn?.classList.remove("active")
+			changeBtnIcon("moon")
+			localStorage.removeItem("theme")
+		}
+		else {
+			toggleThemeBtn?.classList.add("active")
+			changeBtnIcon("sun")
+			localStorage.setItem("theme", "dark")
+			setBodyClass("dark")
+		}
+	})
+
+	/**
+	 * Makes nessesary changes for initial theme load
+	*/
+	function initialThemeLoad(){
+	const toggleThemeBtn = document.getElementById("toggle-theme-button")
+	const theme = localStorage.getItem("theme")
+		if (theme == "dark") {
+			setBodyClass("dark")
+			changeBtnIcon("sun")
+			toggleThemeBtn?.classList.add("active")
+		} else changeBtnIcon("moon")
+	}
+
+	/**
+	 * Changes the body class
+	 * @param {string} className - class name to be set
+	*/
+	function setBodyClass(className){
+		const body = document.body
+		body.className = className
+	}
+
+	/**
+	 * Changes theme toggle button's icon
+	 * @param {"sun" | "moon"} iconType - icon type to be displayed
+	*/
+	function changeBtnIcon(iconType){
+		const sunIcon = document.getElementById("sun-icon")
+		const moonIcon = document.getElementById("moon-icon")
+		if (iconType == "sun"){
+			moonIcon?.classList.remove("active-icon")
+			sunIcon?.classList.add("active-icon")
+		}
+		if (iconType == "moon"){
+			sunIcon?.classList.remove("active-icon")
+			moonIcon?.classList.add("active-icon")
+		}
+	}
+</script>


Can we put this in a separate file just so the code doesn't get too messy? Just somewhere like static/js/color-theme.js would work.

hwittenborn · 2023-02-18T21:36:53Z

Pinging @hiddeninthesand too since you were the one originally wanting it. Do you any comments about how you'd like all this implemented?

hwittenborn · 2023-02-18T21:38:46Z

I think I'm asking a lot but can you make the change that this issue proposes?
https://github.com/makedeb/makedeb/issues/254

@Heus-Sueh: That probably needs to be done as a separate PR under the mprweb repository, right?

Heus-Sueh · 2023-02-18T21:46:20Z

I think I'm asking a lot but can you make the change that this issue proposes?
https://github.com/makedeb/makedeb/issues/254

@Heus-Sueh: That probably needs to be done as a separate PR under the mprweb repository, right?

yeah... because maybe that requires more drastic changes in the structure 👍

hwittenborn · 2023-02-18T21:50:35Z

Yeah it'll probably take more work on the MPR side just because there's more pages to set up, nothing much should have to change for this PR though - every project has it's own code for the CSS, I just kind of copy-paste through the projects as I've needed

hwittenborn

Hey, I just got one tiny change. I didn't quite like how the JavaScript file was added (there was some indentation stuff that looked a bit foreign to me), so I went ahead an added Prettier for code formatting as well so everything's looking clean.

hwittenborn · 2023-02-20T05:03:12Z

themes/makedeb/static/js/theme.js

+function setBodyClass(className) {
+  const body = document.body;
+  body.className = className;
+}


Can't this just be written in one line as document.body.className = blahBlah and then put directly in the toggleThemeBtn?.addEventListener callback above? Since that's how things like toggleThemeBtn?.classList.remove are being used I think that'd make the code more consistent.

hwittenborn · 2023-02-20T05:15:01Z

Some other things too:

Could the cards at /news have a slightly darker background color? #262626 looked good to me.
Could the header background color on the dark theme be changed so the <hr> right below it stands out more? #1e1e1e looked good on my end.
Could we try putting the theme icon in the footer? When using the desktop layout, I'm thinking we could put it in the middle:

And then on the mobile view right above the copyright text on its own line:

hwittenborn

Alright, this is looking really good! I just need to get the two logos from Authelia/Prism Launcher, get one last issue fixed, and this should be good to merge :)

hwittenborn · 2023-02-20T14:59:01Z

themes/makedeb/assets/scss/variables.scss

+.dark .themeHeaderColor {
+  background-color: #1e1e1e;
+}
+.dark .themeTextColor {
+  color: #fff;
+}
+.dark .themeHeaderHrColor {
+  color: #111111;
+  background-color: #111111;
+}
+.dark .themeBackgroundColor {
+  color: #fff;
+  background-color: #2d2d2d;
+  border-color: #464646;
+}
+.dark .themeFactsColor {
+  color: #fff;
+  background-color: #272727;
+}
+.dark .themeCodeColor {
+  color: #fff;
+  background-color: #202020;
+}
+.dark .themeClipboardCheckedColor {
+  color: #40a85c;
+  background-color: #202020;
+}
+.dark .themePostContentDateColor {
+    color: #bfbfbf;
+}
+.dark .themePostHrColor {
+    background-color: #6a6a6a;
+}
+.dark .themeCardsColor {
+    background-color: #262626;
+    color: #fff;
+}


I just realized this, but could .light .theme* variants also be created? The places where these are used already have a background-color defined manually (such as in scss/post.scss:5), and if they were defined in a .light .theme* block in here (and the the background-color/color/etc. calls removed in the other places) it would make the code more consistent.

hwittenborn · 2023-02-20T15:08:24Z

themes/makedeb/static/js/theme.js

+initialThemeLoad();
+
+const toggleThemeBtn = document.getElementById("toggle-theme-button");
+toggleThemeBtn?.addEventListener("click", function () {


I actually didn't know about this ? stuff until now, apparently it's called optional chaining? If so, we should probably remove it so that errors are raised when elements don't exist (since they always should), right?

hiddeninthesand · 2023-02-24T11:28:51Z

Pinging @hiddeninthesand too since you were the one originally wanting it. Do you any comments about how you'd like all this implemented?

It all looks good to me! I can't see anything questionable.

sumanbiswas7 · 2023-02-26T15:56:54Z

@hwittenborn Hey! I just wanted to inquire about the status of my pull request. Can you please let me know if any further changes or updates are needed before it can be merged? I am looking forward to seeing it incorporated into the project.

hwittenborn · 2023-02-26T16:02:09Z

Sorry about that @sumanbiswas7, I got a bit sidetracked with some stuff and forget to get this looked at.

Once I'm home (in about 4 hours) I'll get this reviewed. Everything was pretty much fine the last time I checked though, I'm pretty sure I just need to get a dark theme friendly icon from Authelia and we're set.

hwittenborn · 2023-02-26T20:41:57Z

themes/makedeb/assets/scss/post.scss

+  hr {
+    @extend .themePostHrColor;
+  }
+  hr {
+    @extend .themePostHrColor;
+  }


This duplication isn't needed, right? Also I think we could just remove it entirely and apply this styling to all <hr> in main.scss, as places like /donate have the <hr> element a bit too bright on the dark theme.

hwittenborn · 2023-02-26T20:45:02Z

themes/makedeb/assets/scss/content-section.scss

+    border-color: darken($silver, 10%);
+    border-width: 0.15em;
+    @extend .themeCardsColor;


The border-color here is a bit too light on the dark theme (see the cards on /news) - could this be one of the theme-toggleable colors, with the dark theme having it set to something like #4d4d4d?

hwittenborn · 2023-02-26T20:46:11Z

Those should be the last two things, other than that it should be ready to go once I get the Authelia icon.

sumanbiswas7 · 2023-02-27T09:28:06Z

@hwittenborn The Authelia website link which is attached to homepage is using a different logo. Seems like they have updated their logo. If that's the case can we use the below logos that I extracted?

hwittenborn · 2023-02-27T13:04:04Z

The Authelia website link which is attached to homepage is using a different logo.

I'm pretty sure that's just the font style from the documentation theme they use :p.

hwittenborn · 2023-02-27T14:24:56Z

Just need to get those two icons figured out and this'll be good to merge :)

hwittenborn · 2023-04-12T14:41:17Z

Hey, just sending a message so you know what's going on @sumanbiswas7:

I'm messaging the Authelia people in that issue above, and everything's in the process for getting a dark logo going. Once that's done I'll get the two logos added, get the needed JS code going, and then I'll just have you look over the changes to make sure you're fine with them. After that everything should be good to go, and I'll get this merged in :).

sumanbiswas7 · 2023-05-02T19:40:11Z

@hwittenborn Hello 👋.
Thanks for the update. Everything sounds good!

hwittenborn · 2023-06-02T08:08:03Z

Hey @sumanbiswas7, I just added in the new dark-friendly logos, and I made a few other changes while I was at it. Let me know if there's anything I did that you'd like changed, and then this can get merged right in!

hwittenborn · 2023-06-02T08:42:09Z

I'm terribly sorry on the wait too, it's definitely not the way I wanted everything to go. I've been juggling about 5 projects right now, and some end up falling behind like this when I'm zoning in on a specific one. I'm still trying to figure out how to balance everything in a good way with all my daily activities, but I just wanted to send a message about all that's been going on.

sumanbiswas7 · 2023-06-03T12:02:27Z

@hwittenborn no worries. hoping to see this getting merged soo. :)

hwittenborn · 2023-06-03T17:00:25Z

Alright, sounds great! Thanks a ton for all the work you put in for thing, I'm really happy to finally see some stuff for a dark mode! I'll go ahead and get this merged in :)

Add dark mode feature

01ff422

sumanbiswas7 added 2 commits February 18, 2023 22:17

Change code background color

6b477e2

ToggleTheme button update

746164d

hwittenborn requested changes Feb 18, 2023

View reviewed changes

sumanbiswas7 added 6 commits February 19, 2023 09:16

Theme script seperated

0c46ab3

Clipboard icon and border color changes

c401f8c

Post card date color changes

60432da

Post hr color changes

407f769

Remove border of toggle thme button

2d96ad6

Toggle theme button icon, color changes

3021367

sumanbiswas7 requested a review from hwittenborn February 19, 2023 04:41

Format JS/SCSS with Prettier

8ea226f

hwittenborn requested changes Feb 20, 2023

View reviewed changes

Fix GitHub CI config

203a57d

This was referenced Feb 20, 2023

Provide dark-theme friendly logos PrismLauncher/PrismLauncher#878

Closed

Provide dark-theme friendly logos authelia/authelia#4968

Closed

sumanbiswas7 added 2 commits February 20, 2023 11:34

Refactor theme.js

54fa82b

News/cards background color changes

96b7aac

Fix Merge Conflict

b51b9cb

sumanbiswas7 requested a review from hwittenborn February 20, 2023 07:28

hwittenborn requested changes Feb 20, 2023

View reviewed changes

sumanbiswas7 added 2 commits February 21, 2023 00:01

Remove optional chains

ffaadbf

Light color palette added

f98b30d

sumanbiswas7 requested a review from hwittenborn February 21, 2023 05:59

Run code through prettier

792e29b

hwittenborn requested changes Feb 26, 2023

View reviewed changes

sumanbiswas7 added 3 commits February 27, 2023 14:20

Merge branch 'makedeb:main' into sumanbiswas7/add-darkmode

16df8b0

Duplace .themePostHrColor delete

a9f7d45

Add News Card dark mode border color

5ff1e3b

sumanbiswas7 requested a review from hwittenborn February 27, 2023 09:16

hwittenborn approved these changes Feb 27, 2023

View reviewed changes

Let's get this thing merged!

a1697c1

hwittenborn merged commit cbe3e2a into makedeb:main Jun 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dark mode feature #26

Add dark mode feature #26

sumanbiswas7 commented Feb 18, 2023 •

edited

Loading

netlify bot commented Feb 18, 2023 •

edited

Loading

Heus-Sueh commented Feb 18, 2023 •

edited

Loading

Heus-Sueh commented Feb 18, 2023

hwittenborn commented Feb 18, 2023

sumanbiswas7 commented Feb 18, 2023

Heus-Sueh commented Feb 18, 2023

sumanbiswas7 commented Feb 18, 2023

hwittenborn left a comment

hwittenborn Feb 18, 2023

hwittenborn commented Feb 18, 2023

hwittenborn commented Feb 18, 2023

Heus-Sueh commented Feb 18, 2023

hwittenborn commented Feb 18, 2023

hwittenborn left a comment •

edited

Loading

hwittenborn Feb 20, 2023

hwittenborn commented Feb 20, 2023

hwittenborn left a comment

hwittenborn Feb 20, 2023

hwittenborn Feb 20, 2023

hiddeninthesand commented Feb 24, 2023

sumanbiswas7 commented Feb 26, 2023

hwittenborn commented Feb 26, 2023

hwittenborn Feb 26, 2023

hwittenborn Feb 26, 2023

hwittenborn commented Feb 26, 2023

sumanbiswas7 commented Feb 27, 2023

hwittenborn commented Feb 27, 2023

hwittenborn commented Feb 27, 2023

hwittenborn commented Apr 12, 2023

sumanbiswas7 commented May 2, 2023

hwittenborn commented Jun 2, 2023

hwittenborn commented Jun 2, 2023

sumanbiswas7 commented Jun 3, 2023

hwittenborn commented Jun 3, 2023

Add dark mode feature #26

Add dark mode feature #26

Conversation

sumanbiswas7 commented Feb 18, 2023 • edited Loading

netlify bot commented Feb 18, 2023 • edited Loading

✅ Deploy Preview for makedeb ready!

Heus-Sueh commented Feb 18, 2023 • edited Loading

Heus-Sueh commented Feb 18, 2023

hwittenborn commented Feb 18, 2023

sumanbiswas7 commented Feb 18, 2023

Heus-Sueh commented Feb 18, 2023

sumanbiswas7 commented Feb 18, 2023

hwittenborn left a comment

Choose a reason for hiding this comment

hwittenborn Feb 18, 2023

Choose a reason for hiding this comment

hwittenborn commented Feb 18, 2023

hwittenborn commented Feb 18, 2023

Heus-Sueh commented Feb 18, 2023

hwittenborn commented Feb 18, 2023

hwittenborn left a comment • edited Loading

Choose a reason for hiding this comment

hwittenborn Feb 20, 2023

Choose a reason for hiding this comment

hwittenborn commented Feb 20, 2023

hwittenborn left a comment

Choose a reason for hiding this comment

hwittenborn Feb 20, 2023

Choose a reason for hiding this comment

hwittenborn Feb 20, 2023

Choose a reason for hiding this comment

hiddeninthesand commented Feb 24, 2023

sumanbiswas7 commented Feb 26, 2023

hwittenborn commented Feb 26, 2023

hwittenborn Feb 26, 2023

Choose a reason for hiding this comment

hwittenborn Feb 26, 2023

Choose a reason for hiding this comment

hwittenborn commented Feb 26, 2023

sumanbiswas7 commented Feb 27, 2023

hwittenborn commented Feb 27, 2023

hwittenborn commented Feb 27, 2023

hwittenborn commented Apr 12, 2023

sumanbiswas7 commented May 2, 2023

hwittenborn commented Jun 2, 2023

hwittenborn commented Jun 2, 2023

sumanbiswas7 commented Jun 3, 2023

hwittenborn commented Jun 3, 2023

sumanbiswas7 commented Feb 18, 2023 •

edited

Loading

netlify bot commented Feb 18, 2023 •

edited

Loading

Heus-Sueh commented Feb 18, 2023 •

edited

Loading

hwittenborn left a comment •

edited

Loading