[yunhsuan0510]

Allow users to switch between pen, eraser, and text tools using keyboard shortcuts (P, E, and T), reducing reliance on toolbar clicks and improving interaction efficiency. Repeated P presses cycle through pen colors, while repeated E presses cycle through eraser sizes. Pressing T toggles between the text tool and the previously selected pen or eraser.

Add always-visible shortcut labels beside the corresponding toolbar controls to improve discoverability. To prevent unintended tool changes, ignore shortcuts while typing in text inputs or editing text labels, on repeated keydown events, and when the whiteboard is inactive.

Close #618

---

---

Summary by cubic

Adds whiteboard keyboard shortcuts so users can switch tools without the toolbar: P selects the pen (repeated presses cycle pen colors), E selects the eraser (repeated presses cycle eraser sizes), and T toggles between the text tool and the previously selected pen or eraser.

• Shortcuts fire only on unmodified keys and are ignored while typing in text inputs, on held-key repeats, when the board is locked or mid-draw, and when focus is outside the whiteboard; they also work when focus is on the toolbar.
• Adds always-visible P/E/T labels next to the corresponding toolbar buttons plus aria-keyshortcuts attributes so the shortcuts are discoverable.

^{Written for commit 92c9361. Summary will update on new commits.}

[jserv]

Add screenshots to the pull request description to illustrate your work, as @ColtenOuO suggests. Example: #349

[cubic-dev-ai]

All reported issues were addressed across 2 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.

View guided diff | Re-trigger cubic</sub>

[jserv]

aria-keyshortcuts on the role="group" div is never announced: the group is not focusable, and focus lands on the swatch buttons built in initWhiteboard. Set it on each swatch there (swatch.setAttribute("aria-keyshortcuts", "P"), with a title like Red pen (P)), the way the undo/redo loop does for its buttons. The visible label then only repeats that for sighted users, so hide it from assistive tech:

Suggested change

	></div>
	<kbd class="board-shortcut-key">P</kbd>
	></div>
	<kbd class="board-shortcut-key" aria-hidden="true">P</kbd>

[jserv]

The <kbd> is a separate item in .board-toolbar, which has flex-wrap: wrap, so on a narrow board the E label can wrap onto the next row, away from the Eraser button it names. A screen reader also reads it as a stray "E" after the button. Moving it inside the button with aria-hidden="true" keeps it next to its control. The accessible name stays Eraser, so the getByRole lookups in the test still match. Give .board-shortcut-key a small margin-left for the gap.

Suggested change

	Eraser
	</button>
	<kbd class="board-shortcut-key">E</kbd>
	Eraser <kbd class="board-shortcut-key" aria-hidden="true">E</kbd>
	</button>

[jserv]

Same as Eraser: keep the label inside the button so it cannot wrap away from it.

Suggested change

	Text
	</button>
	<kbd class="board-shortcut-key">T</kbd>
	Text <kbd class="board-shortcut-key" aria-hidden="true">T</kbd>
	</button>

[jserv]

shortcut repeats the key computation further down but skips its guard. So a non-printable event.key such as Dead or Unidentified is mapped through event.code here, and bare P/E/T follow a looser rule than the Z/Y chords. Compute key once, right after the target check, use it in both branches, and the three preventDefault(); return; tails collapse into one:

    const printable = /^[\x20-\x7e]$/.test(event.key);
    if (!printable && !/^[^\x00-\x7f]$/u.test(event.key)) return;
    const key = printable
      ? event.key.toLowerCase()
      : event.code.replace(/^Key/, "").toLowerCase();
    if (!event.shiftKey && !event.ctrlKey && !event.altKey && !event.metaKey) {
      if (boardLocked() || board.model.isDrawing()) return;
      if (key === "p") {
        const at = PEN_COLORS.findIndex(({ color }) => color === board.color);
        selectPen(
          board.tool === "pen"
            ? PEN_COLORS[(at + 1) % PEN_COLORS.length].color
            : board.color,
        );
      } else if (key === "e") {
        const at = ERASER_WIDTHS.indexOf(board.eraserWidth);
        if (board.tool === "eraser")
          selectEraserWidth(ERASER_WIDTHS[(at + 1) % ERASER_WIDTHS.length]);
        else selectTool("eraser");
      } else if (key === "t") {
        selectTool(board.tool === "text" ? board.previousNonTextTool : "text");
      } else {
        return;
      }
      event.preventDefault();
      return;
    }

Then drop the second printable/key block after the modifier check.

[jserv]

The keydown listener is on #board-panel, and a button appended to document.body is outside it, so this assertion would still pass with the target check deleted. The case the description promises and the test never covers is a locked board. Do it the way the Cmd/Ctrl test does: boardTest.lock("ended"), focus #board, press p, e and t, then assert that Text is still pressed and globalThis.keyTaken is false.

[jserv]

Some review comments on this pull request are not settled yet. Please refine the change per the review comments listed below, reply on each thread with what you changed or why you did not, and mark each thread resolved yourself once it is settled (Resolve conversation, at the bottom of the thread). The pull request stays blocked while review threads are open.

Not addressed at the current head:

• Add whiteboard keyboard shortcuts #622 (comment) (web/interview.html)

Addressed at the current head; reply with what changed and resolve:

• Add whiteboard keyboard shortcuts #622 (comment) (web/interview.js)
• Add whiteboard keyboard shortcuts #622 (comment) (web/interview.js)

Checked at 373b512.

[jserv]

Issue #618 asks for exactly this: P, E and T tool shortcuts on the whiteboard toolbar with visible key labels. This branch delivers it, but the description says Close #619, which is a closed pull request from another contributor, not the issue, so merging would leave #618 open. End the description with Close #618 and drop the Close #619 line.

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="README.md">

<violation number="1" location="README.md:243">
P3: This implies a second P press is needed to cycle, but P cycles immediately when the pen is already selected (the initial tool). Say to press P while the pen is selected; the Eraser row has the same issue for E.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

P3: This implies a second P press is needed to cycle, but P cycles immediately when the pen is already selected (the initial tool). Say to press P while the pen is selected; the Eraser row has the same issue for E.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At README.md, line 243:

<comment>This implies a second P press is needed to cycle, but P cycles immediately when the pen is already selected (the initial tool). Say to press P while the pen is selected; the Eraser row has the same issue for E.</comment>

<file context>
@@ -240,9 +240,9 @@ what you can draw, say aloud and defend by tracing an example by hand.
-   | Pen colors | Black, red, blue and green. Write in black and mark up in the others. |
-   | Eraser | Rubs out what it passes over. The three size buttons beside it pick a size and the eraser together: small for one character, medium for one line, large for a region. Pick a pen color, or press Eraser again, to draw again. |
-   | Text | Click or drag on the board to create a text draft. Enter inserts a newline; Ctrl+Enter or Confirm places the label; Escape or Cancel discards it. Committed labels cannot be edited, moved or resized. |
+   | Pen colors | Black, red, blue and green. Write in black and mark up in the others. With focus on the canvas or drawing toolbar, press P to select the pen; press P again to cycle through colors. |
+   | Eraser | Rubs out what it passes over. The three size buttons beside it pick a size and the eraser together: small for one character, medium for one line, large for a region. With focus on the canvas or drawing toolbar, press E to select the eraser; press E again to cycle through sizes. Pick a pen color, or press Eraser again, to draw again. |
+   | Text | With focus on the canvas or drawing toolbar, press T to toggle the text tool and the previously selected pen or eraser. Click or drag on the board to create a text draft. Enter inserts a newline; Ctrl+Enter or Confirm places the label; Escape or Cancel discards it. Committed labels cannot be edited, moved or resized. |
</file context>

[cubic-dev-ai]

All reported issues were addressed across 5 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[jserv]

The P badge sits inside .board-pens, whose background is --paper white, so it renders --sub (#b7b2a5) on #ffffff, about 2.1:1, while the E and T badges on the dark buttons are fine. Give the badge in the pen strip a dark ink that reaches 4.5:1 on white, for example .board-pens .board-shortcut-key { color: var(--surface); }.

[yunhsuan0510]

Added .board-pens .board-shortcut-key { color: var(--surface); } so the P badge has sufficient contrast on the white pen strip. The E/T badges retain their existing colors on the dark buttons.

[jserv]

This kbd is now the first child of #board-pens, and paintToolState in web/interview.js walks nodes.boardPens.children as if every child were a swatch, so each repaint writes aria-pressed="false" onto the badge. Iterate nodes.boardPens.querySelectorAll(".board-color") there instead, so only the pen buttons get a pressed state.