Skip to content

fix: funbox deselection not clearing effects without refresh (@prajith5) #7469

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

Open
prajith5 wants to merge 1 commit into
base: master
Choose a base branch
from
+2,895 −4,031
Open

fix: funbox deselection not clearing effects without refresh (@prajith5) #7469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
134 changes: 67 additions & 67 deletions frontend/src/ts/test/funbox/funbox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,14 @@ export function toggleScript(...params: string[]): void {
}

export function setFunbox(funbox: FunboxName[]): boolean {
if (funbox.length === 0) {
for (const fb of getActiveFunboxesWithFunction("clearGlobal")) {
fb.functions.clearGlobal();
}
const previousFunboxes = getActiveFunboxNames();

const removedFunboxes = previousFunboxes.filter((fb) => !funbox.includes(fb));

for (const fb of removedFunboxes) {
get(fb).functions?.clearGlobal?.();
}

FunboxMemory.load();
setConfig("funbox", funbox);
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The setFunbox() function clears removed funboxes but doesn't apply global CSS for newly activated funboxes. When switching from one set of funboxes to another (e.g., from ["nausea"] to ["upside_down"]), the new funbox's applyGlobalCSS() is never called, which could leave it in an incomplete state.

Consider adding logic to detect newly activated funboxes and call their applyGlobalCSS() function, similar to how toggleFunbox() handles this on line 76.

Suggested change
setConfig("funbox", funbox);
setConfig("funbox", funbox);
const currentFunboxes = getActiveFunboxNames();
const addedFunboxes = currentFunboxes.filter(
(fb) => !previousFunboxes.includes(fb),
);
for (const fb of addedFunboxes) {
get(fb).functions?.applyGlobalCSS?.();
}

Copilot uses AI. Check for mistakes.
return true;
Expand All @@ -56,12 +59,20 @@ export function toggleFunbox(funbox: FunboxName): void {
);
return;
}

FunboxMemory.load();

const wasActive = getActiveFunboxNames().includes(funbox);

configToggleFunbox(funbox, false);

if (!getActiveFunboxNames().includes(funbox)) {
const isActive = getActiveFunboxNames().includes(funbox);

if (wasActive && !isActive) {
get(funbox).functions?.clearGlobal?.();
} else {
}

if (!wasActive && isActive) {
get(funbox).functions?.applyGlobalCSS?.();
}
}
Comment on lines 35 to 78
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The changes to toggleFunbox() and setFunbox() fix a critical bug but lack test coverage. Consider adding tests that verify:

  1. clearGlobal() is called when a funbox is deselected
  2. applyGlobalCSS() is called when a funbox is selected
  3. Multiple funbox effects are properly cleared when switching between different sets

This would prevent regression of the issue described in #7468.

Copilot uses AI. Check for mistakes.
Expand All @@ -77,8 +88,8 @@ export async function clear(): Promise<boolean> {
);

qs(".funBoxTheme")?.remove();

qs("#wordsWrapper")?.show();

MemoryTimer.reset();
ManualRestart.set();
return true;
Expand All @@ -93,8 +104,6 @@ export async function activate(
Config.funbox = funbox;
}

// The configuration might be edited with dev tools,
// so we need to double check its validity
if (!checkCompatibility(getActiveFunboxNames())) {
Notifications.add(
Misc.createErrorMessage(
Expand All @@ -105,9 +114,7 @@ export async function activate(
),
-1,
);
setConfig("funbox", [], {
nosave: true,
});
setConfig("funbox", [], { nosave: true });
await clear();
return false;
}
Expand All @@ -121,30 +128,22 @@ export async function activate(
const { data: language, error } = await tryCatch(
JSONData.getCurrentLanguage(Config.language),
);

if (error) {
Notifications.add(
Misc.createErrorMessage(error, "Failed to activate funbox"),
-1,
);
setConfig("funbox", [], {
nosave: true,
});
setConfig("funbox", [], { nosave: true });
await clear();
return false;
}

if (language.ligatures) {
if (isFunboxActiveWithProperty("noLigatures")) {
Notifications.add(
"Current language does not support this funbox mode",
0,
);
setConfig("funbox", [], {
nosave: true,
});
await clear();
return;
}
if (language.ligatures && isFunboxActiveWithProperty("noLigatures")) {
Notifications.add("Current language does not support this funbox mode", 0);
setConfig("funbox", [], { nosave: true });
await clear();
return;
}

let canSetSoFar = true;
Expand All @@ -155,58 +154,57 @@ export async function activate(
configValue,
getActiveFunboxes(),
);

if (check.result) continue;
if (!check.result) {
if (check.forcedConfigs && check.forcedConfigs.length > 0) {
if (configKey === "mode") {
setConfig("mode", check.forcedConfigs[0] as Mode);
}
if (configKey === "words") {
setConfig("words", check.forcedConfigs[0] as number);
}
if (configKey === "time") {
setConfig("time", check.forcedConfigs[0] as number);
}
if (configKey === "punctuation") {
setConfig("punctuation", check.forcedConfigs[0] as boolean);
}
if (configKey === "numbers") {
setConfig("numbers", check.forcedConfigs[0] as boolean);
}
if (configKey === "highlightMode") {
setConfig("highlightMode", check.forcedConfigs[0] as HighlightMode);
}
} else {
canSetSoFar = false;
break;
}

const value = check.forcedConfigs?.[0];

if (value === undefined) {
canSetSoFar = false;
break;
}

if (configKey === "mode" && typeof value === "string") {
setConfig("mode", value as Mode);
}

if (configKey === "words" && typeof value === "number") {
setConfig("words", value);
}

if (configKey === "time" && typeof value === "number") {
setConfig("time", value);
}

if (configKey === "punctuation" && typeof value === "boolean") {
setConfig("punctuation", value);
}

if (configKey === "numbers" && typeof value === "boolean") {
setConfig("numbers", value);
}

if (configKey === "highlightMode" && typeof value === "string") {
setConfig("highlightMode", value as HighlightMode);
}
}

if (!canSetSoFar) {
if (Config.funbox.length > 1) {
Notifications.add(
`Failed to activate funboxes ${Config.funbox}: no intersecting forced configs. Disabling funbox`,
-1,
);
} else {
Notifications.add(
`Failed to activate funbox ${Config.funbox}: no forced configs. Disabling funbox`,
-1,
);
}
setConfig("funbox", [], {
nosave: true,
});
Notifications.add(
`Failed to activate funbox: no intersecting forced configs. Disabling funbox`,
-1,
);
setConfig("funbox", [], { nosave: true });
await clear();
return;
}

ManualRestart.set();

for (const fb of getActiveFunboxesWithFunction("applyConfig")) {
fb.functions.applyConfig();
}
// ModesNotice.update();

return true;
}

Expand Down Expand Up @@ -235,20 +233,22 @@ async function setFunboxBodyClasses(): Promise<boolean> {

body?.setAttribute(
"class",
[...new Set([...currentClasses, ...activeFbClasses]).keys()].join(" "),
[...new Set([...currentClasses, ...activeFbClasses])].join(" "),
);

return true;
}

async function applyFunboxCSS(): Promise<boolean> {
qs(".funBoxTheme")?.remove();

for (const funbox of getActiveFunboxesWithProperty("hasCssFile")) {
const css = document.createElement("link");
css.classList.add("funBoxTheme");
css.rel = "stylesheet";
css.href = "funbox/" + funbox.name + ".css";
document.head.appendChild(css);
}

return true;
}
Loading
Loading