Add keyboard shortcuts: 'a' to accept, 'd' to dismiss threads#52
Add keyboard shortcuts: 'a' to accept, 'd' to dismiss threads#52HamptonMakes merged 2 commits intomainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5631cf349
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this.currentIndex >= highlights.length) { | ||
| this.currentIndex = 0 | ||
| } | ||
| this.navigateTo(highlights[this.currentIndex]) |
There was a problem hiding this comment.
If a user opens a thread popover via mouse click (without j/k), currentIndex stays -1. After pressing a or d, the mutation callback runs advanceAfterAction, skips the >= highlights.length guard, and calls navigateTo(highlights[-1]), which is undefined and causes a runtime error in navigateTo. This breaks shortcut flow whenever at least one open highlight remains after the action (e.g., any accept, or discard with other open threads).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — this is a real bug. When the popover is opened via mouse click, currentIndex stays at -1, and highlights[-1] is undefined. Fixed in 27bd0a4: currentIndex is now normalized to 0 before dispatching the action.
| if (this.currentIndex >= highlights.length) { | ||
| this.currentIndex = 0 | ||
| } | ||
| this.navigateTo(highlights[this.currentIndex]) |
There was a problem hiding this comment.
advanceAfterAction re-navigates to highlights[this.currentIndex] without incrementing the index first. For accept (pending -> todo), the thread remains open, so it stays in openHighlights and focus returns to the same thread instead of advancing. This contradicts the intended auto-advance behavior and forces an extra j press after every accept.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct — accept changes pending→todo, which is still an open status, so the thread stays in openHighlights at the same index. Fixed in 27bd0a4: advanceAfterAction now takes a shouldAdvance flag. For accept, it increments the index; for discard, the thread drops out of the list so the current index naturally points to the next one.
When navigating threads with j/k, pressing 'a' accepts a pending thread (pending → todo) and 'd' discards it (pending → discarded). Uses the same input guard pattern — skips when in inputs/textareas or when modifier keys are held. Implementation: - Finds the open popover and submits the accept/discard form via form.requestSubmit() so Turbo intercepts properly - Uses data-action-name attributes on forms for robust selection - MutationObserver on #plan-threads detects broadcast DOM updates before auto-advancing to the next thread - Refactored findOpenPopover() as shared helper (was duplicated in focusReply) Amp-Thread-ID: https://ampcode.com/threads/T-019d1c41-53b2-73c9-9b51-d9f0e53de461 Co-authored-by: Amp <amp@ampcode.com>
- Normalize currentIndex to 0 when popover was opened via mouse click (not j/k navigation), preventing highlights[-1] → undefined error - For accept (pending→todo), explicitly advance index since the thread stays in openHighlights; for discard, the thread leaves the list so the current index naturally points to the next one Amp-Thread-ID: https://ampcode.com/threads/T-019d1c41-53b2-73c9-9b51-d9f0e53de461 Co-authored-by: Amp <amp@ampcode.com>
HamptonMakes
force-pushed
the
feat/keyboard-accept-dismiss
branch
from
c6650fa to
6376262
Compare
1 participant
When navigating threads with j/k, pressing 'a' accepts a pending thread (pending → todo) and 'd' discards it (pending → discarded).
Implementation:
data-action-nameattributes on forms for robust DOM selectionform.requestSubmit()so Turbo intercepts properly#plan-threadsdetects broadcast DOM updates before auto-advancingfindOpenPopover()as shared helper (was duplicated infocusReply)Tests: 3 new system tests — accept, discard, and textarea input guard.