feat(electron): resizable Settings popover with persist + restore default size - #95
Merged
Merged
Conversation
- Add 'QA after implementing, before ship' to Industry Standards table - Add 'Ship before QA passes' to Violations table - Add Definition of Done section (QA non-negotiable policy) - Rename 'smoke test/smoke' → 'QA' throughout (2 occurrences) - Add Dev vs Production Electron comparison table (bundle ID, app name, renderer URL, app.isPackaged, NODE_ENV, computer-use grantability)
- WindowManager: resizable:true, min/max constraints, load/clamp persisted size from ConfigManager, 200ms debounce-persist on resize, will-resize flag to guard blur→hide mid-drag, resetSettingsPopoverSize() re-anchors to tray - ConfigManager: SettingsPopoverConfig interface + settingsPopover defaults - IPC: settings:resetPopoverSize channel (types/ipc.ts + ipc-schemas.ts) - main.ts: typedHandle for resetPopoverSize - preload.ts: settings.resetPopoverSize() exposed with version-skew guard - electron-api.d.ts: resetPopoverSize() type declaration - ElectronSettingsPage: Settings Window Card + resize grip + Restore button - Tests: WindowManager.settings-resize (20 cases), ConfigManager.settings-popover (3 cases), ipc-contract extension (1 case) - pnpm validate green (622 unit + 297 electron + build + lint + typecheck)
…sion to utils, useTransition, will-resize failsafe, observable test names
- ElectronSettingsPage: 4 tests for Settings Window card render, button click→IPC call, preload version-skew guard (method absent → no-op) - clampDimension: 9 unit tests covering all invalid input categories (NaN, zero, negative, Infinity, undefined) + undersized, oversized, fractional rounding - ConfigManager.settings-popover: partial-key migration scenario (width only in config.json → height backfilled to default 380)
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughAdds user-resizable Settings popover with config-backed dimension persistence, a ChangesSettings Popover Resize Persistence & Reset Feature
CLAUDE.md QA Governance and Electron Testing Documentation
Sequence DiagramsequenceDiagram
participant Renderer as ElectronSettingsPage
participant preload as preload.ts
participant main as main.ts (IPC handler)
participant WindowManager
participant ConfigManager
rect rgba(70, 130, 180, 0.5)
Note over Renderer,preload: User clicks "Restore default size"
Renderer->>preload: settings.resetPopoverSize()
preload->>main: typedInvoke("settings:resetPopoverSize")
main->>WindowManager: resetSettingsPopoverSize()
WindowManager->>ConfigManager: update(settingsPopover {width:360, height:380})
WindowManager->>WindowManager: setBounds to default + re-anchor to tray
WindowManager-->>main: void
main-->>preload: true
preload-->>Renderer: Promise(true)
end
rect rgba(60, 179, 113, 0.5)
Note over Renderer,WindowManager: User drags resize edge
WindowManager->>WindowManager: will-resize → isResizingSettingsPopover=true
WindowManager->>WindowManager: resize → debounce 200ms
WindowManager->>ConfigManager: update(settingsPopover {width, height})
WindowManager->>WindowManager: isResizingSettingsPopover=false
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #95 +/- ##
==========================================
- Coverage 69.35% 66.83% -2.53%
==========================================
Files 125 128 +3
Lines 4004 4215 +211
Branches 1039 1115 +76
==========================================
+ Hits 2777 2817 +40
- Misses 1058 1195 +137
- Partials 169 203 +34 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@electron/WindowManager.ts`:
- Around line 1451-1457: The `settingsResizeDebounceTimer` field is being used
for two independent purposes: managing the resize blur-guard timeout (500ms) and
managing persistence debounce (200ms). This causes the blur-guard logic to
interfere with persistence by clearing the persistence timer before it fires,
dropping unsaved window sizes. Split this into two separate timer fields: one
dedicated to managing the resize blur-guard state (settingsResizeDebounceTimer
for the 500ms timeout that controls settingsWindowIsResizing) and another
dedicated to persistence debounce (create a new field like
settingsPersistDebounceTimer for the 200ms persistence logic). Update all
references throughout the code to use the appropriate timer field for each
concern.
In `@src/components/electron/ElectronSettingsPage.tsx`:
- Around line 191-193: The async callback passed to startResetSizeTransition
calls resetFn() without checking its return value. Since resetFn() (the
resetPopoverSize() function) returns false on IPC failure, capture the return
value from the await resetFn() call and add error handling logic to notify the
user when the reset fails, preventing silent no-op failures where the button
appears to do nothing.
🪄 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: bb48a062-8f13-485c-8bbb-559bacd6cd8e
📒 Files selected for processing (16)
CLAUDE.mdelectron/ConfigManager.tselectron/WindowManager.tselectron/__tests__/ConfigManager.settings-popover.test.tselectron/__tests__/WindowManager.settings-resize.test.tselectron/__tests__/clampDimension.test.tselectron/__tests__/ipc-contract.test.tselectron/constants.tselectron/ipc/ipc-schemas.tselectron/main.tselectron/preload.tselectron/types/electron-api.d.tselectron/types/ipc.tselectron/utils/clampDimension.tssrc/components/electron/ElectronSettingsPage.test.tsxsrc/components/electron/ElectronSettingsPage.tsx
- Split settingsResizeDebounceTimer (500ms blur-guard) and settingsPersistDebounceTimer (200ms persist debounce) into separate fields so will-resize guard can never cancel a pending size write - Handle resetPopoverSize() false return in handleResetPopoverSize to surface IPC failures in dev instead of silently no-op
5 tasks
5 tasks
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
Adds drag-to-resize for the Settings tray popover and a "Restore default size" button. The window remembers its size across sessions by persisting it to
configManager, with clamping on load to guard against corrupted values.feat: resizable Settings popover + restore default size
WindowManager.createSettingsWindow():resizable: true, min/max constraints, persisted size loaded+clamped at creationwill-resize+resize(200 ms debounce): persist width/height toconfigManagersettingsWindowIsResizingflag skips hide-on-blur during dragwill-resizefires without a followingresize(user clicks handle but doesn't drag), the latch resets after 500 ms — prevents the popover being permanently stuck openresetSettingsPopoverSize(): reverts to 360×380 viasetBounds()+ config updatepointer-events-none) in bottom-right cornerrefactor: fix review findings
electron/constants.tsclampDimension()extracted toelectron/utils/clampDimension.tswith JSDocuseState(false)→useTransitionfor reset button pending stateresetFnref before entering transitionwill-resizelatch bug fixed: 500 ms failsafe timerfix: design-review — remove dead
cursor: se-resizepointer-events-noneelement ignorescursorCSS; removed the propertytest: coverage for settings-popover-resize UI and utilities
ElectronSettingsPage.test.tsx: 4 tests (Settings Window card render, button click→IPC, preload skew guard, null return off-Electron)clampDimension.test.ts: 9 unit tests covering all invalid categories (NaN, zero, negative, Infinity, undefined) + undersized/oversized/fractionalConfigManager.settings-popover.test.ts: +1 partial-key migration test (width-only config → height backfilled)Test Coverage
Tests: 622 → 626 unit (+4), 298 → 308 Electron (+10)
Pre-Landing Review
/reviewran on the full diff before the design-fix commit. All findings addressed:constants.ts✅clampDimensionextracted toutils/with JSDoc ✅useState(false)→useTransition✅will-resizelatch bug (permanent stuck-open) fixed with 500 ms failsafe ✅No new findings on the subsequent test-only commits.
Design Review
/design-reviewran: 1 finding auto-fixed (cursor: se-resizeonpointer-events-noneelement — dead CSS). Committede76eeb4.Plan Completion
No plan file — feature was implemented autonomously per
/goal.TODOS
No TODO items completed in this PR.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation