♿ Make the Minesweeper demo keyboard accessible - #147
Conversation
Greptile SummaryThe PR makes the Minesweeper demo a keyboard-navigable ARIA grid, adds screen-reader announcements and accessible structure, and adjusts its responsive layout.
Confidence Score: 4/5The ARIA-disabled state on flagged cells should be corrected before merging because it tells assistive-technology users that the supported unflag action is unavailable. Flagged cells remain operable through toggleFlag, but the new accessibility metadata exposes them as disabled whenever engine availability is false. Files Needing Attention: docs/src/components/MinesweeperDemo.tsx
|
| Filename | Overview |
|---|---|
| docs/src/components/MinesweeperDemo.tsx | Adds the accessible grid, roving focus, and live announcements, but exposes actionable flagged cells as ARIA-disabled. |
| docs/src/lib/minesweeper-focus.ts | Implements straightforward wrapping and row-edge focus calculations. |
| docs/src/lib/minesweeper-focus.test.ts | Covers horizontal and vertical wrapping plus Home/End behavior. |
| docs/src/styles/components/_components.minesweeper-demo.css | Introduces nested row-grid styling, screen-reader-only presentation, and narrow-viewport layout adjustments. |
| docs/package.json | Adds a Bun test script for the documentation project. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
Tab[Tab enters active grid cell] --> Key{Keyboard input}
Key -->|Arrow keys| Move[Calculate wrapped cell]
Key -->|Home or End| Edge[Select row edge]
Key -->|Enter or Space| Activate[Activate focused cell]
Move --> Focus[Update activeCell and focus button]
Edge --> Focus
Activate --> Mode{Interaction mode}
Mode -->|Dig| Reveal[Reveal cell]
Mode -->|Flag| Flag[Toggle flag]
Reveal --> Announce[Update polite live region]
Flag --> Announce
Reviews (1): Last reviewed commit: "♿ a11y: make Minesweeper keyboard access..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
| aria-rowindex={cell.y + 1} | ||
| aria-colindex={cell.x + 1} | ||
| aria-disabled={!cellAvailability.enabled} |
There was a problem hiding this comment.
When a screen-reader user focuses a flagged cell in Flag mode, aria-disabled announces it as disabled even though activating it removes the flag, misleading the user about the available unflag action.
| aria-disabled={!cellAvailability.enabled} | |
| aria-disabled={ | |
| !cellAvailability.enabled && | |
| !(conditions.flagMode && value === "flagged") | |
| } |
|
Good catch. Flagged cells remain actionable in Flag mode because activating
them removes the flag. I updated aria-disabled to reflect that behavior and
manually verified flag removal with Narrator.
…On Tue, Sep 1, 2026 at 1:05 PM greptile-apps[bot] ***@***.***> wrote:
***@***.***[bot]* commented on this pull request.
------------------------------
In docs/src/components/MinesweeperDemo.tsx
<#147 (comment)>:
> + const isMine = display.kind === "mine";
+
+ return (
+ <button
+ key={key}
+ ref={(element) => {
+ cellRefs.current[key] = element;
+ }}
+ type="button"
+ role="gridcell"
+ tabIndex={
+ cell.x === activeCell.x && cell.y === activeCell.y ? 0 : -1
+ }
+ aria-rowindex={cell.y + 1}
+ aria-colindex={cell.x + 1}
+ aria-disabled={!cellAvailability.enabled}
[image: P1] <#m_-8914349970841576379_> *Actionable cells announced
disabled*
When a screen-reader user focuses a flagged cell in Flag mode,
aria-disabled announces it as disabled even though activating it removes
the flag, misleading the user about the available unflag action.
⬇️ Suggested change
- aria-disabled={!cellAvailability.enabled}
+ aria-disabled={
+ !cellAvailability.enabled &&
+ !(conditions.flagMode && value === "flagged")
+ }
—
Reply to this email directly, view it on GitHub
<#147?email_source=notifications&email_token=A5LH6ZAIYLIPSC6UBLVIRU35M36VTA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBYGA4DQMBQHE3KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-5080880096>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A5LH6ZD3W3SFPUGGFTS7QW35M36VTAVCNFSNUABGKJSXA33TNF2G64TZHMYTCOJWGQ4DSMRQHA5US43TOVSTWNJTGE2DMMBUG4YDRILWAI>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/A5LH6ZCVTMFV6MHEIYVAORT5M36VTA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBYGA4DQMBQHE3KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/A5LH6ZCHQN3HXRWD2BK5P2D5M36VTA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBYGA4DQMBQHE3KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
![umpire-bot[bot]](https://avatars.githubusercontent.com/in/3739611?s=60&v=4){kind=small}
| import { moveFocus } from "./minesweeper-focus.js"; | ||
|
|
||
| describe("moveFocus", () => { | ||
| it("moves horizontally and wraps within the row", () => { |
There was a problem hiding this comment.
Coverage gap: the moveFocus tests only verify wrapping at grid edges (right at x=7, left at x=0, down at y=7, up at y=0) plus home/end. No test covers standard in-grid movement (e.g. right from a center cell), so a regression in the non-wrapping x + 1 / y + 1 paths in minesweeper-focus.ts:17,21,25,29 would go undetected. Add a basic movement assertion per direction.
| "c-minesweeper-demo__mode-button", | ||
| !conditions.flagMode && | ||
| "c-minesweeper-demo__mode-button is-active", | ||
| "c-minesweeper-demo__mode-button is-active", |
There was a problem hiding this comment.
it looks like c-minesweeper-demo__mode-button is duplicated here. pre-existing but would appreciate cleanup to just append is-active for these cases. probably duplicated from a refactor to use cls(). same thing at line 461
sdougbrown
left a comment
sdougbrown
left a comment
There was a problem hiding this comment.
requesting changes to hoist the row indexes etc to module scope 😄
| const rowCells = CELL_ORDER.slice( | ||
| rowIndex * BOARD_WIDTH, | ||
| (rowIndex + 1) * BOARD_WIDTH, | ||
| ); |
There was a problem hiding this comment.
yeah you need to separate into rows for aria semantics ok cool.
not a big deal perf-wise for this size of a grid, but for this exercise let's try to do this differently.
it's kind of pointless repeated work on every render even if not a big perf hit. the board dimensions are constant, so the row grouping is static. it's the same kind of invariant that CELL_ORDER and EMPTY_BOARD already hoist to module scope. would recommend to precompute the rows once at module level and just map:
const CELL_ROWS = Array.from({ length: BOARD_HEIGHT }, (_, rowIndex) =>
CELL_ORDER.slice(rowIndex * BOARD_WIDTH, (rowIndex + 1) * BOARD_WIDTH),
);
// render: CELL_ROWS.map((rowCells, rowIndex) => (...))That keeps the render-body focused on the actual per-cell work and matches the file's own existing pattern. It also trims a bit of the diff (the .slice logic disappears from the JSX).
| className={cls( | ||
| "c-minesweeper-demo__cell", | ||
| !isRevealed && "c-minesweeper-demo__cell is-hidden", | ||
| isRevealed && "c-minesweeper-demo__cell is-revealed", |
There was a problem hiding this comment.
ah ha repeated c-minesweeper-demo__cell here too. cls should concat so we don't need the duplicates haha
2 participants
Closes #146.
Problem
The Minesweeper board exposed all 64 cells in the normal Tab sequence and did
not announce game events to screen-reader users. The supporting controls and
inspector also lacked accessible structure, and the board overflowed on narrow
viewports around 360px.
Approach
narrow viewports.
I used roving tabindex instead of 64 individual tab stops because the board is
one composite grid widget. Tab enters and leaves the grid, while the arrow keys
handle directional navigation within it.
Tradeoffs
Cells shrink below their desktop size on narrow screens so all eight columns
remain visible without horizontal overflow. The board remains functional at the
approximately 360px target from the issue.
Verification
yarn --cwd docs test— 3 tests passedyarn docs:build— passedSpace, and Flag mode