Add in-app Game Theme Editor (BL-16618) - #8118
Closed
hatton wants to merge 10 commits into
Closed
Conversation
Introduce a standalone Game Theme Editor web app (src/gameThemeEditor) and wire it into Bloom. The editor lets users view and customize the CSS color variables that drive game themes, with a color picker, a per-color outline, and a contrast checker that flags pairs failing WCAG/UI contrast minimums. - New React/TypeScript app under src/gameThemeEditor with its own build (package.json, tsconfig, host interface) hosted in a draggable/resizable frame. - Host integration from the Games toolbox: ThemeChooser launches the editor and gameThemeEditorHost bridges editor and Bloom. - C# GameThemeEditorApi to read/write theme variables, registered in ProjectContext. - BloomBrowserUI build wiring (vite.config, tsconfig, package.json, yarn.lock).
Brings the in-app Game Theme Editor branch (364 commits behind) up to date with master. Notable resolutions: - Adopted master's yarn->pnpm migration: removed yarn.lock, kept the pnpm-lock.yaml / pnpm-workspace.yaml from master. - package.json: took master's exact version pins over the branch's ^ ranges (react-table 6.11.5, react-tabs 3.2.2, etc.), while preserving react-rnd 10.4.13 which the editor feature (gameThemeEditorHost.ts) depends on and master never carried. - Ran `pnpm install --lockfile-only` so pnpm-lock.yaml includes react-rnd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…es (BL-16323) Brings the in-app game theme editor branch back to life on top of a freshly merged master, and finishes the game-theme contrast and localization work. Build: the editor is a self-contained project whose source lives in a sibling directory (src/gameThemeEditor/src), outside BloomBrowserUI. After master's yarn->pnpm migration and its new tsgo type-check gate, two build steps could no longer resolve the editor's bare dependency imports (@emotion/react, @emotion/cache, react-dom, react-rnd) by walking up out of that folder: - Type check: added tsconfig "paths" entries mapping those specifiers into BloomBrowserUI/node_modules, mirroring the existing "react"/"gameThemeEditor" mappings. - Production build (vite / Rollup): added a small, build-only Vite plugin that re-resolves those ids from BloomBrowserUI's root, scoped to importers inside the editor directory so the rest of the app is untouched and react stays deduped. This is the build-time counterpart to the dev server's existing optimizeDeps.include handling. Localization: added the three missing Games-tool theme-chooser strings to BloomLowPriority.xlf (NewTheme "New…", CustomizeTheme "Customize…", EditThemeColors "Edit theme colors"), each translate="no" with a translator context note, and routed the edit-button tooltip through useL10n. Themes: further contrast fixes to the game themes (gamesThemes.less). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…X (BL-16323) Two preflight fixes to the game theme editor branch: pnpm-lock.yaml had been rewritten in raw pnpm style (2-space indent, single quotes) instead of the repo's prettier-formatted style (4-space, double quotes). Nothing about the dependency graph actually changed -- the file is not in .prettierignore, so prettier owns it -- but the reformatting inflated the diff to ~30,400 lines and would have made merging master (151 commits ahead) needlessly painful. Running prettier over it reduces the change to the 58 lines that genuinely add react-rnd and its transitive deps (react-draggable, re-resizable, tslib). tsconfig.json was missing path mappings for @emotion/react/jsx-runtime and jsx-dev-runtime. The editor sets jsxImportSource to @emotion/react, so every .tsx file under ../gameThemeEditor resolves its JSX factory through that subpath; from the sibling directory the lookup failed and all eight components reported TS2875. Because TS2875 is not one of the blunder-class codes the typecheck gate fails on, this went unnoticed and the editor's JSX was silently going unchecked. With the mappings in place the editor type-checks clean, so this closes the hole without hiding any existing errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
UpsertThemeRule passed the generated CSS rule to Regex.Replace as a replacement
string. .NET interprets "$" in a replacement as a substitution token ($1, $&,
${name}), so any theme variable whose value contained a "$" would have been
silently mangled when overwriting an existing rule -- while appending a brand-new
rule, which does not go through Regex.Replace, would have written it correctly.
Substituting via a MatchEvaluator inserts the rule verbatim. Slugs are unaffected
either way (slugify() strips them to [\w-]).
GameThemeEditorPanel decided whether to auto-focus and select the name field by
testing displayName.startsWith("Untitled"). The panel already computes isNewTheme
a hundred lines earlier from the host's explicit "new" signal
(getNewThemeName() !== null), so the string sniff was both redundant and wrong at
the edges: it would have selected the name of an existing user theme actually
called something like "Untitled sketch", and it silently coupled focus behavior to
the English wording of the suggested name, so localizing "Untitled Theme N" later
would have broken it with no compile-time signal. Use isNewTheme.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # DistFiles/localization/en/BloomLowPriority.xlf
…on/cache (BL-16323) Two findings from Devin's review of 5aee1b2. ThemeChooser's two mount-only effects (the editor open-state subscription and the developer-capability fetch) were written as bare useEffect(..., []). The front-end AGENTS.md asks for the useMountEffect helper instead, which exists precisely so the single justified exhaustive-deps suppression lives in one place rather than being rewritten per call site. No runtime change; the third effect in this file has real dependencies and is left alone. @emotion/cache was imported by the editor (src/gameThemeEditor/src/index.tsx) but declared nowhere: BloomBrowserUI depends only on @emotion/core, /react and /styled, so the package was reaching it as a transitive dependency of @emotion/react. That resolves today, and the production bundle builds, but pnpm's isolated node_modules layout only guarantees direct dependencies at the project root, so this was working by accident of the current layout rather than by declaration. Added it as a direct dependency pinned to 11.10.5 -- the version already resolved in the tree -- so nothing else moves. The lockfile is re-prettified after the install: pnpm rewrites it in its own 2-space style, which is what produced the ~30,000-line diff cleaned up in 7716b7b. The net lockfile change here is 41 lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…6323) Two more findings from Devin's review of d979ffc. mount() created a fresh Emotion cache on every call, anchored to the page document's <head>, and unmount() only tore down the React root. Emotion's <style> elements live in that head rather than inside the container, so removing the container (which the host does on close) left them behind -- and because the host recreates the container on each open, every open/close cycle added another full set of style nodes to the page. The cache is now kept alongside the root and reused for re-renders instead of being recreated, and unmount() calls cache.sheet.flush(), Emotion's own API for removing the elements it inserted. The default save target was chosen with a stacked ternary. The root AGENTS.md explicitly rules that out ("Avoid stacking/nesting ternary operators ... Use an if/else-if chain (or a switch) instead"), so it is now an if/else-if chain. No behavior change. Verified with typecheck, the full Vitest suite, and an isolated production Vite build (agent-vite), since the Emotion change affects real bundling rather than just types. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…6323) Devin promoted this to a bug on re-review of fae350e, and it holds up. save() guards against writing over a different theme with `theme.slug !== startingSlug && host.themeExists(theme.slug)`. But startingSlug is whatever theme the page was displaying when the editor opened, and for "New…" and "Customize…" the host deliberately switches the page to the base theme first. So if the user names the new theme after the one it was based on, the first half of the condition is false, the existence check never runs, and Save writes that slug with no warning. Where the base is a custom book/collection theme, that overwrites its colors outright; where it is a factory theme, it silently creates an override the user did not ask for. Comparing against renameFromSlug instead gives exactly the intended semantics: it is "" for a new theme, so any existing name now trips the guard; it equals the theme's own slug when editing in place, so saving over yourself is still allowed; and it is the old slug when renaming, so the new name is still checked. The guard already had the right intent -- it was just comparing against the wrong thing. Likelihood is low, since new themes are pre-seeded with "Untitled Theme N" and the user has to retype the base name, but the failure is silent data loss, so it is worth the one-word fix. Also updates two PAPERCUTS entries with what this run cost: - the agent-dotnet environmental-failure baseline is 19 now, not 9, and needs the trx logger to enumerate; recording it by test-class pattern rather than count. - the pnpm-lock format drift is prettier's formatting, not an old pnpm style, so `prettier --write pnpm-lock.yaml` after an install fixes it in one step -- no hand-patching of hashes as that entry previously advised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The factory theme file had been round-tripped through the editor's "Save Factory Theme to Source" path, which rewrote each theme block alphabetically, stripped the explanatory comments, and dropped values it considered redundant with what they would derive anyway. That silently undid part of the contrast work already shipped in PR #7985 (BL-16323): white-and-orange-on-blue lost --game-control-button-bg-color and --game-checkbox-outline-color, and coral-reef lost --game-control-button-bg-color, leaving the control-button background to fall back to #ffb453 -- 2.81:1 on the blue page, under the 3:1 UI minimum that PR was specifically fixing. The editor work does not need any of this, so restore the file to master's version. Any genuine theme adjustments can be redone deliberately later, on their own card. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
[Claude Opus 5 (1M context) following a prompt from Hatton] Superseded by #8148. The card for this work is BL-16458 (https://issues.bloomlibrary.org/youtrack/issue/BL-16458); BL-16618 was a duplicate. The branch was renamed |
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
1 participant
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.
In-app Game Theme Editor
Adds a self-contained editor for Bloom game themes (named sets of CSS custom properties that style drag-activity game pages), reachable from the Games tool's theme chooser in the Edit-tab toolbox.
What's here
src/gameThemeEditor/— a self-contained React/TypeScript editor project (color pickers, contrast checker, draggable/resizable floating panel). It depends on nothing in BloomBrowserUI; the only coupling is theIGameThemeEditorHostcontract plusmount()/unmount().bookEdit/toolbox/games/gameThemeEditorHost.tsmounts the editor over the live page for real-time recoloring;ThemeChooser.tsxgains New…, Customize…, and an edit (pencil) button that open it.GameThemeEditorApi— reads/writes theme variables (collection + developer-only factory source).paths(type check) and a small, build-only, editor-scoped Vite plugin (production build) — the build-time counterpart to the dev server's existingoptimizeDepshandling.BloomLowPriority.xlf(NewTheme,CustomizeTheme,EditThemeColors) and routed the edit-button tooltip throughuseL10n.No longer here: theme colour changes
Earlier revisions of this branch also modified
gamesThemes.less. Those changes were a side effect of round-tripping the file through the editor's own "Save Factory Theme to Source" path, and they partly undid the contrast work already shipped in #7985 (BL-16323). That file is now reverted to master; any real theme adjustments will be redone deliberately on their own card.Still open from the #8086 review
These four were escalated as developer decisions and have not been resolved:
translate="no"in the new XLF entries (assessed as the documented convention, awaiting confirmation).Notes for reviewers
Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16618
Previous PR: #8086
This change is