fix(floating-button): persist resting position across restarts - #1715
Merged
Conversation
The floating button relied on electron-window-state, which tracks live
window bounds. Because the widget idles in a peeked (half-off-screen)
position, electron-window-state recorded an off-screen rectangle and then
reset it to {0,0} on the next launch (ensureWindowVisibleOnSomeDisplay),
so the button always jumped to the top-left corner after a restart.
Persist the snapped, fully on-screen docked position (y + dock side) via
configPresenter on drag end, and restore it on window creation, re-docking
to the saved edge and clamping into the current display work area.
Contributor
📝 WalkthroughWalkthroughThis PR replaces ChangesFloating Button Position Persistence
Sequence DiagramssequenceDiagram
participant FloatingButtonPresenter
participant ConfigPresenter
participant FloatingButtonWindow
participant Display
FloatingButtonPresenter->>ConfigPresenter: getFloatingButtonBounds()
ConfigPresenter-->>FloatingButtonPresenter: FloatingButtonBounds | null
FloatingButtonPresenter->>FloatingButtonWindow: new FloatingButtonWindow(config, persistedBounds)
FloatingButtonWindow->>FloatingButtonWindow: resolveInitialBounds()
alt persistedBounds exist
FloatingButtonWindow->>Display: getDisplayNearestPoint(y)
Display-->>FloatingButtonWindow: display work area
FloatingButtonWindow->>FloatingButtonWindow: clamp y to work area
FloatingButtonWindow->>FloatingButtonWindow: align x to dockSide edge
else no persisted bounds
FloatingButtonWindow->>FloatingButtonWindow: compute from config.position
FloatingButtonWindow->>Display: clamp to work area
end
FloatingButtonWindow-->>FloatingButtonPresenter: initialized with bounds
sequenceDiagram
participant User
participant FloatingButtonPresenter
participant FloatingButtonWindow
participant ConfigPresenter
User->>FloatingButtonWindow: drag start/move/end
FloatingButtonPresenter->>FloatingButtonPresenter: onDragEnd()
FloatingButtonPresenter->>FloatingButtonPresenter: snap bounds to edge and dockSide
FloatingButtonPresenter->>FloatingButtonPresenter: persistFloatingBounds(snapped)
FloatingButtonPresenter->>ConfigPresenter: setFloatingButtonBounds({x, y, dockSide})
ConfigPresenter->>ConfigPresenter: setSetting('floatingButtonBounds', value)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/presenter/floatingButtonPresenter/index.ts (1)
344-355:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist the collapsed resting bounds, not the dragged rectangle.
Line 354 currently saves whatever size was being dragged. If the user drops while the widget is expanded or otherwise revealed, the saved
ybelongs to that larger rect, so the next launch restores the collapsed button at the wrong resting height.💡 Suggested fix
const currentDisplay = screen.getDisplayMatching(stableBounds) const snapped = snapWidgetBoundsToEdge(stableBounds, currentDisplay.workArea) + const restingBounds = repositionWidgetForResize( + snapped, + getWidgetSizeForSnapshot({ ...this.snapshot, expanded: false }), + currentDisplay.workArea, + snapped.dockSide + ) this.floatingWindow.setDockSide(snapped.dockSide) this.floatingWindow.setBounds(snapped) - this.persistFloatingBounds(snapped) + this.persistFloatingBounds({ ...restingBounds, dockSide: snapped.dockSide })🤖 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/main/presenter/floatingButtonPresenter/index.ts` around lines 344 - 355, The code is persisting the dragged rectangle (snapped) instead of the widget's collapsed resting bounds; update the logic in the drop/finish-drag path so that after computing snapped via snapWidgetBoundsToEdge(stableBounds, currentDisplay.workArea) and applying it with this.floatingWindow.setDockSide/snapped and this.floatingWindow.setBounds(snapped), you compute the collapsed resting rect (e.g., via this.floatingWindow.getCollapsedBounds() or by using the widget's collapsed height/y) and call this.persistFloatingBounds(collapsedBounds) instead of this.persistFloatingBounds(snapped); keep this.isDragging = false as-is.
🧹 Nitpick comments (1)
test/main/presenter/floatingButtonPresenter/index.test.ts (1)
280-286: ⚡ Quick winTest doesn't actually verify restoration.
Since
getFloatingButtonBoundsis mocked to returnnull(Line 197), this test only proves the method is called duringinitialize()— it never exercises the restore-to-persisted-position path that the title promises. Consider overriding the stub to return a concrete persisted record and asserting the window is re-docked/clamped to it (e.g.setBounds/setDockSidecalled with the expected on-screen coordinates).💚 Example: assert actual restoration
it('restores the persisted resting position on initialization', async () => { const configPresenter = createConfigPresenter() + configPresenter.getFloatingButtonBounds.mockReturnValue({ x: 0, y: 300, dockSide: 'left' }) floatingPresenter = new FloatingButtonPresenter(configPresenter) await floatingPresenter.initialize() expect(configPresenter.getFloatingButtonBounds).toHaveBeenCalled() + expect(floatingWindowState.dockSide).toBe('left') + expect(floatingWindowState.bounds.y).toBe(300) })🤖 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 `@test/main/presenter/floatingButtonPresenter/index.test.ts` around lines 280 - 286, The test currently only verifies that configPresenter.getFloatingButtonBounds is called; update the test to stub getFloatingButtonBounds to return a concrete persisted bounds/dock record and then assert FloatingButtonPresenter.initialize actually restores it by verifying calls to window methods like setBounds and setDockSide (or presenter methods that apply positioning) on the floatingPresenter; locate the stubbed getFloatingButtonBounds (used when creating configPresenter), set it to return the expected persisted object, call floatingPresenter.initialize(), and assert the expected setBounds/setDockSide calls were made with the on-screen coordinates.
🤖 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.
Outside diff comments:
In `@src/main/presenter/floatingButtonPresenter/index.ts`:
- Around line 344-355: The code is persisting the dragged rectangle (snapped)
instead of the widget's collapsed resting bounds; update the logic in the
drop/finish-drag path so that after computing snapped via
snapWidgetBoundsToEdge(stableBounds, currentDisplay.workArea) and applying it
with this.floatingWindow.setDockSide/snapped and
this.floatingWindow.setBounds(snapped), you compute the collapsed resting rect
(e.g., via this.floatingWindow.getCollapsedBounds() or by using the widget's
collapsed height/y) and call this.persistFloatingBounds(collapsedBounds) instead
of this.persistFloatingBounds(snapped); keep this.isDragging = false as-is.
---
Nitpick comments:
In `@test/main/presenter/floatingButtonPresenter/index.test.ts`:
- Around line 280-286: The test currently only verifies that
configPresenter.getFloatingButtonBounds is called; update the test to stub
getFloatingButtonBounds to return a concrete persisted bounds/dock record and
then assert FloatingButtonPresenter.initialize actually restores it by verifying
calls to window methods like setBounds and setDockSide (or presenter methods
that apply positioning) on the floatingPresenter; locate the stubbed
getFloatingButtonBounds (used when creating configPresenter), set it to return
the expected persisted object, call floatingPresenter.initialize(), and assert
the expected setBounds/setDockSide calls were made with the on-screen
coordinates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7df14693-cc49-4fd2-88f9-0be9b3db3d70
📒 Files selected for processing (10)
docs/issues/floating-button-position-persistence/plan.mddocs/issues/floating-button-position-persistence/spec.mddocs/issues/floating-button-position-persistence/tasks.mdsrc/main/presenter/configPresenter/index.tssrc/main/presenter/floatingButtonPresenter/FloatingButtonWindow.tssrc/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/floatingButtonPresenter/layout.tssrc/shared/types/floating-widget.tssrc/shared/types/presenters/legacy.presenters.d.tstest/main/presenter/floatingButtonPresenter/index.test.ts
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
1 participant
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.
背景
Closes #1714
悬浮按钮被拖动后，重启应用不会记住位置，每次都跳回主屏幕左上角。
根因
悬浮按钮通过
electron-window-state持久化位置，它跟踪实时窗口边界。但按钮空闲时停靠在 peeked（半隐藏）状态，一半在屏幕外：electron-window-state记录了这个屏幕外矩形；ensureWindowVisibleOnSomeDisplay()判定坐标不可见，重置为{0,0}，按钮跳到左上角；destroy()（非close()）销毁，只触发closed，退出前位置捕获不可靠。方案
不再用
electron-window-state跟踪悬浮按钮的实时边界，改为持久化逻辑停靠位置（吸附后、完全可见的坐标 + dock 方向），通过本项目惯用的configPresenter（electron-store）存储：DRAG_END）吸附到边缘后，保存snapped的{ x, y, dockSide }（始终在屏幕内）。纯主进程改动，不涉及 renderer / IPC 路由，无新增依赖，无数据迁移（旧的
floating-button-window-state.json不再读取即可）。行为对比（BEFORE / AFTER）
改动文件
src/shared/types/floating-widget.ts：新增FloatingWidgetDockSide、FloatingButtonBounds类型。src/main/presenter/floatingButtonPresenter/layout.ts：复用共享FloatingWidgetDockSide（单一来源）。src/main/presenter/configPresenter/index.ts+IConfigPresenter：新增getFloatingButtonBounds/setFloatingButtonBounds。src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts：移除electron-window-state，接收并恢复持久化位置。src/main/presenter/floatingButtonPresenter/index.ts：创建时读取、拖拽结束时保存。docs/issues/floating-button-position-persistence/：SDD spec/plan/tasks。测试
DRAG_END后持久化吸附位置、初始化时读取持久化位置。pnpm vitest run test/main/presenter/floatingButtonPresenter→ 11/11 通过。pnpm run typecheck、pnpm run lint、pnpm run format、pnpm run i18n通过。🤖 Generated with Claude Code
Summary by CodeRabbit