From 4f41abde25adb196550dc25176f1b17944bef933 Mon Sep 17 00:00:00 2001 From: Gordon Beeming Date: Sat, 11 Jul 2026 01:23:51 +1000 Subject: [PATCH 1/4] Explain why Cmd+Enter did not commit instead of doing nothing silently --- src/App.test.tsx | 31 +++++++++++++++++++++++++++++++ src/App.tsx | 26 +++++++++++++++++++++++++- src/styles.css | 27 ++++++++++++++------------- 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/App.test.tsx b/src/App.test.tsx index 040f758..5665beb 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -4687,6 +4687,37 @@ describe("Git commit sidebar", () => { expect(commitButton).toBeDisabled(); }); + it("explains why Cmd+Enter did nothing and clears the hint once the user acts", async () => { + render(); + + expect(await treeButton("README.md")).toBeInTheDocument(); + fireEvent.click(screen.getByTitle("Source control")); + + const panel = await screen.findByLabelText("Git commit panel"); + await waitFor(() => expect(within(panel).getByText("2 / 2")).toBeInTheDocument()); + const message = within(panel).getByPlaceholderText("Commit message"); + + fireEvent.click(within(panel).getByLabelText("Deselect all changes")); + fireEvent.change(message, { target: { value: "Update readme" } }); + fireEvent.keyDown(message, { key: "Enter", metaKey: true }); + expect( + await within(panel).findByText("Select at least one file to commit."), + ).toBeInTheDocument(); + + fireEvent.click(within(panel).getByLabelText("Select all changes")); + await waitFor(() => + expect( + within(panel).queryByText("Select at least one file to commit."), + ).not.toBeInTheDocument(), + ); + + fireEvent.change(message, { target: { value: " " } }); + fireEvent.keyDown(message, { key: "Enter", metaKey: true }); + expect( + await within(panel).findByText("Enter a commit message first."), + ).toBeInTheDocument(); + }); + it("commits only the selected paths and refreshes the status afterward", async () => { render(); diff --git a/src/App.tsx b/src/App.tsx index 923ae7f..1fcf43f 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -688,6 +688,9 @@ export default function App() { const [gitCommitInFlight, setGitCommitInFlight] = useState(false); const [gitCommitError, setGitCommitError] = useState(); const [gitCommitSuccess, setGitCommitSuccess] = useState(); + // Why-nothing-happened hint for Cmd/Ctrl+Enter in the message box — the + // Commit button disables itself, but the shortcut has no visual state. + const [gitCommitHint, setGitCommitHint] = useState(); const [gitSyncInFlight, setGitSyncInFlight] = useState(false); const [gitSyncResult, setGitSyncResult] = useState(); const [gitSyncError, setGitSyncError] = useState(); @@ -1652,6 +1655,12 @@ export default function App() { return () => window.clearTimeout(timeoutId); }, [gitCommitSuccess]); + // The commit hint answers "why did Cmd+Enter do nothing" — the moment the + // user changes the message or the file selection, the answer is stale. + useEffect(() => { + setGitCommitHint(undefined); + }, [gitCommitMessage, gitCommitSelectedPaths]); + // Sync notices are scoped to a commit-panel session; clear them on the way out // so reopening the panel doesn't show a stale "Synced"/conflict line. useEffect(() => { @@ -1736,8 +1745,17 @@ export default function App() { const handleGitCommit = useCallback(async () => { const trimmedMessage = gitCommitMessage.trim(); const selectedPaths = changedFilePaths.filter((path) => gitCommitSelectedPaths.has(path)); - if (!trimmedMessage || selectedPaths.length === 0 || gitCommitInFlight) return; + if (gitCommitInFlight) return; + if (selectedPaths.length === 0) { + setGitCommitHint("Select at least one file to commit."); + return; + } + if (!trimmedMessage) { + setGitCommitHint("Enter a commit message first."); + return; + } + setGitCommitHint(undefined); setGitCommitInFlight(true); setGitCommitError(undefined); setGitCommitSuccess(undefined); @@ -5453,6 +5471,12 @@ export default function App() { {gitCommitSuccess} ) : null} + {gitCommitHint ? ( +
+ + {gitCommitHint} +
+ ) : null} )} diff --git a/src/styles.css b/src/styles.css index 8a9efc8..86c0046 100644 --- a/src/styles.css +++ b/src/styles.css @@ -976,6 +976,20 @@ button { color: var(--text); } +.commit-panel__notice--warning { + display: flex; + align-items: center; + gap: 6px; + border: 1px solid color-mix(in oklch, var(--warning) 42%, var(--border)); + background: color-mix(in oklch, var(--warning) 8%, var(--surface)); + color: var(--text); +} + +.commit-panel__notice--warning svg { + flex-shrink: 0; + color: var(--warning); +} + /* Transient success toasts (sync result, completed merge) hold, then ease out over the last stretch of their ~5s life so removal doesn't just blink away. The JS timeout clears the element at 5s; this only animates the opacity. */ @@ -1110,19 +1124,6 @@ button { font-weight: 600; } -.commit-panel__sync-conflicts { - margin: 0; - padding-left: 18px; - display: flex; - flex-direction: column; - gap: 2px; -} - -.commit-panel__sync-conflicts li { - overflow-wrap: anywhere; - word-break: break-word; -} - /* Live merge-resolution block: a bordered call-out that replaces the normal sync notice while a merge is unfinished. */ .commit-panel__merge { From 60c0b4d2ba317c72a85057ced89c6c9a27f638ea Mon Sep 17 00:00:00 2001 From: Gordon Beeming Date: Sat, 11 Jul 2026 01:36:22 +1000 Subject: [PATCH 2/4] Fix PR review feedback Address reviewer comments: - clear the commit hint on semantic selection changes, not Set identity - clear stale success/error lines when a precondition shows the hint --- src/App.tsx | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/App.tsx b/src/App.tsx index 1fcf43f..826c009 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1657,9 +1657,16 @@ export default function App() { // The commit hint answers "why did Cmd+Enter do nothing" — the moment the // user changes the message or the file selection, the answer is stale. + // Keyed on a sorted-path signature rather than the Set itself: background + // status polls rebuild the Set with identical contents, and clearing on + // reference identity would dismiss the hint without any user action. + const gitCommitSelectionSignature = useMemo( + () => [...gitCommitSelectedPaths].sort().join("\n"), + [gitCommitSelectedPaths], + ); useEffect(() => { setGitCommitHint(undefined); - }, [gitCommitMessage, gitCommitSelectedPaths]); + }, [gitCommitMessage, gitCommitSelectionSignature]); // Sync notices are scoped to a commit-panel session; clear them on the way out // so reopening the panel doesn't show a stale "Synced"/conflict line. @@ -1746,11 +1753,18 @@ export default function App() { const trimmedMessage = gitCommitMessage.trim(); const selectedPaths = changedFilePaths.filter((path) => gitCommitSelectedPaths.has(path)); if (gitCommitInFlight) return; + // A failed precondition also clears the previous attempt's outcome lines — + // a leftover "Committed …" or error next to the warning reads as if this + // attempt did something. if (selectedPaths.length === 0) { + setGitCommitError(undefined); + setGitCommitSuccess(undefined); setGitCommitHint("Select at least one file to commit."); return; } if (!trimmedMessage) { + setGitCommitError(undefined); + setGitCommitSuccess(undefined); setGitCommitHint("Enter a commit message first."); return; } From a69bee0097280238b15b1ca1a8dac58fbd5a1041 Mon Sep 17 00:00:00 2001 From: Gordon Beeming Date: Sat, 11 Jul 2026 01:52:22 +1000 Subject: [PATCH 3/4] Fix PR review feedback Address reviewer comments: - clear the commit hint when the commit panel closes - assert the hint clears after editing the message --- src/App.test.tsx | 7 +++++++ src/App.tsx | 1 + 2 files changed, 8 insertions(+) diff --git a/src/App.test.tsx b/src/App.test.tsx index 5665beb..79ef98e 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -4716,6 +4716,13 @@ describe("Git commit sidebar", () => { expect( await within(panel).findByText("Enter a commit message first."), ).toBeInTheDocument(); + + fireEvent.change(message, { target: { value: "Update readme" } }); + await waitFor(() => + expect( + within(panel).queryByText("Enter a commit message first."), + ).not.toBeInTheDocument(), + ); }); it("commits only the selected paths and refreshes the status afterward", async () => { diff --git a/src/App.tsx b/src/App.tsx index 826c009..1ea6d31 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1676,6 +1676,7 @@ export default function App() { setGitSyncError(undefined); setGitMergeError(undefined); setGitMergeSuccess(undefined); + setGitCommitHint(undefined); }, [commitModeActive]); // Clear the "Completed merge" line after a beat, matching the commit-success From 42941ce14436156a4ce724f614904d9c469eaa6d Mon Sep 17 00:00:00 2001 From: Gordon Beeming Date: Sat, 11 Jul 2026 02:02:03 +1000 Subject: [PATCH 4/4] Cover the Ctrl+Enter path in the commit hint test --- src/App.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/App.test.tsx b/src/App.test.tsx index 79ef98e..5a4028e 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -4712,7 +4712,8 @@ describe("Git commit sidebar", () => { ); fireEvent.change(message, { target: { value: " " } }); - fireEvent.keyDown(message, { key: "Enter", metaKey: true }); + // Ctrl instead of Cmd here so both halves of metaKey || ctrlKey get covered. + fireEvent.keyDown(message, { key: "Enter", ctrlKey: true }); expect( await within(panel).findByText("Enter a commit message first."), ).toBeInTheDocument();