Expose board background settings per board size

[lykahb]

This depends on the goban renderer support in: online-go/goban#252. For motivation see the screenshots of themed boards in that PR.

The changes are made with Codex. I've reviewed them and closely steered the technical design.

The existing custom board background URL remains the default background. This PR adds three optional URLs for baked-grid board assets:

• 9x9
• 13x13
• 19x19

If a matching baked-grid URL is absent, invalid, or not applicable to the current board, rendering falls back to the default custom board background and
vector grid.

Screenshots

What this PR enables

This is of the themes I made and plan to release as Creative Commons once OGS supports backgrounds per board size.

It relies on two features:

• Backgrounds per board size that this PR adds
• Stone variants. Sabaki themes support it, and I plan to open a follow-up PR to OGS with that too.

Details

• Adds goban-theme-custom-board-grid-backgrounds preference.
• Adds a custom board grid background URL picker to the Goban theme picker.
• Passes the optional grid background map through selected goban themes.
• Adds z-index variables for the new goban grid and baked-grid background layers.
• Leaves existing custom board settings backward-compatible.

Compatibility

Existing customized boards are unaffected:

• goban-theme-custom-board-url keeps its current meaning.
• The new preference defaults to empty URLs.
• Boards without a matching baked-grid asset use the existing board/vector-grid rendering.

Testing

I followed the testing recommendations in the CONTRIBUTION doc, and separately from that tested for graceful degradation when the asset URL's become unavailable.

[github-actions]

Review: Expose board background settings per board size

Two issues worth addressing:

---

Potential state loss in setGridBackground (stale closure)

setGridBackground spreads the captured grid_backgrounds closure directly:

function setGridBackground(size: keyof CustomBoardGridBackgrounds, url: string): void {
    setGridBackgrounds({
        ...grid_backgrounds,
        [size]: url,
    });
}

Under React 18 automatic batching, if two calls to setGridBackground are enqueued before a re-render (e.g., two rapid reset clicks, or programmatic calls), both will spread the same stale snapshot of grid_backgrounds. The second update will silently drop the first change.

Use the functional update form instead:

function setGridBackground(size: keyof CustomBoardGridBackgrounds, url: string): void {
    setGridBackgrounds((prev) => ({ ...prev, [size]: url }));
}

---

Misleading z-index variable name

--z-goban-grid-layer: 5;
--z-goban-grid-background-layer: 8;

--z-goban-grid-background-layer is numerically above --z-goban-grid-layer. The "background" naming implies lower stacking order, which is the opposite of what's defined here. If the intent is that the baked-grid image renders on top of the vector grid to visually replace it, a name like --z-goban-baked-grid-layer would communicate that clearly and prevent future developers from "correcting" the ordering.

[github-actions]

Review: Bugs & Performance

Stale closure in setGridBackground (GobanCustomBoardGridBackgroundPicker.tsx)

setGridBackground closes over grid_backgrounds from the current render rather than using the functional updater form:

// current — second call in a batched update discards the first
setGridBackgrounds({
    ...grid_backgrounds,
    [size]: url,
});

// correct
setGridBackgrounds((prev) => ({ ...prev, [size]: url }));

Under React 18's default batching this is safe for typical one-key-at-a-time edits, but autofill can trigger multiple onChange events in the same batch, causing all but the last update to be silently dropped.

---

Inverted z-index naming (src/global_styl/01_variables.css)

--z-goban-grid-layer: 5;
--z-goban-grid-background-layer: 8;

--z-goban-grid-background-layer has a higher z-index than --z-goban-grid-layer. The "background" suffix conventionally implies a lower layer; here it sits above the grid layer. If the intent is for the baked-grid image to overlay and replace the vector grid, the variable name should reflect that (e.g. --z-goban-baked-grid-layer) to prevent future maintainers from assigning the wrong value when inserting new layers between these two.

[github-actions]

In GobanCustomBoardGridBackgroundPicker.tsx, getResetGridBackgroundLabel(size) is called twice per button — once for title and once for aria-label — despite returning the same string. This means 6 redundant pgettext lookups per render cycle. Consider computing it once per row:

const reset_label = getResetGridBackgroundLabel(size);
// ...
<button
    title={reset_label}
    aria-label={reset_label}
>

[github-actions]

Code Review

Overview: Adds per-board-size baked-grid background URL settings (9×9, 13×13, 19×19), wiring them through usePreference, getSelectedThemes, and a new GobanCustomBoardGridBackgroundPicker component. The goban submodule is bumped to the companion renderer PR.

One finding:

In GobanCustomBoardGridBackgroundPicker.tsx, getResetGridBackgroundLabel(size) is called twice per button — once for title and once for aria-label — on every render:

title={getResetGridBackgroundLabel(size)}
aria-label={getResetGridBackgroundLabel(size)}

Each call goes through pgettext(). Extracting the result to a variable inside the .map() callback would avoid the duplicate lookup:

const resetLabel = getResetGridBackgroundLabel(size);
// ...
title={resetLabel}
aria-label={resetLabel}

Otherwise the implementation looks solid: the useRef stale-closure workaround is well-motivated and correctly implemented, the new preference is properly wired through defaults, getSelectedThemes, and watchSelectedThemes, and the z-index layering variables are a clear way to document the new rendering stack.

[github-actions]

Code Review

GobanCustomBoardGridBackgroundPicker.tsx — stale ref window in useEffect sync

The component correctly uses a ref to handle batched updates (since usePreference setters don't accept functional updates). However, the ref is synced via useEffect:

React.useEffect(() => {
    grid_backgrounds_ref.current = grid_backgrounds;
}, [grid_backgrounds]);

useEffect runs asynchronously after painting. Between the render (when grid_backgrounds has a new value) and the effect (when the ref is updated), grid_backgrounds_ref.current holds stale data. If setGridBackground is called in that window — e.g. when an external preference change (reset, sync from another tab via the watch subscription in usePreference) triggers a re-render just as the user edits a field — the call will spread the stale ref and silently overwrite the external update.

The standard React pattern for an always-current ref is to assign it directly during render, not in useEffect:

const grid_backgrounds_ref = React.useRef(grid_backgrounds);
grid_backgrounds_ref.current = grid_backgrounds; // synchronous, no useEffect needed

This removes the stale window entirely and is simpler. The synchronous batching behaviour described in the comment is preserved because setGridBackground still writes back to the ref before calling setGridBackgrounds.

[github-actions]

Code Review

I reviewed this PR for bugs and performance issues and the implementation looks solid overall. No significant bugs or performance issues were found.

Two minor observations (not blocking):

1. The label helper functions (getGridBackgroundLabel, getResetGridBackgroundLabel) use switch statements with no default clause. TypeScript exhaustiveness analysis handles this correctly today since the keys are a closed union, but if CustomBoardGridBackgrounds ever gains a new size key, both functions would silently return undefined at runtime and render empty labels. A defensive default branch would future-proof this.

2. In the JSX, the reset-label helper is called twice per row -- once for the title attribute and once for aria-label. Worth computing once per iteration.

Neither affects correctness in the current implementation. The ref-based pattern for avoiding stale closures is well-justified and handles batched updates correctly.

[github-actions]

Code Review

grid_background_sizes array can silently fall out of sync with CustomBoardGridBackgrounds

In GobanCustomBoardGridBackgroundPicker.tsx, the board sizes used to render input rows are hardcoded in a separate array:

const grid_background_sizes: (keyof CustomBoardGridBackgrounds)[] = ["9", "13", "19"];

Meanwhile, getGridBackgroundLabel and getResetGridBackgroundLabel use switch statements over the same keys. These two approaches have mismatched type-safety guarantees: if a new board size is added to CustomBoardGridBackgrounds, TypeScript will produce a "not all code paths return a value" error in the switch functions — but grid_background_sizes will silently continue listing only the old sizes, so the new board size's input row will simply never be rendered.

Deriving the array from emptyCustomBoardGridBackgrounds would keep it automatically in sync:

const grid_background_sizes = Object.keys(emptyCustomBoardGridBackgrounds) as (keyof CustomBoardGridBackgrounds)[];

This also makes the switch exhaustiveness the single source of truth, rather than two separate lists that must be kept in sync manually.

[github-actions]

Reviewed for bugs and performance issues — the implementation is clean. A few notes:

Merge dependency: The goban submodule is pinned to a commit from online-go/goban#252, which is still open. This OGS PR should not be merged until that goban PR lands on its main branch and the submodule is updated to that merged commit, otherwise the pointer could become stale if the goban branch is rebased or squashed.

Ref-based state batching: The useRef workaround for the lack of functional-update support in usePreference is valid — the ref is updated synchronously inside setGridBackground before the state setter is called, so concurrent edits accumulate correctly. The inline comment explains the rationale well.

No other issues found.

[github-actions]

Review

This PR adds per-board-size baked-grid background URLs (9×9, 13×13, 19×19) to the custom board theme picker. The preference is stored as a single structured object, threaded through `getSelectedThemes` and `watchSelectedThemes`, and exposed via a new `GobanCustomBoardGridBackgroundPicker` component. The design is consistent with how sibling custom-theme pickers work and the z-index layering comment accurately describes the intended fallback behavior.

---

GobanCustomBoardGridBackgroundPicker.tsx — unguarded object key access on controlled input

`value={grid_backgrounds[size]}` is safe today (the default always carries all three keys), but `grid_backgrounds` is a raw deserialized preference object with no per-key shape validation. If the stored value ever arrives without one of the size keys — cross-device sync sending a partial object, a future migration that adds a new size, or manual localStorage editing — `grid_backgrounds[size]` is `undefined`. Passing `undefined` to a controlled `` silently switches it to uncontrolled, which React warns about and which prevents the reset button from clearing the field.

The one-character fix:

value={grid_backgrounds[size] ?? ""}

This matches the intent of the reset button (which sets the value to `""`) and makes the component resilient to any future schema evolution.