Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FIX: Refactor & adjustments for better functionality #14

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

FIX: Refactor & adjustments for better functionality #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
09f9cbc
Init fix
jordanvidrine Nov 14, 2022
a09455c
prettier
jordanvidrine Nov 14, 2022
7adde92
more
jordanvidrine Nov 14, 2022
44273d5
more
jordanvidrine Nov 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion javascripts/discourse/initializers/dark-light-toggle-hamburger.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Have you selected two different themes for your dark/light schemes in user prefe
lightTheme.media = "none";

Session.currentProp("defaultColorSchemeIsDark", true);
Session.currentProp("darkModeAvailable", true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's that for? Is it used (read) in this theme component? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I found that if your system is set to dark mode, and you click toggle light color scheme the logo would not change. Adding these Session.currentProp methods fixed this. Here is where it is used in core.

https://github.com/discourse/discourse/blob/38bb00edce0764d3e0c4700115622af446c84db5/app/assets/javascripts/discourse/app/widgets/home-logo.js#L37

The only draw back is now, when changing your system from either dark to light, and this component is set to auto, the page needs a refresh to load the correct logo. I feel the middle ground I've come to is fine for this component.

I also contemplated turning off the auto select.

};

let switchToLight = function () {
Expand All @@ -81,6 +82,7 @@ Have you selected two different themes for your dark/light schemes in user prefe
darkTheme.media = "none";

Session.currentProp("defaultColorSchemeIsDark", false);
Session.currentProp("darkModeAvailable", false);
};

let switchToAuto = function () {
Expand All @@ -93,8 +95,10 @@ Have you selected two different themes for your dark/light schemes in user prefe

if (window?.matchMedia("(prefers-color-scheme: dark)").matches) {
Session.currentProp("defaultColorSchemeIsDark", true);
Session.currentProp("darkModeAvailable", true);
} else {
Session.currentProp("defaultColorSchemeIsDark", false);
Session.currentProp("darkModeAvailable", false);
}
};

Expand Down Expand Up @@ -198,7 +202,9 @@ Have you selected two different themes for your dark/light schemes in user prefe
},
});

if (settings.add_color_scheme_toggle_to_header) {
// with new sidebar rolling out, this will be the main option of showing
// allow those who dont use sidebar to remove from header
if (!settings.remove_color_scheme_toggle_from_header) {
api.addToHeaderIcons("dark-light-toggle");
}

Expand All @@ -207,6 +213,7 @@ Have you selected two different themes for your dark/light schemes in user prefe

click() {
toggleDarkLight();
this.scheduleRerender();
},

html() {
Expand Down Expand Up @@ -245,6 +252,7 @@ Have you selected two different themes for your dark/light schemes in user prefe
} else {
switchToAuto();
}
this.scheduleRerender();
},

html() {
Expand Down
6 changes: 3 additions & 3 deletions settings.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ svg_icons:
default: "moon|sun"
type: "list"
list_type: "compact"
add_color_scheme_toggle_to_header:
default: false
remove_color_scheme_toggle_from_header:
default: true
type: bool
description: "Add color scheme toggle button to site header"
description: "Do not show color scheme toggle button in site header"