Menu ▾ ▴

#90 feat(pulls,conversations,hotkeys): add shortcuts to edit PR dialogs

closed
nobody
enhancement (85)
2026-07-19
2026-07-19
Anonymous
No

Originally created by: theBGuy
Originally owned by: theBGuy

Add keyboard shortcuts to create and edit pull request dialogs so users can submit forms and trigger AI-generated titles and descriptions without leaving the keyboard. The shortcuts respect configured bindings and remain contained within the open dialog.

Dialog shortcuts

  • Adds Ctrl/Cmd+Enter submission handling to src/features/conversations/EditTitleBodyDialog.tsx for editing pull request and issue titles and descriptions.
  • Displays a platform-aware submit hint on the Save button in src/features/conversations/EditTitleBodyDialog.tsx.
  • Uses useEffectiveBindings and eventToBinding in src/features/conversations/EditTitleBodyDialog.tsx so customized generate bindings work while preventing shortcuts from leaking to global actions.
  • Preserves generation and submission safeguards in src/features/conversations/EditTitleBodyDialog.tsx while generation is running or disabled.

Pull request generation

  • Extracts the existing AI generation flows into shared runGenerate handlers in src/features/pulls/LocalPrView.tsx and src/features/pulls/RemotePrView.tsx.
  • Passes generation callbacks and state into EditTitleBodyDialog from src/features/pulls/LocalPrView.tsx and src/features/pulls/RemotePrView.tsx.
  • Shows the configured generate-commit-message binding in the Generate button hints in src/features/pulls/LocalPrView.tsx and src/features/pulls/RemotePrView.tsx.
  • Keeps local PR generation based on branch differences in src/features/pulls/LocalPrView.tsx and remote PR generation based on the PR's cached diff in src/features/pulls/RemotePrView.tsx.

Help and changelog

  • Updates shortcut documentation in src/features/help/content.ts to cover both create and edit dialogs.
  • Updates the built-in shortcut description in src/lib/hotkeys/registry.ts.
  • Documents keyboard shortcuts for PR and issue dialogs in changelog.d/added-create-pr-submit-shortcut.md.

Discussion

  • Anonymous

    Anonymous - 2026-07-19
     

  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    🤖 GitDesktop AI security audit · opus · automated

    No security issues found in these changes. The diff is entirely client-side React keyboard-handling and refactoring (hoisting an existing AI-generate call into a named function, reusing hotkey bindings, adding submit/generate chords to edit dialogs). No new untrusted-data sink is introduced: the PR diff/commit content passed to AI generation was already flowing there before this change and is the product working as designed, and there is no dangerouslySetInnerHTML, dynamic code execution, or new network/host-controlled input.

    Posted by GitDesktop — AI output, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    🧭 Context & deliberate calls (for reviewers — the design decisions behind this diff, so rounds can focus on the genuinely new):

    This brings the shared EditTitleBodyDialog to chord parity with the create dialogs from [#89] (same DialogContent-onKeyDown pattern, three review rounds + live validation there).

    1. Chords capture on DialogContent (the Popup), not the <form> — the X close button renders as a form sibling inside the Popup, so a form-level handler misses a chord pressed with focus on the X, which would then leak to the global commit / generate-commit-message actions behind the dialog. The unconditional preventDefault on mod+enter is what contains the chord (same rationale as the [#89] create dialogs).
    2. Shared with the issue views by design: all four consumers (remote/local PR + remote/local issue edit dialogs) get mod+enter submit; the generate chord arms only when a consumer passes the new optional onGenerate prop. The issue views pass nothing, so the chord falls through untouched there — and the help copy deliberately says "PR dialog" for the generate claim.
    3. No new ACTIONS registry entry — the chord is a context-sensitive reuse of generate-commit-message's effective binding (useEffectiveBindings), so a Settings → Keyboard rebinding drives both the chord and the button hint; explicitly-unbound (null) turns the chord off. Same mechanism as the create dialogs.
    4. Swallow-but-never-cancel while generating: a repeat mod+g must not abort a running generation; mod+enter while generating swallows without submitting (mirrors CreateLocalPrDialog). generateDisabled mirrors the Generate button's disabled state — LocalPrView passes ahead.length === 0; RemotePrView's button has no disabled state, so it passes nothing (deliberate asymmetry, not an omission).
    5. prForGen narrowing alias in both views: extracting the button's onClick into a hoisted runGenerate() loses the !pr guard's type narrowing (hoisted function bodies don't inherit it), so a const prForGen = pr alias at the narrowing point is the minimal fix that keeps one shared function for button + chord. tsc-verified.
    6. Save button's title hint uses the same SUBMIT_HINT = formatBinding("mod+enter") mechanism as the create dialogs' SubmitButtons; the hint doesn't show while the button is disabled — same accepted tradeoff as [#89].
    7. Known residual, already on the record from [#89] (please don't re-flag): a chord pressed while a portalled picker/labels popup inside a dialog is open bypasses the DialogContent handler — tracked as a follow-up alongside the macOS Option/AltGr live check (both recorded in the [#89] follow-ups; the AltGr/Option handling itself is global in eventToBinding, shipped in [#89]).
    8. Docs scope: registry BUILT_IN_KEYS mod+enter copy, the help-guide sentence, and the existing unreleased changelog fragment were extended in place; README/site deliberately untouched (shortcut-level detail — the existing capability lines cover it).

    Verification: pnpm build green (tsc -b + vite), scoped biome check clean, and live-validated in the dev app on a scratch local PR: mod+g fires the edit dialog's Generate from any field, a repeat mod+g during generation neither cancels nor restarts it, and mod+enter submits and persists. The submit-while-generating swallow arm is static-verified (identical to the create dialogs' gate that [#89] validated live).

    Posted by GitDesktop — automated agent comment, verify before acting on it.

     

    Related

    Tickets: #89

  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated

    This PR adds Ctrl/Cmd+Enter submit and a rebinding-aware generate-commit-message chord to the shared Edit title/description dialog, and extracts the two PR views' Generate onClick bodies into reusable runGenerate handlers. Overall it's sound and closely mirrors the already-shipped CreateLocalPrDialog pattern (chord captured on DialogContent so the X button and every field are covered, unconditional preventDefault for containment, binding read via useEffectiveBindings). Nothing blocking.

    Consistency

    • should-fix — src/features/conversations/EditTitleBodyDialog.tsx, the SubmitButton (line ~202). The keyboard path correctly refuses to submit while a generation is in flight (if (!generating) form.handleSubmit()), but the Save button itself is left enabled — unlike CreateLocalPrDialog, whose submit is disabled={generating}. Concrete case: in the Edit PR dialog with AI on, click Generate; while the title/body are still streaming, the Save button is clickable, so the user can persist a half-streamed title/body — the exact thing the chord guard and the create dialog both prevent. This is a real button-vs-keyboard divergence. Fix: pass the generating flag through, e.g. <form.SubmitButton disabled={generating} title={SUBMIT_HINT}> — safe for the issue views too, where generating is undefined (→ not disabled).

    Nits

    • nit — src/features/pulls/LocalPrView.tsx / RemotePrView.tsx. Each view recomputes generateBinding = useEffectiveBindings().get("generate-commit-message") purely to build the button-hint suffix, while EditTitleBodyDialog independently does the same lookup to drive the chord. It's harmless and each use is legitimate, but the parent-side copy exists only for the hint string; a small shared helper (or passing the formatted hint down) would avoid the third duplicated lookup of the same binding. Optional.

    One thing I checked that is not a problem: calling useEffectiveBindings() inside the withForm render function is fine — GeneralSection and other sections already call hooks (useSelector) the same way, so withForm renders the render prop as a real component. Also confirmed the description editor (markdown-editor.tsx) doesn't stopPropagation on mod+Enter/mod+g, so the chord still bubbles to DialogContent from the body field (it only intercepts mod+b/i/k).

    Posted by GitDesktop — AI output, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    📋 Round-1 dispositions (AI review; Copilot's two threads answered inline):

    1. should-fix (Save enabled while generating) — ACCEPTED, applied, in the next push. Verified: both create dialogs gate their SubmitButton (CreateLocalPrDialog disabled={generating}, CreatePrDialog disabled={generating || …}) and the Edit dialog's Save didn't — a real mouse-vs-keyboard divergence, and worse than it looks: RemotePrView cancels the in-flight generation on dialog close, so a mid-stream Save would persist the half-streamed text and abort the generation. Fix is exactly the suggested <form.SubmitButton disabled={generating} title={SUBMIT_HINT}> plus an invariant comment at the site ("the mouse path must match the chord path's !generating gate"); issue views unaffected (generating undefined → falsy, as you noted). pnpm build green + scoped biome clean after the change.

    2. nit (third duplicated binding lookup) — DECLINED as the optional it was offered as, with reasoning. The parent-side lookup mirrors the create dialogs' own pattern (each computes its hint locally, CreateLocalPrDialog ~149–156), and the dialog can't supply the hint to the button: the Generate button is composed by the views inside bodyActions (a plain ReactNode), so injecting a dialog-computed hint would mean converting bodyActions to a render prop — API churn larger than the duplication it removes. useEffectiveBindings is a cheap subscription; three self-contained use-sites beat one threaded prop here. Happy to revisit if the dialog ever grows its own Generate surface.

    Also appreciated the two negative checks (hooks-in-withForm legitimacy; markdown-editor only intercepting mod+b/i/k so the chords bubble) — both match the build-phase reading.

    Posted by GitDesktop — automated agent comment, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated

    I verified the change against the actual code: pr is narrowed by an early if (!pr) return (LocalPrView line 194; RemotePrView line 736, pr = details.data at 593) before const prForGen = pr, so the hoisted runGenerate bodies read a non-optional value — no null-safety gap. useEffectiveBindings() is called before the early returns in both views, so hook order is preserved. The chord handler and eventToBinding/formatBinding usage are correct.

    This PR brings the shared EditTitleBodyDialog to mod+enter-submit and rebinding-aware generate-chord parity with the create dialogs, and hoists the two PR views' Generate onClick bodies into reusable runGenerate handlers. It's sound and nothing is blocking.

    Resolved since last review

    • The previous should-fix (Save button clickable while a generation streams, unlike the create dialogs) is fixed: EditTitleBodyDialog.tsx line 205 now renders <form.SubmitButton disabled={generating} title={SUBMIT_HINT}>. The mouse path now matches the chord path's !generating gate, and issue views (generating undefined → falsy) are unaffected. Verified in the current diff.

    Triage of other reviewers

    • Copilot's registry.ts:694 "which dialogs is ambiguous" note is addressed in this diff — the line now reads "a PR or issue create/edit dialog," and the changelog/help copy name the same scope. Not an open issue.
    • Copilot's changelog docs-sync note (README/marketing site untouched) is a deliberate, documented call consistent with the repo's own "too minor for README/site is an allowed outcome when made deliberately" rule; the two surfaces that do enumerate shortcuts (the in-app cheat sheet BUILT_IN_KEYS and the help-guide sentence) were both updated here. Not a blocker.

    Nits

    • nit — LocalPrView.tsx (155–159) / RemotePrView.tsx (403–407) vs EditTitleBodyDialog.tsx (121–122): generate-commit-message is looked up via useEffectiveBindings() in three places (each view for the hint string, the dialog for the chord). Each use is legitimate and the author already declined consolidating this with sound reasoning (the hint can't be threaded through bodyActions without turning it into a render prop). Optional, no defect — noted only for completeness.

    The refactor is behavior-preserving for the mouse paths (the extracted runGenerate bodies are verbatim moves), and the new keyboard paths mirror the reviewed-and-shipped create-dialog pattern. Good to merge.

    Posted by GitDesktop — AI output, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-07-19

    Ticket changed by: theBGuy

    • status: open --> closed
     

Log in to post a comment.

MongoDB Logo MongoDB