fix: address code review — test regression, theme consolidation, validation, dead CSS

- Fix test regression: update theme option count 3→4 and midnight index [1]→[2] for neon-glow insertion
- Fix stale Voice & Audio tests: update select count 3→4 (video quality + device added), replace sensitivity slider test with meter bar render check
- Consolidate theme systems: applyTheme() in helpers.ts now delegates body class + persistence to applyThemeByName() so both paths write to owncord:theme:active
- Add input validation to loadCustomTheme(): reject non-object payloads and entries missing name/colors fields
- Remove dead CSS tokens --strip-width and --members-width from tokens.css
- Extend 800px responsive breakpoint to cover .unified-sidebar alongside .channel-sidebar
This commit is contained in:
jevb
2026-03-27 12:22:22 +01:00
parent 6fd4779624
commit 0aeaa50856
7 changed files with 145 additions and 23 deletions
@@ -0,0 +1,48 @@
/**
* ServerStrip component — vertical strip on the far left showing server icons.
* Single-server for now: Home button, separator, add server button.
*/
import { createElement, appendChildren } from "@lib/dom";
import type { MountableComponent } from "@lib/safe-render";
export function createServerStrip(): MountableComponent {
const ac = new AbortController();
let root: HTMLDivElement | null = null;
function mount(container: Element): void {
root = createElement("div", { class: "server-strip", "data-testid": "server-strip" });
const homeIcon = createElement(
"div",
{ class: "server-icon active", style: "background: var(--accent)" },
"O",
);
const separator = createElement("div", { class: "server-separator" });
const addIcon = createElement("div", { class: "server-icon add" }, "+");
// Add server button click — placeholder for future multi-server support
addIcon.addEventListener(
"click",
() => {
// No-op for single-server mode
},
{ signal: ac.signal },
);
appendChildren(root, homeIcon, separator, addIcon);
container.appendChild(root);
}
function destroy(): void {
ac.abort();
if (root !== null) {
root.remove();
root = null;
}
}
return { mount, destroy };
}
@@ -3,6 +3,7 @@
*/
import { createElement } from "@lib/dom";
import { applyThemeByName } from "@lib/themes";
// ---------------------------------------------------------------------------
// Constants
@@ -86,13 +87,12 @@ export function createToggle(
// ---------------------------------------------------------------------------
export function applyTheme(name: ThemeName): void {
const vars = THEMES[name];
// Apply CSS variables for the theme (keeps existing behavior for inline var overrides)
const theme = THEMES[name];
const root = document.documentElement;
for (const [prop, val] of Object.entries(vars)) {
root.style.setProperty(prop, val);
for (const [key, value] of Object.entries(theme)) {
root.style.setProperty(key, value);
}
for (const cls of [...document.body.classList]) {
if (cls.startsWith("theme-")) document.body.classList.remove(cls);
}
document.body.classList.add(`theme-${name}`);
// Delegate body class and persistence to the theme manager
applyThemeByName(name);
}
+10 -2
View File
@@ -80,12 +80,20 @@ export function saveCustomTheme(theme: OwnCordTheme): void {
);
}
/** Loads a custom theme by name, or null if not found / parse error. */
/** Loads a custom theme by name, or null if not found / parse error / invalid shape. */
export function loadCustomTheme(name: string): OwnCordTheme | null {
const raw = localStorage.getItem(STORAGE_KEY_CUSTOM_PREFIX + name);
if (raw === null) return null;
try {
return JSON.parse(raw) as OwnCordTheme;
const parsed: unknown = JSON.parse(raw);
if (
typeof parsed !== "object" || parsed === null ||
typeof (parsed as Record<string, unknown>).name !== "string" ||
typeof (parsed as Record<string, unknown>).colors !== "object"
) {
return null;
}
return parsed as OwnCordTheme;
} catch {
return null;
}
+1 -1
View File
@@ -1969,7 +1969,7 @@
.member-list { width: 0; padding: 0; overflow: hidden; }
}
@media (max-width: 800px) {
.channel-sidebar { width: 0; overflow: hidden; }
.channel-sidebar, .unified-sidebar { width: 0; overflow: hidden; }
}
@@ -105,9 +105,7 @@
--radius-circle: 50%;
/* Spacing */
--strip-width: 72px;
--sidebar-width: 240px;
--members-width: 240px;
--header-height: 48px;
/* Message layout */
@@ -0,0 +1,69 @@
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { createServerStrip } from "@components/ServerStrip";
describe("ServerStrip", () => {
let container: HTMLDivElement;
let comp: ReturnType<typeof createServerStrip>;
beforeEach(() => {
container = document.createElement("div");
document.body.appendChild(container);
});
afterEach(() => {
comp?.destroy?.();
container.remove();
});
it("mounts with server-strip class", () => {
comp = createServerStrip();
comp.mount(container);
expect(container.querySelector(".server-strip")).not.toBeNull();
});
it('renders home icon with "O"', () => {
comp = createServerStrip();
comp.mount(container);
const icons = container.querySelectorAll(".server-icon");
const homeIcon = icons[0];
expect(homeIcon).not.toBeUndefined();
expect(homeIcon?.textContent).toBe("O");
});
it("renders separator", () => {
comp = createServerStrip();
comp.mount(container);
expect(container.querySelector(".server-separator")).not.toBeNull();
});
it('renders add icon with "+"', () => {
comp = createServerStrip();
comp.mount(container);
const addIcon = container.querySelector(".server-icon.add");
expect(addIcon).not.toBeNull();
expect(addIcon?.textContent).toBe("+");
});
it("home icon has active class", () => {
comp = createServerStrip();
comp.mount(container);
const icons = container.querySelectorAll(".server-icon");
const homeIcon = icons[0];
expect(homeIcon?.classList.contains("active")).toBe(true);
});
it("destroy removes DOM", () => {
comp = createServerStrip();
comp.mount(container);
expect(container.querySelector(".server-strip")).not.toBeNull();
comp.destroy?.();
expect(container.querySelector(".server-strip")).toBeNull();
});
});
@@ -153,9 +153,9 @@ describe("SettingsOverlay", () => {
getTab(container, 1).click();
const themeOptions = container.querySelectorAll(".theme-opt");
expect(themeOptions.length).toBe(3);
expect(themeOptions.length).toBe(4);
const midnight = themeOptions[1] as HTMLElement;
const midnight = themeOptions[2] as HTMLElement;
midnight.click();
expect(midnight.classList.contains("active")).toBe(true);
@@ -247,7 +247,8 @@ describe("SettingsOverlay", () => {
getTab(container, 5).click();
const selects = container.querySelectorAll("select.form-input");
expect(selects.length).toBe(3);
// input device, output device, video quality, video device = 4
expect(selects.length).toBe(4);
const sliders = container.querySelectorAll(".settings-slider");
expect(sliders.length).toBeGreaterThanOrEqual(1);
@@ -260,18 +261,16 @@ describe("SettingsOverlay", () => {
overlay.destroy?.();
});
it("persists voice sensitivity setting", () => {
it("renders voice sensitivity meter bar", () => {
const overlay = createSettingsOverlay(defaultOptions);
overlay.mount(container);
getTab(container, 5).click();
// Sensitivity slider is the 3rd .settings-slider (after input volume and output volume)
const sliders = container.querySelectorAll(".settings-slider");
const slider = sliders[2] as HTMLInputElement;
slider.value = "75";
slider.dispatchEvent(new Event("input"));
expect(localStorage.getItem("owncord:settings:voiceSensitivity")).toBe("75");
// Sensitivity is now a draggable meter bar, not a slider.
const meterBar = container.querySelector(".mic-meter-bar") as HTMLElement;
expect(meterBar).not.toBeNull();
const threshold = container.querySelector(".mic-meter-threshold") as HTMLElement;
expect(threshold).not.toBeNull();
overlay.destroy?.();
});