Repository navigation
fix(shell): honour reduced motion, announce answers, and test the resize geometry - #92
Merged
Merged
Conversation
…ize geometry (#91) The remaining items from the second critique run that did not need a product decision. **Renderer coverage for the resize arithmetic.** `ResizableLayout`'s geometry was inline, so the only way to exercise it was to drag a mouse — and it was wrong in a way nothing could catch: `ArrowLeft` grew the left panel and shrank the right one, so the two seams told a screen-reader user the opposite of what they had pressed. Typecheck passed. Lint passed. The second critique run found it by reading. Everything pure moved to `components/layouts/panelGeometry.ts`: the constraint math, the golden-ratio split, the stored-layout parser, and the key-to-width mapping. `test/panelGeometry.test.ts` covers all of it — including the arrow direction on both seams, the `Shift` step, `Home`/`End`, the clamps, and the parser against absent, corrupt, `NaN`, `Infinity` and zero inputs. I checked that these tests actually fail on the buggy version before keeping them: reintroducing the inverted flag turns three of them red. One thing that did *not* catch it, and is worth knowing: the invariant "the two seams move oppositely" holds when both are flipped together, so only assertions on the absolute direction detect an inversion. **The guard's allowlist predicate is tested too**, because CI found that bug and a local run could not. `isChartSurfaceAllowed` is exported (with an entrypoint guard so importing does not run the scan) and `test/designGuard.test.ts` asserts both separator styles — reintroducing the forward-slash-only version turns two tests red. The same file also asserts `--list` still names every rule, so one cannot be deleted silently, and that a plain run stays green. **`prefers-reduced-motion` is honoured.** The app pulses rows and cursors while work is in flight and animates every popover, and none of it was conditional on the setting. Transitions and decorative animation now collapse and scroll behaviour goes instant; spinners keep turning, slowed rather than stopped, because a spinner is a status indicator and removing it removes information rather than motion. The `!important` in that block is load-bearing and says so in a comment: a `*` selector cannot outrank Tailwind's utility classes. **The transcript announces completion, never tokens.** It previously announced nothing at all: an answer streamed into a plain div with no `aria-live`, so the product's entire output arrived silently. A `role="status"` region now reports when a turn ends, with the node keyed so an identical message re-announces. A live region on the transcript itself would chatter on every token — worse than the silence it replaces. **The tab close control is a sibling of its tab, not a child.** It was a `role="button" tabIndex={0}` span nested inside a `role="tab"` (itself a `<button>`), with its own Enter/Space handling duplicating the trigger's. Because the tablist uses a roving tabindex, that inner `tabIndex={0}` also added a second focus stop per open notebook. It is now a real `<button>` with an accessible name, positioned beside the trigger rather than inside it. (A focusable control inside `role="tablist"` is still not ideal — the fully correct structure needs the tab strip rebuilt so the close lives outside the list, which is a visual change I could not verify here.) DESIGN.md records the geometry module and what the tests do and do not catch, the motion and announcement rules, the tab-strip structure, and the guard's own test.
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
1 participant
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.
What does this PR do?
Lands the content of #91 on
main.#91was a stacked PR based on#90's branch. I merged it expecting it to reachmain, but a stacked PR merges into its own base, so its squash landed onfix/restore-panel-gutterinstead.maingot#90and not#91.This is that content, cherry-picked onto a clean
main-based branch: the identical 12 files, nothing else. No new work, no review needed beyond confirming the diff is what #91 reviewed.What it contains (from #91)
ResizableLayoutintocomponents/layouts/panelGeometry.ts, andtest/panelGeometry.test.tscovers the constraint math, the golden-ratio split, the stored-layout parser (absent, corrupt,NaN,Infinity, zero) and the arrow-key direction on both seams — the bug that passed typecheck and lint and was only found by a second reviewer. Both new suites were checked against a deliberately reintroduced version of the bug they cover and go red.test/designGuard.test.ts, which pins the allowlist predicate on POSIX and Windows paths — the bug that failed the Windows job while linux and macOS passed.prefers-reduced-motionintheme.css, with spinners slowed rather than stopped so status information survives.aria-liveon the transcript, keyed so an identical message re-announces.<button>with an accessible name, instead of arole="button"span nested in arole="tab"that added a focus stop per open notebook.Verification
Same as #91:
npm test73/73,typecheck,lint(0 errors),check:design,build,prettier --checkall clean,impeccable detect src→[]. CI runs on this PR too.