add settings support to dark theme #129

yoper12 · 2025-06-16T10:06:29Z

No description provided.

Co-authored-by: BanOcean <47253870+banocean@users.noreply.github.com>

wombatus2137 · 2025-06-28T19:24:29Z

Ocean ma rację nie rób tego co zrobiłeś w 50051ed.

banocean

Branch chyba wymaga updata

yoper12 · 2025-09-07T16:49:25Z

@banocean

Branch chyba wymaga updata

nie wymaga

Copilot

Pull request overview

This PR adds comprehensive settings support to the browser extension, enabling users to configure patch behavior through a new settings interface. The most significant change is making the dark theme configurable with auto/enabled/disabled options, replacing the previous media query-only approach.

Key changes:

Introduces a complete settings infrastructure with storage synchronization, UI components for desktop and mobile, and an API for getting/saving settings
Converts the dark theme from media query-based to a configurable setting with three modes (auto, enabled, disabled)
Adds configurable plus/minus grade values to the "Count averages" patch

Reviewed changes

Copilot reviewed 14 out of 18 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
`settingsSaver.js`	Synchronizes settings between Chrome storage and sessionStorage, handles patches.json loading
`patches/apis/settings.js`	Core API for getting and saving patch settings with type conversion and validation
`patches/patchesSettings/settingRenderers.js`	HTML template generators for different setting input types (text, boolean, select, etc.)
`patches/patchesSettings/generateSettingsList.js`	Generates the settings UI with search functionality and event handlers
`patches/patchesSettings/markers.js`	Utility functions for highlighting search terms in the settings UI
`patches/patchesSettings/desktop.js`	Desktop modal implementation for settings interface
`patches/patchesSettings/mobile.js`	Mobile-specific settings interface implementation
`patches/patchesSettings/style.css`	Complete styling for the settings UI components
`patches/darkTheme/index.js`	New JavaScript implementation to apply dark theme based on settings
`patches/darkTheme/main.css`	Changes selector from media query to class-based and adds dark theme styles for settings UI
`patches/darkTheme/evHome.css`	Updates selector from media query to class-based for eduVulcan homepage
`patches/countAverage.js`	Integrates settings API to use configurable plus/minus values
`patches.json`	Adds settings schema definitions for dark theme and count averages patches
`manifest.json`	Includes settingsSaver.js in content scripts
`assets/icons/*.svg`	Adds UI icons for settings, search, close, and clear buttons

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-07T15:56:59Z

patches/patchesSettings/markers.js

+ * @param {Node} element element, z którego usuwamy znaczniki <mark>
+ * @returns {void}
+ */
+export async function removeMarks(element) {


The function is declared as async but doesn't contain any await expressions or return a Promise. Remove the async keyword as it's unnecessary and misleading.

Copilot · 2026-01-07T15:56:59Z

patches/patchesSettings/markers.js

+ * @param {string} textQueryToHighlight string do wyszukania
+ * @returns {void}
+ */
+export async function markTextInElement(element, textQueryToHighlight) {


The function is declared as async but doesn't contain any await expressions or return a Promise. Remove the async keyword as it's unnecessary and misleading.

Copilot · 2026-01-07T15:57:00Z

patches/patchesSettings/markers.js

+        }
+    }
+
+    nodesToModify.forEach(async ({ node, text, query }) => {


The async keyword is used on the forEach callback, but the function doesn't await anything and async forEach doesn't wait for async operations to complete. Remove the async keyword from the callback as it serves no purpose here.

Copilot · 2026-01-07T15:57:00Z

settingsSaver.js

+fetch(chrome.runtime.getURL("patches.json"))
+    .then((response) => response.json())
+    .then((data) =>
+        data.sort((a, b) => a.name.localeCompare(b.name, "pl"))
+    )
+    .then((data) => {
+        sessionStorage.setItem("IFV_PATCHES", JSON.stringify(data));


There's a potential race condition: the patches.json fetch is asynchronous, but getSetting can be called before IFV_PATCHES is stored in sessionStorage (line 39). This could cause getSetting to return an empty array for patches, leading to "Patch not found" errors. Consider ensuring patches are loaded before other code runs, or add appropriate error handling.

Suggested change

fetch(chrome.runtime.getURL("patches.json"))

.then((response) => response.json())

.then((data) =>

data.sort((a, b) => a.name.localeCompare(b.name, "pl"))

)

.then((data) => {

sessionStorage.setItem("IFV_PATCHES", JSON.stringify(data));

// Ensure there is always some value for IFV_PATCHES in sessionStorage

if (!sessionStorage.getItem("IFV_PATCHES")) {

sessionStorage.setItem("IFV_PATCHES", JSON.stringify([]));

}

// Expose a shared promise so other code can wait for patches to be loaded

window.ifvPatchesPromise = fetch(chrome.runtime.getURL("patches.json"))

.then((response) => response.json())

.then((data) =>

data.sort((a, b) => a.name.localeCompare(b.name, "pl"))

)

.then((data) => {

sessionStorage.setItem("IFV_PATCHES", JSON.stringify(data));

return data;

})

.catch((error) => {

console.error("Failed to load patches.json", error);

// Fall back to whatever is already in sessionStorage or an empty array

const existing = sessionStorage.getItem("IFV_PATCHES");

try {

return existing ? JSON.parse(existing) : [];

} catch {

return [];

}

Copilot · 2026-01-07T15:57:00Z

patches/darkTheme/index.js

+function applyDarkTheme() {
+    if (
+        (getSetting("Dark theme", "darkThemeEnabled") === "auto" &&
+            window.matchMedia("(prefers-color-scheme: dark)").matches) ||
+        getSetting("Dark theme", "darkThemeEnabled") === "enabled"
+    ) {
+        if (window.location.hostname === "eduvulcan.pl") {
+            document.documentElement.classList.add("evHome-dark");
+        } else {
+            document.documentElement.classList.add("dark");
+        }
+    }


If getSetting throws an error (e.g., when settings haven't loaded yet or patch isn't found), the entire applyDarkTheme function will fail and no dark theme will be applied. Consider wrapping the getSetting calls in a try-catch block and providing a fallback behavior, such as using the system preference or a default value.

Copilot · 2026-01-07T15:57:02Z

patches/patchesSettings/settingRenderers.js

+    color: (setting, patchName, currentValue) => `
+        <input type="color" class="setting-color" data-patch="${patchName}" data-setting="${setting.id}" value="${currentValue}">


Similar to the text input, currentValue is directly interpolated into the value attribute without escaping. This could break the HTML structure if currentValue contains quotes or other special characters. Consider using proper HTML escaping.

Copilot · 2026-01-07T15:57:03Z

patches/darkTheme/main.css

@@ -1,4 +1,4 @@
-@media (prefers-color-scheme: dark) {
+:root.dark {


The CSS selector changed from @media (prefers-color-scheme: dark) to :root.dark, which means the dark theme will only apply when the .dark class is added to the root element. However, the old media query selector is completely removed, so users who had the automatic dark theme enabled via system preferences will lose that functionality unless they explicitly enable it through the new settings. Consider maintaining backward compatibility or communicating this breaking change clearly.

Copilot · 2026-01-07T15:57:03Z

patches/patchesSettings/generateSettingsList.js

+ * @param {Node} patchesSettingsDiv
+ * @returns {void}
+ */
+function addBttmButtons(patchesSettingsDiv, patches) {


The abbreviation 'Bttm' in the function name is unclear and inconsistent with naming conventions. Consider renaming to addBottomButtons for better clarity and readability.

Copilot · 2026-01-07T15:57:03Z

patches/patchesSettings/style.css

+.setting-boolean-toggle .toggle-input:checked + .toggle-switch::before {
+    transform: translateX(16px);
+}
+


The duplicate rule for .setting-boolean-toggle .toggle-input:checked + .toggle-switch::before (lines 394 and 402) applies the same transform. This is redundant - consider removing one of these duplicate rules.

Suggested change

.setting-boolean-toggle .toggle-input:checked + .toggle-switch::before {

transform: translateX(16px);

}

Copilot · 2026-01-07T15:57:03Z

patches/patchesSettings/settingRenderers.js

+    text: (setting, patchName, currentValue) => `
+        <input type="text" class="setting-text" data-patch="${patchName}" data-setting="${setting.id}" value="${currentValue}" placeholder="${setting.default}">


The currentValue is directly interpolated into HTML attributes without escaping. If currentValue contains special HTML characters (like quotes or angle brackets), it could break the HTML structure or potentially lead to XSS vulnerabilities. Consider using proper HTML escaping or creating elements programmatically with textContent/value properties instead of string interpolation.

yoper12 and others added 5 commits June 11, 2025 13:21

Add settings (banocean#120)

9ffbfba

Co-authored-by: BanOcean <47253870+banocean@users.noreply.github.com>

Merge remote-tracking branch 'upstream/main' into dark-theme

50051ed

add settings support

5a8b454

why was this even here

28ec998

aeb93b2

(fix): settings configuration in patches.json (banocean#130)

5917ed8

banocean requested changes Jul 15, 2025

View reviewed changes

yoper12 requested a review from banocean September 7, 2025 16:50

(fix): settings configuration in patches.json (banocean#130)

654f833

Copilot AI review requested due to automatic review settings January 7, 2026 15:50

Copilot started reviewing on behalf of yoper12 January 7, 2026 15:51 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

wombatus2137 mentioned this pull request Feb 1, 2026

feat(dark theme): add settings support wombatus2137/ifv#2

Merged

yoper12 added 2 commits February 2, 2026 20:03

minor fixes

d44699e

Merge branch 'main' into dark-theme

611ee63

-fetch(chrome.runtime.getURL("patches.json"))
-    .then((response) => response.json())
-    .then((data) =>
-        data.sort((a, b) => a.name.localeCompare(b.name, "pl"))
-    )
-    .then((data) => {
-        sessionStorage.setItem("IFV_PATCHES", JSON.stringify(data));
+// Ensure there is always some value for IFV_PATCHES in sessionStorage
+if (!sessionStorage.getItem("IFV_PATCHES")) {
+    sessionStorage.setItem("IFV_PATCHES", JSON.stringify([]));
+}
+// Expose a shared promise so other code can wait for patches to be loaded
+window.ifvPatchesPromise = fetch(chrome.runtime.getURL("patches.json"))
+    .then((response) => response.json())
+    .then((data) =>
+        data.sort((a, b) => a.name.localeCompare(b.name, "pl"))
+    )
+    .then((data) => {
+        sessionStorage.setItem("IFV_PATCHES", JSON.stringify(data));
+        return data;
+    })
+    .catch((error) => {
+        console.error("Failed to load patches.json", error);
+        // Fall back to whatever is already in sessionStorage or an empty array
+        const existing = sessionStorage.getItem("IFV_PATCHES");
+        try {
+            return existing ? JSON.parse(existing) : [];
+        } catch {
+            return [];
+        }

		color: (setting, patchName, currentValue) => `
		<input type="color" class="setting-color" data-patch="${patchName}" data-setting="${setting.id}" value="${currentValue}">

		@@ -1,4 +1,4 @@
		@media (prefers-color-scheme: dark) {
		:root.dark {

	.setting-boolean-toggle .toggle-input:checked + .toggle-switch::before {
	transform: translateX(16px);
	}

		text: (setting, patchName, currentValue) => `
		<input type="text" class="setting-text" data-patch="${patchName}" data-setting="${setting.id}" value="${currentValue}" placeholder="${setting.default}">

add settings support to dark theme #129

Are you sure you want to change the base?

add settings support to dark theme #129

Uh oh!

Conversation

yoper12 commented Jun 16, 2025

Uh oh!

wombatus2137 commented Jun 28, 2025

Uh oh!

banocean left a comment

Choose a reason for hiding this comment

Uh oh!

yoper12 commented Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yoper12 commented Sep 7, 2025 •

edited

Loading