Feature: Generalised Dark Mode into General Theme #4348

arjunjayan999 · 2025-02-05T07:53:29Z

This PR address issue #4340 and focuses on refactoring the theme management system, replacing the old dark mode implementation with a more flexible theme selection mechanism. The changes introduce a new ThemeBox class to handle theme preferences and update various files to support this new approach.

Key changes include:

Theme Management Refactor:

Removed old dark mode styles from css/darkmode.css and css/activities.css. [1] [2]
Added new theme styles for light and dark modes to css/themes.css.
Updated index.html to reference css/themes.css instead of css/darkmode.css and added a theme selection dropdown. [1] [2] [3]
Updated README.md in js/utils to explain thoroughly how to customise theme, or add one.
Added automatic reload after choosing theme.

JavaScript Updates:

Introduced ThemeBox class in js/themebox.js to manage theme preferences.
Updated js/activity.js to integrate ThemeBox and remove old dark mode toggle logic. [1] [2] [3] [4] [5] [6] [7] [8]
Modified js/toolbar.js to render the new theme selection icon and handle theme changes. [1] [2] [3] [4] [5] [6] [7]

…heme selection, Updated README.md for theme customisation

arjunjayan999 · 2025-02-05T07:54:34Z

@walterbender here is the new PR without linting about General Theme Selection

walterbender · 2025-02-05T21:45:39Z

css/themes.css

+  }
+
+
+/* Your Custom Theme can go down if you dont want to modify the existing dark mode */


maybe ^can go down^can go here^ ?

walterbender · 2025-02-05T21:47:00Z

js/themebox.js

@@ -0,0 +1,75 @@
+// Copyright (c) 2018-21 Walter Bender


Should be your name here and 2025

Yeah, I saw that. I actually used the existing languagebox.js to make the new themebox.js to maintain consistency, so I didn't understand how copyright works in open source projects. But I will make the changes asked.

walterbender · 2025-02-05T21:47:40Z

js/themebox.js

+    //  * @returns {void}
+    //  */
+    // custom_onclick() {
+    //     this._theme = "pastel";


maybe replace pastel with "custom" or "your custom theme"?

Hmm, my bad, missed it here and in other places, will make the changes

walterbender · 2025-02-05T21:49:00Z

js/toolbar.js

+                ["ur", _("اردو"), "innerHTML"],
+                ["light", _("Light Mode"), "innerHTML"],
+                ["dark", _("Dark Mode"), "innerHTML"]
+                // ["custom", _("Theme Name"), "innerHTML"],


We should be consistent with the string (even if it is commented out). How about "Custom Theme" everywhere?

walterbender · 2025-02-05T21:49:58Z

js/utils/README.md


-1. **Refresh Required for Theme Application**
+1. When the page loads for the first time, the default theme of light mode is used. (because no themePreference has been set yet)


maybe drop "(because... yet)"

walterbender · 2025-02-05T21:50:18Z

js/utils/README.md


-   The site must be refreshed to apply a new theme in Music Blocks.
+2. When you choose a theme, an Object stores inside the localStorage called themePreference.


an object is stored...

walterbender · 2025-02-05T21:51:20Z

js/utils/README.md


-2. **Canvas and Palette Button Styles**
+3. As of now, all the styling of Music Blocks happens at the time of loading. So we will simply load the page.


All styling... time of loading. When we change themes, we reload the page.

walterbender · 2025-02-05T21:51:51Z

js/utils/README.md


-   Styles for the canvas and palette buttons are initialized at the time of loading MB.
-   - These cannot be directly modified using CSS.
+4. The stylings on the main page happen due to two things:


Most of the Music Blocks theme styles are defined in two places:

walterbender · 2025-02-05T21:53:05Z

js/utils/README.md

-   MB uses a combination of internal and external CSS:
-   - **Internal CSS**: Block colors, pie menu, background, and other essential styles are defined in `js/utils/platformstyle.js`.
-   - **External CSS**: Elements like the navbar and dropdown menu are styled using external CSS files.
+5. the styling on the planet page happens due to one file


The Plant theme styles are defined in:

walterbender · 2025-02-05T21:55:07Z

js/utils/README.md

+            <a id="dark"></a>
+        </li>
+        <li>
+            <a id="pastel"></a>


this is no longer there.

walterbender · 2025-02-05T21:55:27Z

js/utils/README.md

+    string = [[...],
+        ["light", _("Light Mode"), "innerHTML"],
+        ["dark", _("Dark Mode"), "innerHTML"],
+        ["pastel", _("Pastel Theme"), "innerHTML"],


walterbender · 2025-02-05T21:55:34Z

js/utils/README.md

+    string = [_(...),
+    _("Light Mode"),
+    _("Dark Mode"),
+    _("Pastel Theme"),


walterbender · 2025-02-05T21:56:09Z

js/utils/README.md

+    class Activity {
+        constructor(
+            ...
+            this.themes = ["light", "pastel", "dark", "your_theme"];


walterbender · 2025-02-05T21:58:17Z

js/utils/README.md


-5. **Applying the Theme After Reload**
+    If no theme is selected, i.e., there is no themePreference in localStorage. The styles will default to light mode, as it is the base styling without a class in themes.css. But when there is a themePreference in localStorage, the class respective to the chosen theme will be added to the elements which will apply because of higher specificity.


If no theme has been selected (i.e., themePreference has not been defined in localStorage), the styles will default to "light". Otherwise, the theme defined in themePreference will be applied.

walterbender · 2025-02-05T21:59:10Z

js/utils/README.md

+    });
+    ```
+
+    Add your theme here as well. This for loop above checks (after the DOM is loaded) for all themes in themes array to add the class in planetThemes.css (which you will add in later steps) which match themePreference in localStorage to the elements on planet/index.html. That is why the changes happen after the reload.


The "for" loop shown above...

walterbender · 2025-02-05T21:59:35Z

js/utils/README.md

+    ```
+
+    Add your theme here as well. This for loop above checks (after the DOM is loaded) for all themes in themes array to add the class in planetThemes.css (which you will add in later steps) which match themePreference in localStorage to the elements on planet/index.html. That is why the changes happen after the reload.
+    If no theme is selected (i.e., there is no themePreference in localStorage), the styles will default to light mode because it is the base styling with no class in planetThemes.css . But when there is a themePreference in localStorage, the class respective to the chosen theme will be added to the elements which will apply because of higher specificity.


use the same text as in the comment above.

walterbender · 2025-02-05T22:00:08Z

js/utils/README.md

+     * @returns {void}
+     */
+    pastel_onclick() {
+        this._theme = "pastel";


arjunjayan999 · 2025-02-06T04:56:23Z

@walterbender Made the revisions, please review it.

omsuneri · 2025-02-06T18:16:07Z

@arjunjayan999 i dont think we need to have a dropdown menu of theme list as we currently only have 2 modes and switching means chnaging to another theme
as by default you are in light mode so the option for light theme while we are light mode is not seems suitable to me needs to be recreated what we have earlier
@walterbender what do you think over this

omsuneri · 2025-02-06T18:19:48Z

@walterbender Also i m confused what sort of changes do this creates as we can can simply use css variable for the selection of different themes in my mind this needs more dicussion i guess

arjunjayan999 · 2025-02-07T03:15:04Z

A drop-down simplifies the approach in which a user can customise an existing theme, or maybe they want some (or many) changes but don't want to loose their previous themes.
It also is a plus in UI since users can immediately see the two options, making it clear they are choosing between light and dark themes and offers clarity, especially if there are other theme-related options or settings in the future.
Graying out or disabling a current theme is a good option like if you are in light mode, then disable the light mode option.
And if you are confused about the changes, please read the README.md in the PR, I have explained where the stylings are set aside from CSS. If you are thinking of using CSS variables, then you need to transcribe the platformColor object to those variables, which can turn out to be a good thing since keeping styling in css has always been good separation of concerns(but a lengthy thing to do)

omsuneri · 2025-02-07T04:41:41Z

@arjunjayan999 but I ll feel the drop-down option weird must get review from Walterv

Refactored the dark mode into general theme, Automatic reload after t…

49c6456

…heme selection, Updated README.md for theme customisation

walterbender reviewed Feb 5, 2025

View reviewed changes

js/utils/README.md Outdated

<a id="dark"></a>

</li>

<li>

<a id="pastel"></a>

Copy link

Member

walterbender Feb 5, 2025

Choose a reason for hiding this comment

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

this is no longer there.

walterbender reviewed Feb 5, 2025

View reviewed changes

js/utils/README.md Outdated

string = [_(...),

_("Light Mode"),

_("Dark Mode"),

_("Pastel Theme"),

Copy link

Member

walterbender Feb 5, 2025

Choose a reason for hiding this comment

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

same

walterbender reviewed Feb 5, 2025

View reviewed changes

js/utils/README.md Outdated

* @returns {void}

*/

pastel_onclick() {

this._theme = "pastel";

Copy link

Member

walterbender Feb 5, 2025

Choose a reason for hiding this comment

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

again...

Grammatical Revisions

b3ae5e0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Generalised Dark Mode into General Theme #4348

Feature: Generalised Dark Mode into General Theme #4348

arjunjayan999 commented Feb 5, 2025

arjunjayan999 commented Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

arjunjayan999 Feb 6, 2025

walterbender Feb 5, 2025

arjunjayan999 Feb 6, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025 •

edited

Loading

walterbender Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

walterbender Feb 5, 2025

arjunjayan999 commented Feb 6, 2025

omsuneri commented Feb 6, 2025 •

edited

Loading

omsuneri commented Feb 6, 2025

arjunjayan999 commented Feb 7, 2025 •

edited

Loading

omsuneri commented Feb 7, 2025

		}


		/* Your Custom Theme can go down if you dont want to modify the existing dark mode */


		1. Refresh Required for Theme Application
		1. When the page loads for the first time, the default theme of light mode is used. (because no themePreference has been set yet)


		The site must be refreshed to apply a new theme in Music Blocks.
		2. When you choose a theme, an Object stores inside the localStorage called themePreference.


		2. Canvas and Palette Button Styles
		3. As of now, all the styling of Music Blocks happens at the time of loading. So we will simply load the page.


		5. Applying the Theme After Reload
		If no theme is selected, i.e., there is no themePreference in localStorage. The styles will default to light mode, as it is the base styling without a class in themes.css. But when there is a themePreference in localStorage, the class respective to the chosen theme will be added to the elements which will apply because of higher specificity.

Feature: Generalised Dark Mode into General Theme #4348

Are you sure you want to change the base?

Feature: Generalised Dark Mode into General Theme #4348

Conversation

arjunjayan999 commented Feb 5, 2025

Theme Management Refactor:

JavaScript Updates:

arjunjayan999 commented Feb 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterbender Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arjunjayan999 commented Feb 6, 2025

omsuneri commented Feb 6, 2025 • edited Loading

omsuneri commented Feb 6, 2025

arjunjayan999 commented Feb 7, 2025 • edited Loading

omsuneri commented Feb 7, 2025

walterbender Feb 5, 2025 •

edited

Loading

omsuneri commented Feb 6, 2025 •

edited

Loading

arjunjayan999 commented Feb 7, 2025 •

edited

Loading