refactor(status-page): tighten the theme-fields backend after second review
Apply review feedback: - Extract makeStatusPage from the two test files that duplicated it (test/unit/services/statusPageService.test.ts and test/unit/controllers/statusPageThemeFlag.test.ts) into test/helpers/makeStatusPage.ts. Both files now import the shared helper, matching the existing test/helpers/ convention used for repository fakes and provider contracts - Add a blank line in statusPageController.getStatusPageByUrl between the normalizedMonitors map and the themesEnabled lookup so the response-shaping section breathes a bit - Drop the obsolete localStorage references from the plan doc that survived from an earlier draft of the design — the public page never persisted a per-visitor mode choice in localStorage, so the test bullet under 2.11 and the risk row both pointed at functionality that does not exist
This commit is contained in:
1 parent
a5af4e48df
commit
a2786a77ec
5 files changed
+18
-34
No files matched your search
@@ -232,7 +232,7 @@ Keys to add:
|
||||
**Component tests (existing test setup):**
|
||||
- Admin form: selecting a theme card updates form state and survives submit
|
||||
- Admin form: `themeMode` dropdown changes payload
|
||||
- `StatusPageThemeProvider`: `auto` mode resolves to `prefers-color-scheme` when no localStorage; localStorage overrides when set; admin-forced mode wins over both
|
||||
- `StatusPageThemeProvider`: `auto` mode resolves to `prefers-color-scheme`; admin-forced `light`/`dark` modes win over the OS setting
|
||||
- Mode resolution: `themeMode="auto"` picks up `prefers-color-scheme`; `themeMode="light"` and `"dark"` lock the mode regardless of OS setting
|
||||
- Admin picker shows both light + dark preview SVGs for every theme, regardless of admin UI mode
|
||||
- Jest/Vitest snapshot tests (plain, no visual-regression tool): `Status/index.tsx` rendered under each of `refined | modern | bold | editorial` × `light | dark`. 8 snapshots via a single parametrised test.
|
||||
@@ -263,7 +263,6 @@ Two PRs, small-enough individually to review carefully.
|
||||
| Four themes = 4× visual-regression surface going forward | Parametrised snapshot tests in PR 2 cover all 4 themes × both modes × key status states. Regressions caught at PR time. |
|
||||
| Editorial hairline dividers fail WCAG 3:1 for UI borders | Axe pass in 2.12; darken divider token until it clears. |
|
||||
| Older clients posting create/update without `theme` break | Validator makes fields optional; model default handles it. Covered by backend test 1.6. |
|
||||
| Per-page localStorage theme mode leaks across status pages | Storage key namespaced by `statusPage.url`. Covered in 2.3 and 2.11. |
|
||||
| `customCSS` collides with theme tokens | Themes set CSS variables on a wrapper; `customCSS` is injected inline after and can override any `--sp-*` variable. No code change. Document in settings help text. |
|
||||
| Shared chart components look off-brand on bold/modern dark | Decided in 2.6: pass an optional `paletteOverride` prop from the status page subtree. |
|
||||
| Rollback needed if feature misbehaves in prod | Feature flag `STATUS_PAGE_THEMES_ENABLED` in 1.5 disables writes and forces `refined` on reads. |
|
||||
|
||||
@@ -130,6 +130,7 @@ class StatusPageController implements IStatusPageController {
|
||||
}
|
||||
return { ...monitor, checks: normalizedChecks };
|
||||
});
|
||||
|
||||
const themesEnabled = this.settingsService.areStatusPageThemesEnabled();
|
||||
return res.status(200).json({
|
||||
success: true,
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
import type { StatusPage } from "../../src/types/index.ts";
|
||||
|
||||
export const makeStatusPage = (overrides?: Partial<StatusPage>): StatusPage =>
|
||||
({
|
||||
id: "sp-1",
|
||||
teamId: "team-1",
|
||||
userId: "user-1",
|
||||
url: "my-status-page",
|
||||
companyName: "Test Co",
|
||||
monitors: ["mon-1"],
|
||||
createdAt: "2026-01-01T00:00:00Z",
|
||||
updatedAt: "2026-01-01T00:00:00Z",
|
||||
...overrides,
|
||||
}) as StatusPage;
|
||||
@@ -1,22 +1,7 @@
|
||||
import { describe, expect, it } from "@jest/globals";
|
||||
import { applyDefaultTheme, withoutThemeFields } from "../../../src/controllers/statusPageThemeFlag.ts";
|
||||
import { DEFAULT_STATUS_PAGE_THEME, DEFAULT_STATUS_PAGE_THEME_MODE } from "../../../src/types/statusPage.ts";
|
||||
import type { StatusPage } from "../../../src/types/index.ts";
|
||||
|
||||
const makeStatusPage = (overrides?: Partial<StatusPage>): StatusPage =>
|
||||
({
|
||||
id: "sp-1",
|
||||
teamId: "team-1",
|
||||
userId: "user-1",
|
||||
url: "my-status-page",
|
||||
companyName: "Test Co",
|
||||
monitors: ["mon-1"],
|
||||
theme: "modern",
|
||||
themeMode: "dark",
|
||||
createdAt: "2026-01-01T00:00:00Z",
|
||||
updatedAt: "2026-01-01T00:00:00Z",
|
||||
...overrides,
|
||||
}) as StatusPage;
|
||||
import { makeStatusPage } from "../../helpers/makeStatusPage.ts";
|
||||
|
||||
describe("statusPageThemeFlag", () => {
|
||||
describe("withoutThemeFields", () => {
|
||||
|
||||
@@ -1,22 +1,7 @@
|
||||
import { describe, expect, it, jest } from "@jest/globals";
|
||||
import { StatusPageService } from "../../../src/service/business/statusPageService.ts";
|
||||
import type { IStatusPagesRepository } from "../../../src/repositories/status-pages/IStatusPagesRepository.ts";
|
||||
import type { StatusPage } from "../../../src/types/index.ts";
|
||||
|
||||
// ── Helpers ──────────────────────────────────────────────────────────────────
|
||||
|
||||
const makeStatusPage = (overrides?: Partial<StatusPage>): StatusPage =>
|
||||
({
|
||||
id: "sp-1",
|
||||
teamId: "team-1",
|
||||
userId: "user-1",
|
||||
url: "my-status-page",
|
||||
companyName: "Test Co",
|
||||
monitors: ["mon-1"],
|
||||
createdAt: "2026-01-01T00:00:00Z",
|
||||
updatedAt: "2026-01-01T00:00:00Z",
|
||||
...overrides,
|
||||
}) as StatusPage;
|
||||
import { makeStatusPage } from "../../helpers/makeStatusPage.ts";
|
||||
|
||||
const createRepo = () =>
|
||||
({
|
||||
|
||||
Reference in new issue
Block a user