feat: setting to auto-expand commit details in History tab#1695
Open
ubuntudroid wants to merge 6 commits intogeneralaction:mainfrom
Open
feat: setting to auto-expand commit details in History tab#1695ubuntudroid wants to merge 6 commits intogeneralaction:mainfrom
ubuntudroid wants to merge 6 commits intogeneralaction:mainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@ubuntudroid is attempting to deploy a commit to the General Action Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (clicks toggle / selects commit)"
participant UI as "Settings UI\nCommitDetailSettingsCard"
participant Settings as "AppSettings Provider\n(normalize & persist)"
participant History as "HistoryTab"
User->>UI: Toggle "Expand commit details by default"
UI->>Settings: updateSettings({ interface:{ expandCommitDetail } })
Settings-->>UI: confirm saved
User->>History: Select commit
History->>Settings: read settings.interface.expandCommitDetail
Settings-->>History: return boolean
History->>History: set detailExpanded = expandCommitDetail
History-->>User: render commit detail expanded/collapsed (subject truncation conditional)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ubuntudroid
force-pushed
the
sven/show-full-commit-message-above-selected-commit-6c3
branch
from
d30e01c to
05c7edf
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/renderer/components/CommitDetailSettingsCard.tsx (1)
21-28: Remove redundantdefaultCheckedprop on controlledSwitch.The
Switchis controlled via thecheckedprop. UsingdefaultCheckedon a controlled component is unnecessary and can cause React warnings or confusion. Remove it for cleaner code.♻️ Proposed fix
<Switch checked={expandCommitDetail} - defaultChecked={expandCommitDetail} disabled={loading} onCheckedChange={(checked) => updateSettings({ interface: { expandCommitDetail: checked } }) } />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/CommitDetailSettingsCard.tsx` around lines 21 - 28, The Switch is being used as a controlled component (checked={expandCommitDetail}), so remove the redundant defaultChecked prop to avoid React warnings; update the JSX for the Switch (the element using props checked, defaultChecked, disabled, onCheckedChange) by deleting defaultChecked={expandCommitDetail} and keep checked={expandCommitDetail}, disabled={loading}, and the onCheckedChange handler that calls updateSettings({ interface: { expandCommitDetail: checked } }).src/test/main/settings.test.ts (1)
287-302: Good test coverage; consider adding an edge case test.The test suite correctly covers the main scenarios. For consistency with other test suites (e.g.,
showResourceMonitorat lines 100-103), consider adding a test for when theinterfacesection exists butexpandCommitDetailis undefined:it('coerces missing value inside interface to false', () => { const result = normalizeSettings(makeSettings({ interface: {} })); expect(result.interface?.expandCommitDetail).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/main/settings.test.ts` around lines 287 - 302, Add an edge-case unit test to the "normalizeSettings – expandCommitDetail" suite: when an interface object exists but expandCommitDetail is omitted, normalizeSettings should coerce it to false; add a spec that calls normalizeSettings(makeSettings({ interface: {} })) and asserts result.interface?.expandCommitDetail === false so the behavior mirrors other suites like showResourceMonitor and covers the undefined-inside-interface case for normalizeSettings and makeSettings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/components/CommitDetailSettingsCard.tsx`:
- Around line 21-28: The Switch is being used as a controlled component
(checked={expandCommitDetail}), so remove the redundant defaultChecked prop to
avoid React warnings; update the JSX for the Switch (the element using props
checked, defaultChecked, disabled, onCheckedChange) by deleting
defaultChecked={expandCommitDetail} and keep checked={expandCommitDetail},
disabled={loading}, and the onCheckedChange handler that calls updateSettings({
interface: { expandCommitDetail: checked } }).
In `@src/test/main/settings.test.ts`:
- Around line 287-302: Add an edge-case unit test to the "normalizeSettings –
expandCommitDetail" suite: when an interface object exists but
expandCommitDetail is omitted, normalizeSettings should coerce it to false; add
a spec that calls normalizeSettings(makeSettings({ interface: {} })) and asserts
result.interface?.expandCommitDetail === false so the behavior mirrors other
suites like showResourceMonitor and covers the undefined-inside-interface case
for normalizeSettings and makeSettings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9919a3b5-d7a4-41a9-80f2-ab33cfdb2b58
📒 Files selected for processing (6)
src/main/settings.tssrc/renderer/components/CommitDetailSettingsCard.tsxsrc/renderer/components/SettingsPage.tsxsrc/renderer/components/diff-viewer/HistoryTab.tsxsrc/test/main/settings.test.tssrc/test/renderer/CommitDetailSettingsCard.component.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes
Fixes #1693
Fixes #1694
Snapshot
N/A
Type of change
Mandatory Tasks
Checklist
Summary by CodeRabbit
New Features
Tests