refactor: collapse the Setup steps and fix the card content model - #70
Merged
Merged
Conversation
**Three steps, one shape (#48).** `renderCharacterStep`, `renderSkillStep` and `renderScenarioStep` did the same seven things in the same order; only the title, prompt, collection and target field varied. `renderChoiceStep` takes those, plus an optional `extra` for the character pips — the one structural difference between them. The heading had already drifted from the copying (h3 for characters and skills, h4 for scenarios); now there is one element to choose. The `onChange: () => void` parameter goes with it: all three call sites passed `() => render()`, and the collapsed function is defined where `render` is already in scope. **Cards are conforming HTML again (#47).** Every card was a `<button>` with an `<h3>`, a `<p>` and — for characters — a `<ul>` of `<li>`s appended to it. `button`'s content model is phrasing content, and assistive technology builds a button's accessible name from its subtree, so each character card announced as the archetype name plus the whole flavour paragraph plus every pip label. The children are now spans, with an `aria-label` carrying just the name. Verified in the browser: the card contains only `SPAN` elements, zero flow-content elements, and its accessible name is "The Monk" rather than a ninety-character run. `aria-pressed` is added while the builder was open — selection was previously communicated only by colour. **The scenario detail had two copies (#40).** `renderRecallPanel` and Setup's detail block both walked the profile, emitted a rules heading, then emitted the rules. `letterhead.ts` becomes `fragments.ts` and holds both fragments, per #40's note that the repo does not need two one-function screen helpers. The function takes the wrapper class, an optional profile heading and an optional heading class, so both call sites keep the appearance they had rather than one silently changing. The visual result is unchanged, which is the thing worth checking here since neither screen has a test. Captured full-page screenshots either side of the change: the first pass came out 8px shorter, because the blurb `<p>` carried UA default margins that a span does not. `.card__blurb` now reproduces them and both renders are 2048x2002 with identical layout. The recall panel's heading computes to the same font, size, margin, transform and colour as the `<p class="small-caps">` it replaced. `setup.ts` 225 → 196 lines; `play.ts` 677 → 653. Closes #48 Closes #47 Closes #40 Co-Authored-By: Claude
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.
Closes #48, #47, #40 — all of milestone 6, one PR, because #48 has to land before #47 or the accessibility fix gets made three times instead of once.
Three steps, one shape (#48)
renderCharacterStep,renderSkillStepandrenderScenarioStepdid the same seven things in the same order; only the title, prompt, collection and target field varied.renderChoiceSteptakes those, plus an optionalextrafor the character pips — the one structural difference between them.The heading had already drifted from the copying (
h3for characters and skills,h4for scenarios, chosen by nobody); now there is one element to choose. TheonChange: () => voidparameter goes too — all three call sites passed() => render(), wrapping a function that was already() => void.Cards are conforming HTML again (#47)
Every card was a
<button>with an<h3>, a<p>, and for characters a<ul>of<li>s appended to it.button's content model is phrasing content — and assistive technology builds a button's accessible name from its subtree, so each character card announced as the archetype name plus the whole flavour paragraph plus every pip label.Children are now spans, with an
aria-labelcarrying just the name. Verified in the browser rather than by reading:That last line is what used to be the accessible name.
aria-pressedis added while the builder was open — selection was previously communicated by colour alone.Not doing: converting the group to
role="radiogroup". These are a single-select set so radio semantics would be more correct, but it changes keyboard behaviour and is beyond what #47 asks. Happy to file it separately.The scenario detail had two copies (#40)
renderRecallPaneland Setup's detail block both walked the profile, emitted a rules heading, then emitted the rules.letterhead.tsbecomesfragments.tsand holds both, per #40's own note that the repo doesn't need two one-function screen helpers. The function takes the wrapper class, an optional profile heading and an optional heading class, so both call sites keep the appearance they had rather than one silently changing to match the other.Verification — the visual result is unchanged
Neither screen has a test, so this is the thing that actually needed checking. I captured full-page screenshots either side of the change by stashing.
The first pass came out 8px shorter. The blurb
<p>carried UA default margins that a<span>does not..card__blurbnow reproduces them, and both renders are 2048×2002 with identical layout — same card positions, same text wrapping, same pip rows.The recall panel's heading computes to the same font, size (16.2px), margin (9px 0), transform and colour as the
<p class="small-caps">it replaced.Step gating still works: 1 step → 2 → 3 → Begin button, with
aria-pressedandcard--selectedboth tracking selection.bun run check:ciclean, 83 tests.setup.ts225 → 196;play.ts677 → 653.THEORY.mdandWALKTHROUGH.mduntouched.