feat: add hover state handling and opacity adjustments for floating button#1396
Merged
feat: add hover state handling and opacity adjustments for floating button#1396
Conversation
Contributor
📝 WalkthroughWalkthroughIntroduces hover-driven "peek and reveal" behavior: adds hover IPC/event, exposes renderer API to set hover, reduces collapsed widget size to 50×50, computes peeked off-screen bounds, adds opacity control (0.5 idle, 1 revealed), and implements a collapse reveal-lock and closing-motion state. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Renderer as FloatingButton.vue
participant Preload as floating-preload
participant Presenter as FloatingButtonPresenter
participant Window as FloatingButtonWindow
User->>Renderer: mouse enter
Renderer->>Preload: floatingButtonAPI.setHovering(true)
Preload->>Presenter: HOVER_STATE_CHANGED(true)
Presenter->>Presenter: update hover state, resolve opacity/bounds
Presenter->>Window: setOpacity(1)
Presenter->>Presenter: apply revealed layout
User->>Renderer: mouse leave
Renderer->>Preload: floatingButtonAPI.setHovering(false)
Preload->>Presenter: HOVER_STATE_CHANGED(false)
Presenter->>Presenter: update hover state, resolve opacity/bounds (peek)
Presenter->>Window: setOpacity(0.5)
Presenter->>Presenter: apply peeked layout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
test/main/presenter/floatingButtonPresenter/index.test.ts (1)
226-232:⚠️ Potential issue | 🟠 MajorThis drag-move expectation is still using the pre-resize x-offset.
After
DRAG_START, the presenter snaps the collapsed widget to the fully revealed x-position before applying the drag delta. With the new50pxwidth, moving from(100, 100)to(220, 150)lands at1270, so1256is stale.Proposed fix
await emitEvent(FLOATING_BUTTON_EVENTS.DRAG_MOVE, { x: 220, y: 150 }) expect(floatingWindowState.bounds).toMatchObject({ - x: 1256, + x: + electronState.workArea.x + + electronState.workArea.width - + getCollapsedWidgetSize(0).width + + (220 - 100), y: 230, width: getCollapsedWidgetSize(0).width, height: getCollapsedWidgetSize(0).height })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/main/presenter/floatingButtonPresenter/index.test.ts` around lines 226 - 232, The test expectation for the DRAG_MOVE event uses the old x-offset (1256); update the expected x to account for the presenter's snap-to-fully-revealed behavior after DRAG_START plus the drag delta so it becomes 1270. Modify the assertion in the FLOATING_BUTTON_EVENTS.DRAG_MOVE test to expect floatingWindowState.bounds.x === 1270 while keeping the width/height checks using getCollapsedWidgetSize(0).height/width and leave the rest of the assertion unchanged.src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts (1)
130-140:⚠️ Potential issue | 🟡 MinorDon't overwrite presenter-controlled opacity in
updateConfig().
src/main/presenter/floatingButtonPresenter/index.tsnow owns the0.5/1opacity state. This unconditional reset leaves a collapsed idle widget fully opaque after any config update until some later hover/layout event reapplies the dimmed state.Proposed fix
public updateConfig(config: Partial<FloatingButtonConfig>): void { this.config = { ...this.config, ...config } if (!this.window) { return } - - this.window.setOpacity(1) if (config.alwaysOnTop !== undefined) { this.window.setAlwaysOnTop(this.config.alwaysOnTop, 'floating') } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts` around lines 130 - 140, updateConfig in FloatingButtonWindow currently forces the window to full opacity with this.window.setOpacity(1), which overwrites the presenter-managed dimmed state; change it so opacity is only updated when an explicit opacity value is provided in the incoming config (e.g., if (config.opacity !== undefined) this.window.setOpacity(config.opacity)) and remove the unconditional setOpacity(1) call so presenter-controlled 0.5/1 state remains intact.src/renderer/floating/FloatingButton.vue (1)
1-2:⚠️ Potential issue | 🟡 MinorRun Prettier to fix formatting issues.
The pipeline reports Prettier formatting violations. As per coding guidelines, run
pnpm run formatto fix code style issues before merging.pnpm run format🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/floating/FloatingButton.vue` around lines 1 - 2, Run the project's formatter on the changed Vue script block to resolve Prettier violations: open src/renderer/floating/FloatingButton.vue (the <script setup lang="ts"> section where computed, onMounted, onUnmounted, ref are imported) and run the project's format command (pnpm run format) to apply Prettier rules, then review and stage the resulting formatting changes and update the commit/PR.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/floating/FloatingButton.vue`:
- Around line 132-138: handleMouseLeave skips while dragging so if a drag ends
outside the widget the hover state can remain true; update handleMouseUp to
reconcile hover by checking dragState.value.isDragging (or whether a drag just
finished) and call setHovering(false) (or invoke the same logic as
handleMouseLeave) after clearing dragState so isHovering is reset when the drag
ends outside the component; reference handleMouseUp, handleMouseLeave,
dragState, setHovering and isDragging when making the change.
In `@test/main/presenter/floatingButtonPresenter/layout.test.ts`:
- Around line 85-103: The test assumes a 64px collapsed button but the layout
now uses 50px; update the expectation in the 'hides half of the collapsed widget
outside the work area when idle' test to match the new collapsed size by
changing the asserted peeked x from 832 to 839 (or compute it dynamically using
getCollapsedWidgetSize(0).width) so it aligns with getPeekedCollapsedBounds and
the collapsed size defined in layout.ts.
---
Outside diff comments:
In `@src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts`:
- Around line 130-140: updateConfig in FloatingButtonWindow currently forces the
window to full opacity with this.window.setOpacity(1), which overwrites the
presenter-managed dimmed state; change it so opacity is only updated when an
explicit opacity value is provided in the incoming config (e.g., if
(config.opacity !== undefined) this.window.setOpacity(config.opacity)) and
remove the unconditional setOpacity(1) call so presenter-controlled 0.5/1 state
remains intact.
In `@src/renderer/floating/FloatingButton.vue`:
- Around line 1-2: Run the project's formatter on the changed Vue script block
to resolve Prettier violations: open src/renderer/floating/FloatingButton.vue
(the <script setup lang="ts"> section where computed, onMounted, onUnmounted,
ref are imported) and run the project's format command (pnpm run format) to
apply Prettier rules, then review and stage the resulting formatting changes and
update the commit/PR.
In `@test/main/presenter/floatingButtonPresenter/index.test.ts`:
- Around line 226-232: The test expectation for the DRAG_MOVE event uses the old
x-offset (1256); update the expected x to account for the presenter's
snap-to-fully-revealed behavior after DRAG_START plus the drag delta so it
becomes 1270. Modify the assertion in the FLOATING_BUTTON_EVENTS.DRAG_MOVE test
to expect floatingWindowState.bounds.x === 1270 while keeping the width/height
checks using getCollapsedWidgetSize(0).height/width and leave the rest of the
assertion unchanged.
🪄 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
Run ID: a62c009e-9622-4bbe-8490-3c2cd3771728
📒 Files selected for processing (9)
src/main/events.tssrc/main/presenter/floatingButtonPresenter/FloatingButtonWindow.tssrc/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/floatingButtonPresenter/layout.tssrc/preload/floating-preload.tssrc/renderer/floating/FloatingButton.vuesrc/renderer/floating/env.d.tstest/main/presenter/floatingButtonPresenter/index.test.tstest/main/presenter/floatingButtonPresenter/layout.test.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/renderer/floating/FloatingButton.vue (1)
128-138:⚠️ Potential issue | 🟡 MinorKeep hover latched for the full press/drag gesture.
Line 133 only suppresses
mouseleaveafterisDraggingflips true. During the 180ms delay / before the 4px threshold, a quick exit still sendssetHovering(false)while the button is held, so the main presenter can peek the window back toward the edge mid-gesture (src/preload/floating-preload.ts:62-64,src/main/presenter/floatingButtonPresenter/index.ts:369-381).handleMouseUpalso never reconciles hover afterward, so once it drifts it can stay wrong until the next enter/leave.💡 Possible fix
const handleMouseLeave = () => { - if (dragState.value.isDragging) { + if (dragState.value.isMouseDown) { return } setHovering(false) } + +const reconcileHovering = (event: MouseEvent) => { + const hovered = document.elementFromPoint(event.clientX, event.clientY) as HTMLElement | null + setHovering(Boolean(hovered?.closest('.widget-stage'))) +} const handleMouseUp = (event: MouseEvent) => { if (event.button !== 0) { return } @@ if (wasDragging) { dragState.value.isDragging = false isDragging.value = false window.floatingButtonAPI.onDragEnd(event.screenX, event.screenY) } else if (!snapshot.value.expanded) { toggleExpanded() } + + reconcileHovering(event) document.removeEventListener('mousemove', handleMouseMove) document.removeEventListener('mouseup', handleMouseUp) }Also applies to: 189-202, 266-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/floating/FloatingButton.vue` around lines 128 - 138, The mouseleave handler currently only blocks clearing hover when dragState.value.isDragging is true, which allows clearing during the initial press-to-drag delay; update handleMouseLeave to keep hover latched for the entire press/drag gesture by also checking whatever flag indicates an ongoing press (e.g., dragState.value.isPressed or dragState.value.isPointerDown) and return early if that flag is set; additionally, update handleMouseUp to re-evaluate/reconcile hover state after release (call setHovering(...) based on pointer position/element under pointer or simply setHovering(false) only when not pressed/dragging) so hover can't remain out-of-sync—apply the same guarded logic pattern to the other similar handlers mentioned (lines around 189-202 and 266-267) using the same dragState symbols and setHovering function.
🧹 Nitpick comments (1)
src/renderer/floating/FloatingButton.vue (1)
570-572: These close-motion overrides look effectively dead.By the time
data-motion='closing'renders,snapshot.expandedhas already flipped false, so the collapsed side no longer carries.collapsed-layer-hidden. That means these selectors never participate in the close path and can mislead later motion tweaks.Also applies to: 652-654
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/floating/FloatingButton.vue` around lines 570 - 572, The CSS rules targeting ".widget-stage[data-motion='closing'] .collapsed-layer-hidden" are dead because snapshot.expanded is already false by the time data-motion='closing' is applied, so elements no longer have the .collapsed-layer-hidden class; remove these closing-specific overrides (the rules around ".widget-stage[data-motion='closing'] .collapsed-layer-hidden" and the duplicate block at 652-654) or, if you intended to affect the close animation, instead ensure the element keeps .collapsed-layer-hidden until after the closing motion by adjusting the snapshot.expanded toggle logic; reference the selectors ".widget-stage[data-motion='closing'] .collapsed-layer-hidden", the ".collapsed-layer-hidden" class and the snapshot.expanded state when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/renderer/floating/FloatingButton.vue`:
- Around line 128-138: The mouseleave handler currently only blocks clearing
hover when dragState.value.isDragging is true, which allows clearing during the
initial press-to-drag delay; update handleMouseLeave to keep hover latched for
the entire press/drag gesture by also checking whatever flag indicates an
ongoing press (e.g., dragState.value.isPressed or dragState.value.isPointerDown)
and return early if that flag is set; additionally, update handleMouseUp to
re-evaluate/reconcile hover state after release (call setHovering(...) based on
pointer position/element under pointer or simply setHovering(false) only when
not pressed/dragging) so hover can't remain out-of-sync—apply the same guarded
logic pattern to the other similar handlers mentioned (lines around 189-202 and
266-267) using the same dragState symbols and setHovering function.
---
Nitpick comments:
In `@src/renderer/floating/FloatingButton.vue`:
- Around line 570-572: The CSS rules targeting
".widget-stage[data-motion='closing'] .collapsed-layer-hidden" are dead because
snapshot.expanded is already false by the time data-motion='closing' is applied,
so elements no longer have the .collapsed-layer-hidden class; remove these
closing-specific overrides (the rules around
".widget-stage[data-motion='closing'] .collapsed-layer-hidden" and the duplicate
block at 652-654) or, if you intended to affect the close animation, instead
ensure the element keeps .collapsed-layer-hidden until after the closing motion
by adjusting the snapshot.expanded toggle logic; reference the selectors
".widget-stage[data-motion='closing'] .collapsed-layer-hidden", the
".collapsed-layer-hidden" class and the snapshot.expanded state when making the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f222862-ac35-4ff1-a6ea-ddc543fc759e
📒 Files selected for processing (1)
src/renderer/floating/FloatingButton.vue
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.
close #1393
20260326_144013.mp4
Summary by CodeRabbit
New Features
Tests