Repository navigation
[3245 by Claude Opus 5.5] Dismiss the toolbar To-do popover with Escape and restore focus - #3246
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…rd focus back on its trigger. That works whether the popover was opened from the trigger or with the native New To-do shortcut (Ctrl+Shift+D). Escape never creates or submits a To-do. Only `QuickAddTodo.tsx` and `QuickAddTodo.test.tsx` changed, and I left PR3244's files alone. I haven't tried it in the installed Linux preview, so the operator's native keyboard retest is still needed. How it behaves (`propr-ui/src/components/QuickAddTodo.tsx`): - Like an outside click: the popover closes and the unsent draft is thrown away, the same as clicking outside it does today. This also applies to the "To-Do added" screen. - Nested controls first: the repository filter already handles its own Escape, and that Escape doesn't also close the popover. If the category list is open, the first Escape closes just that list and returns focus to its toggle; a second Escape closes the popover. - Only Escape meant for it: Escape pressed in an unrelated control elsewhere on the page is ignored. Escape still works when nothing has focus, for example after clicking a blank part of the popover. Escape that ends an IME composition is ignored. - Mobile form unchanged: the inline New To-do form adds no Escape handling, so the Create sheet's existing Escape and focus behavior stays as it was. - Nothing else changed: submission and Ctrl/Cmd+Enter work as before, demo/disabled mode is untouched, and no new permissions are involved. Tests (`QuickAddTodo.test.tsx`, 8 new): they cover opening from the trigger and from the shortcut, Escape closing it with focus back on the trigger, no submission, the draft being cleared, the success screen, the repository filter, the category list, Escape from outside, the IME case, and the inline form inside the real Create sheet. Results: - All 17 tests in the file pass, and the MobileBottomNavigation and GlobalHeader suites still pass (62 tests). - With the original component swapped back in, the 5 tests that expect the popover to close fail. The other three new tests check that something stays open, so they pass either way. - ESLint (`--max-warnings 0`) and `tsc` are clean. I didn't make any preview images, because the change is keyboard behavior with nothing new to see on screen.
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationReady to merge within the visible scope, conditional on pending checks passing. The change adds focused Escape handling without altering submission logic. The supplied issue text is truncated, so requirements beyond the visible portion could not be assessed. This review used static analysis only; no commands or tests were run. ✅ Correct dismissal and focus — Escape closes the toolbar popover, applies the existing draft reset, and focuses its actual trigger without calling ✅ Nested dismissal respected — The handler honors prevented events, ignores IME composition, and closes the category list before the outer popover. ✅ Relevant regression coverage — Added tests cover trigger and external-request opening, focus restoration, draft clearing, nested controls, and delegation to the inline form’s containing sheet. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Verify installed Linux behaviorRepeat the original reproduction in the installed Linux preview: open with Ctrl+Shift+D, press Escape, and verify dismissal, trigger focus, and no submission. The new component test simulates the parent’s external-open request rather than exercising native keyboard dispatch. This is useful end-to-end confirmation, not a demonstrated code blocker. ScoreThe implementation is localized and consistent with the visible requirements, with good targeted coverage and no verified regression. The supplied current-head status reports no failures and six pending checks; installed native behavior remains unverified. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
integry
added
the
desktop-ci
1 participant
AI Implementation Summary
Closes #3245
Branch:
3245/claude-opus-5-5-dismiss-the-toolbar-to-do-20261018-1932-n4cCommits: ✅ Changes committed (bd76ce3)
AI Processing Completed
Execution Details:
Summary:
Escape now closes the toolbar Quick add to-do popover and puts keyboard focus back on its trigger. That works whether the popover was opened from the trigger or with the native New To-do shortcut (Ctrl+Shift+D). Escape never creates or submits a To-do. Only
QuickAddTodo.tsxandQuickAddTodo.test.tsxchanged, and I left PR3244's files alone. I haven't tried it in the installed Linux preview, so the operator's native keyboard retest is still needed.How it behaves (
propr-ui/src/components/QuickAddTodo.tsx):Tests (
QuickAddTodo.test.tsx, 8 new): they cover opening from the trigger and from the shortcut, Escape closing it with focus back on the trigger, no submission, the draft being cleared, the success screen, the repository filter, the category list, Escape from outside, the IME case, and the inline form inside the real Create sheet.Results:
--max-warnings 0) andtscare clean.I didn't make any preview images, because the change is keyboard behavior with nothing new to see on screen.
Detailed Logs:
2548647c-de55-4894-b248-0399a5272bb4Log files stored at:
/usr/src/app/logs/executions/issue-3245-2026-10-18T19-37-27-921Z-conversation.json/usr/src/app/logs/executions/issue-3245-2026-10-18T19-37-27-921Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #3245.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!