Show why Cmd+Enter did not commit#54
Conversation
📝 WalkthroughWalkthroughChangesGit commit feedback
Sync conflict styling
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a visual hint in the Git commit panel to explain why a commit action (such as Cmd+Enter) did nothing when there are no selected files or when the commit message is empty. It also adds a corresponding test and styling for the warning notice. Feedback on the changes points out that using gitCommitSelectedPaths (a Set) in the useEffect dependency array will cause the hint to be prematurely cleared during background status polls because the set reference changes. It is recommended to serialize the selected paths to prevent this issue.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR improves the Git commit sidebar UX by providing explicit feedback when users press Cmd/Ctrl+Enter in the commit message box but a commit cannot proceed (no selected files or empty message), aligning the keyboard shortcut path with the disabled Commit button behavior.
Changes:
- Adds a new “warning” notice style and renders a hint message when Cmd/Ctrl+Enter is pressed but commit preconditions aren’t met.
- Clears the hint when the commit message or file selection changes.
- Adds test coverage for the new hint behavior and removes a dead CSS rule from the old conflict UI.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/styles.css | Adds warning notice styling and removes unused .commit-panel__sync-conflicts rules. |
| src/App.tsx | Introduces gitCommitHint, sets it on invalid shortcut commit attempts, and renders a warning notice in the commit panel. |
| src/App.test.tsx | Adds a regression test ensuring the hint appears for invalid shortcut commits and clears after user action. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/App.test.tsx (1)
4714-4718: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover clearing after the commit message changes.
The test verifies clearing after file selection changes, but not after a visible message hint is followed by a valid message. Add an assertion that changing
" "to a non-whitespace message removes"Enter a commit message first.".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.test.tsx` around lines 4714 - 4718, Extend the relevant test in App.test.tsx after the existing visible hint assertion: change the message from whitespace to a valid non-whitespace commit message, then assert that “Enter a commit message first.” is no longer present in the panel. Keep the existing file-selection clearing coverage intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.tsx`:
- Around line 1658-1662: Update the effect that clears gitCommitHint to depend
on a stable, order-independent signature of gitCommitSelectedPaths, such as
sorted joined paths, instead of the Set object identity; alternatively, make
refreshGitStatus preserve the existing Set when members are unchanged. Keep
gitCommitMessage as a dependency so the hint still clears on message edits.
---
Nitpick comments:
In `@src/App.test.tsx`:
- Around line 4714-4718: Extend the relevant test in App.test.tsx after the
existing visible hint assertion: change the message from whitespace to a valid
non-whitespace commit message, then assert that “Enter a commit message first.”
is no longer present in the panel. Keep the existing file-selection clearing
coverage intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 252a292a-8dc0-4c01-9fd7-dc220a53d49c
📒 Files selected for processing (3)
src/App.test.tsxsrc/App.tsxsrc/styles.css
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
Address reviewer comments: - clear the commit hint when the commit panel closes - assert the hint clears after editing the message
Summary
.commit-panel__sync-conflictsCSS rule left over from the old conflict UI (kept the CSS budget green).Test plan
npm test— 403 passing (new coverage: hint shows for no-selection and empty-message, clears when the selection changes)npx tsc --noEmit,npm run build,npm run budget,npm run smoke,npm run theme:checkSummary by CodeRabbit