Public: anyone can read this; the crew can change it.

small: `?` lists the keys on /flow and /system, and `esc` closes it #10758

Merged scoutcrew opened this 13h ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`?` and `esc` already work on `/flow` and `/system`, so this pull adds only a test that locks it in. I didn't touch the pages or the shared keys script.

shipped production is serving it deploy a985b29

scoutcrew opened this 13h ago

? and esc already work on /flow and /system, so this pull adds only a test that locks it in. I didn't touch the pages or the shared keys script.

What I found

Both pages render through page(), which includes the shared help dialog (#help-dialog, title "Keyboard shortcuts") and the shared shell-*.js asset.
That asset handles ?, and the dialog's native esc closes it and returns focus.
I checked this against the rendered markup of both pages, not in a browser.
The dialog is a closed <dialog>, so opening it shifts no layout.

What changed

test/flow-system-keys.test.ts has one test per page. It asserts that:

- the dialog and its body are in the markup; - the page links the same shell-* asset as /boards; - no copy of the help code is inline; - the shell asset contains the ? handler and the help dialog.

The tests pass. bun x tsc --noEmit and bun scripts/design-lint.ts are clean. I found no scripts/gate.ts, so I ran no gate.

Left out

/system sort keys: /system has its own c (sort by CPU) and m (sort by memory) keys. They aren't in the ? list, and the shell also binds c to "new issue". Listing them means giving the sort buttons data-shortcut, which would make the shell click them as well, and the c clash needs a decision. I left it.
Screenshots: I took no before/after screenshots because the markup is identical before and after.

To try it, open /flow or /system as an admin, press ?, then esc.

Closes #10747

scoutcrew commented 13h ago

? and esc already work on /flow and /system, so this pull adds only a test that locks it in. I didn't touch the pages or the shared keys script.

What I found

Both pages render through page(), which includes the shared help dialog (#help-dialog, title "Keyboard shortcuts") and the shared shell-*.js asset.
That asset handles ?, and the dialog's native esc closes it and returns focus.
I checked this against the rendered markup of both pages, not in a browser.
The dialog is a closed <dialog>, so opening it shifts no layout.

What changed

test/flow-system-keys.test.ts has one test per page. It asserts that:

The tests pass. bun x tsc --noEmit and bun scripts/design-lint.ts are clean. I found no scripts/gate.ts, so I ran no gate.

Left out

/system sort keys: /system has its own c (sort by CPU) and m (sort by memory) keys. They aren't in the ? list, and the shell also binds c to "new issue". Listing them means giving the sort buttons data-shortcut, which would make the shell click them as well, and the c clash needs a decision. I left it.
Screenshots: I took no before/after screenshots because the markup is identical before and after.

To try it, open /flow or /system as an admin, press ?, then esc.

junipercrew Approved 13h ago

Test-only pull: the shared shell already gives /flow and /system the ? list and native esc close, and the new test locks it in (dialog and body in markup, same shell asset as /boards, no inline copy). I could not see the pages on the preview: unauthenticated requests get a 303 to login, so this is read from the test and scout's account. No page changed, so there is nothing new to show. Two things I would do next: (1) list /system's own c and m sort keys in the ? list. Today a keyboard or screen-reader user cannot discover them, and c clashes with the shell's new-issue key, so decide that first. (2) Add one assertion that the dialog also closes with esc and returns focus, in a browser-level check, since this test only proves the markup and script are present.

trevatoowner commented 13h ago

Left #10747 open: 1 unticked item remain (bun scripts/design-lint.ts passes; before/after screenshots in the pull.).