Configurator: TypeScript rewrite + Pages deployment#14
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Replaces the legacy inline HTML configurator with a dedicated TypeScript/Vite app under configurator/, adds unit + e2e test coverage, and introduces a GitHub Actions workflow to build and deploy the configurator to GitHub Pages.
Changes:
- Adds a modular TypeScript configurator (generator, URL/localStorage state, platform presets, DOM UI, syntax highlighting) with Tailwind styling.
- Adds comprehensive Vitest unit tests and Playwright e2e tests for configurator behavior and invariants.
- Adds a Pages deployment workflow that runs typecheck → unit → e2e → build and deploys
configurator/dist.
Reviewed changes
Copilot reviewed 30 out of 34 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| configurator/vite.config.ts | Vite build/test configuration (relative base for Pages/custom domains). |
| configurator/tsconfig.json | Strict TS compiler settings for the configurator app. |
| configurator/src/vite-env.d.ts | Vite client type references. |
| configurator/src/types.ts | Shared domain types for config/platform/validation. |
| configurator/src/options.ts | Central option definitions for boolean + volume fields. |
| configurator/src/options.test.ts | Ensures option table stays in sync with index.html and URL keys. |
| configurator/src/urlState.ts | Encodes/decodes shareable state via query params. |
| configurator/src/urlState.test.ts | Unit tests for URL state encoding/decoding and constraints. |
| configurator/src/persistence.ts | localStorage persistence wrapper mirroring URL encoding format. |
| configurator/src/platforms.ts | Loads platform JSON presets and implements path validation helpers. |
| configurator/src/platforms.test.ts | Validates platform JSON files against schema + behavior tests for validators. |
| configurator/src/generator.ts | Pure generation of compose + docker run outputs with quoting/escaping. |
| configurator/src/generator.test.ts | Snapshot + invariant + security regression tests for generated output. |
| configurator/src/highlight.ts | DOM-based syntax highlighting for output preview (no innerHTML). |
| configurator/src/styles.css | Tailwind theme tokens + accessibility/focus/HC-mode styles. |
| configurator/src/main.ts | App bootstrap, DOM wiring, URL/localStorage hydration, validation + actions. |
| configurator/index.html | New app shell markup, CSP meta, loading overlay, and UI structure. |
| configurator/e2e/configurator.spec.ts | Playwright e2e coverage for UX flows (switching, URL state, validation, extras). |
| configurator/playwright.config.ts | Playwright config running against vite preview build artifact. |
| configurator/public/roon-moon.svg | SVG asset used by the UI/loading overlay. |
| configurator/public/platforms/index.json | Platform manifest controlling dropdown order/default. |
| configurator/public/platforms/schema.json | JSON schema for validating platform preset files in CI. |
| configurator/public/platforms/qnap.json | QNAP platform preset. |
| configurator/public/platforms/synology.json | Synology platform preset. |
| configurator/public/platforms/unraid.json | Unraid platform preset + updated safety guidance. |
| configurator/public/platforms/truenas.json | TrueNAS platform preset with userOverride: "0:0". |
| configurator/public/platforms/linux.json | Linux platform preset defaults. |
| configurator/public/platforms/custom.json | Custom platform placeholder defaults. |
| configurator/package.json | Configurator package scripts and devDependencies. |
| configurator/package-lock.json | Dependency lockfile for reproducible builds. |
| configurator/.gitignore | Ignores dist/test artifacts for the configurator package. |
| .github/workflows/pages.yml | CI workflow to test/build/deploy configurator to GitHub Pages. |
| .gitignore | Adds .playwright-mcp/ ignore at repo root. |
Files not reviewed (1)
- configurator/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
decriptor
pushed a commit
that referenced
this pull request
Apr 25, 2026
Four small fixes from PR review: - styles.css: drop the @import for Inter from fonts.googleapis.com. Inter wasn't actually in the font stack (--font-sans starts with system-ui), so the import was both unused AND would have been blocked by the meta CSP's `style-src 'self'` in production. Removing the import is cleaner than adding the Google Fonts CSP exception for a font we don't apply. - index.html: rewrite the /RoonBackups field description. The input has aria-required="true" and the generator blocks export when empty, so saying "Optional." was contradictory. The new wording clarifies that a host path is required by the generator, while Roon's own Settings > Backups feature is what actually schedules backups. - urlState.test.ts: rename the "completely empty querystring" test to match its assertion. The test asserts `tzOff=1` is present; the description was a leftover from before the tz-default fix. Updated text + comment now explain why the encoder serializes tz=false explicitly instead of omitting it. - pages.yml: pin Node to 20.19.0 (was '20'). Vite 8.0.9 declares engines.node = '^20.19.0 || >=22.12.0'; a runner image stuck on an earlier 20.x would fail `npm ci` with EBADENGINE.
22c2280 to
8b40d8a
Compare
Replaces the existing inline ~1100-line index.html configurator with a proper TypeScript app under configurator/. Same user-facing surface (compose / docker run output, share-link round-trip, platform presets, advanced options), now with unit + e2e tests, type-checked code, and per-file modularity. Highlights: - src/generator.ts: pure Config → string spec/render split. Always emits ROON_INSTALL_BRANCH explicitly (resolves issue #12 — the configurator no longer omits the var for the production case, which was leaving users stranded when the entrypoint had sticky behavior). Quotes user: "0:0" to avoid YAML 1.1 sexagesimal parsing. Defense- in-depth YAML/shell quoting with positive + negative tests for every option. - public/platforms/: 6 presets (qnap, synology, unraid, truenas, linux, custom) as JSON files validated against schema.json. truenas.json uses userOverride: "0:0" to force root inside the container, working around TrueNAS Apps' UID 568 default. unraid.json's hint warns about the cache-only precondition before suggesting /mnt/cache/ — earlier wording risked DB corruption when Mover migrated the SQLite database mid-write. - src/main.ts: DOM orchestration. Single source of truth in DOM, with URL > localStorage > defaults precedence on init. Validation surfaces via aria-invalid + aria-describedby; output regen on every change. - WCAG 2.1 AA: skip link, focus-visible outlines, roving tabindex on the output mode tabs, contrast-tuned surface palette, prefers- reduced-motion + forced-colors handling. - Loading overlay: full-viewport icon (left half-moon + three audio bars pulsing out of phase, modeled on the desktop app's footer_loading animateddrawable). Fades out when the app marks itself ready; window 'load' listener is the failsafe; <noscript> hides the overlay for JS-disabled visitors. - CSP: meta http-equiv with default-src 'self', script-src 'self', no unsafe-eval, frame-ancestors 'none'. The app doesn't render user HTML and uses textContent + createTextNode exclusively, but the CSP catches any future regression. - Tests: 119 Vitest unit tests + 23 Playwright e2e covering generator output, URL state encoding/decoding, platform validation/loading, options uniqueness invariants, and full user flows. e2e tests click the wrapping <label> rather than the sr-only <input> so newer Playwright's post-click state verification passes. - Build: Vite 8, ~22 kB JS gzipped, ~6 kB CSS gzipped.
GitHub Actions workflow that runs typecheck → unit tests → e2e tests → build, then uploads configurator/dist as the Pages artifact and deploys via actions/deploy-pages. Triggers on push to main and pull_request — both with the same paths filter (configurator/** + the workflow itself) so unrelated commits don't burn a runner. The deploy job is gated on push-to-main so PR runs validate the build but never publish PR contents to the live site. Concurrency is per-PR for pull_request events (rapid PR pushes cancel each other) and shared for push events (main-branch deploys queue serially under a single 'pages' group). Node pinned to 20.19.0 — vite@8 declares engines.node ^20.19.0 || >=22.12.0, so a runner image stuck on an earlier 20.x would fail npm ci with EBADENGINE. Playwright HTML report is uploaded on test failure for CI debugging. Manual one-time step required: in repo Settings → Pages, change Source from "Deploy from a branch" to "GitHub Actions". Until that flip happens, this workflow runs successfully but the published site keeps serving the existing root index.html.
8b40d8a to
0c9de24
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the existing inline
index.htmlconfigurator with a proper TypeScript/Vite app underconfigurator/, plus a GitHub Actions workflow that builds and deploys it to Pages.This pairs with #13 to fully resolve issue #12 (MrFibble's branch-downgrade bug): the merged entrypoint now defaults to production on unset env, and this configurator always emits
ROON_INSTALL_BRANCH=explicitly so the user's compose file matches their intent regardless of entrypoint defaults.What's in the box
Configurator (
configurator/)generator,options,platforms,urlState,persistence,highlight,mainmodulesschema.json"Consider /mnt/cache/") risked DB corruption when Mover migrated the SQLite database mid-write; now warns about the cache-only precondition explicitlyuserOverride: "0:0"to force root inside the container, working around TrueNAS Apps' UID 568 defaultfooter_loadinganimateddrawabledefault-src 'self'; script-src 'self'; frame-ancestors 'none')Deploy (
.github/workflows/pages.yml)mainwith a paths filter (configurator/**and the workflow itself)configurator/dist, deploys viaactions/deploy-pagesRequired follow-up after merge
index.html.Defer / known follow-ups
index.html(the legacy configurator) is intentionally left in place for one release cycle as a rollback safety net. After the new configurator is verified on Pages, a follow-up PR will remove it./share/Multimediaover/share/Music), Synology rootPattern broaden for USB/SATA volumes. Tracked separately; both Low/Medium severity.Test plan
npm run typecheckcleannpm test— 119/119npm run build— clean, ~22 kB JS gzRefs #12.