Repository navigation
fix: Escape and hotkeys in test details, merge branch encoding, failing comment save - #436
Merged
Merged
Conversation
The Escape hotkey listened on the document, but the MUI Dialog around the details handles Escape itself and stops it, so a real key press never got there. The Dialog's onClose now closes on escapeKeyDown, through the same path as the close button, so unsaved ignore areas still ask first. Escape typed into a text field, such as the comment, stays with the field, as the other hotkeys do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… switch The hotkey library skips keys pressed in any form field, and the "Toggle diff" switch is a checkbox that keeps focus after a click, so D, the arrows, A and X did nothing until focus moved elsewhere. The dialog's hotkeys now take keys on form fields and leave out only the fields being typed into, which a checkbox is not. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
testVariationService.merge put both branch names into the query as they were, so a "/" went raw and an "&" or "#" cut the name short and added a stray parameter. They are now encoded, as builds.getList does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CommentsPopper chained no handler for a failing save, so a rejection went unhandled, and the details dialog caught its own failure and resolved, so the box closed and the draft was gone from view while the error showed. The dialog now rethrows after its error snackbar and the popper keeps the box open with the draft. The e2e test for this passed only while the box was still fading out; it now checks after the fade. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SonarCloud flagged the 500 ms wait (S2925). A box that closes starts to fade in the same update that shows the error, so its Paper's inline opacity tells an open box from a closing one at once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The details dialog's hotkeys now act on a focused checkbox, and the review over it is full of them. Its own window listener already holds a, x, d and the arrows back; this pins that, so the run behind cannot be approved, rejected or stepped from inside the review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since Escape closes the details dialog, a press with focus on Comments, Save or the editor went past the box and closed the whole dialog, so an unsaved comment dropped out of view. The box now takes Escape while it is open: it closes, keeps the draft, and the key goes no further. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The opacity check ran right after the error showed, so in principle it could pass before a closing box had started to fade. It now waits two animation frames first and checks Save is still there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pashidlos
force-pushed
the
claude/frosty-yalow-08cb06
branch
from
October 24, 2026 12:13
a2a2839 to
cf0f59f
Compare
The test for a branch name with / and & repeated the setup of the merge test line for line, which SonarCloud counted as 5.7% duplicated new code against a 3% gate. Both are now one test run over a table of branch names, so the special name also gets the notice and redirect checked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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.
While writing the tests in #434, we found four bugs and pinned the current behaviour with
// NOTE:comments. This PR fixes them. Each fix starts by changing the pinned test to the expected behaviour and seeing it fail. The branch is rebased onto master after #434's squash merge.useHotkeys("ESC")never saw it. The Dialog'sonClosenow acts onescapeKeyDown, using the same path as the close button, so unsaved ignore areas still ask first. Escape pressed in a text field stays with that field;_helpers/hotkeys.helper.ts#isTypingTargetchecks for this.hotkeyOptions: they are enabled on form fields and ignored only in fields being typed into. A checkbox only takes Space, so it does not count.testVariationService.mergeencodes branch names withencodeURIComponent, likebuilds.getList. There is a new e2e test withrelease/1.0&hotfix.CommentsPopperhad no rejection handler, and the dialog caught its own failure and resolved, so the box closed while the error showed. The dialog now rethrows after its error snackbar, and the popper keeps the box open with the draft. The e2e test now checks two animation frames after the error, instead of while the box was still fading out.Follow-ups from review:
MatchingVariationsDialogalready holds back a, x, d and the arrows with its own window listener. A new e2e test pins that with a checkbox in the review focused.Also: the coverage floors in
vitest.config.tsare raised to the current whole-number levels.Checks (after the rebase onto master)
npm run lint: 0 errors.npm run typecheck: passes.npm test: 562 tests pass.npm run build: passes.npm run test:pw:docker: 297/297 pass. No screenshot baselines changed.🤖 Generated with Claude Code