feat: add visual feedback when Force Release Stuck Drag is clicked #81
Conversation
The menu item is a one-shot action (not a toggle), so there's no checkmark. Added a brief flash of the status bar icon to confirm the action was triggered.
There was a problem hiding this comment.
Pull request overview
This PR adds visual feedback when the "Force Release Stuck Drag" menu item is clicked. Since this is a one-shot action without a toggle state, a brief flash animation of the status bar icon was added to confirm the action was triggered.
Changes:
- Added call to
flashStatusBarIcon()in theforceReleaseStuckDrag()method - Implemented new
flashStatusBarIcon()method that attempts to flash the status bar icon by temporarily changing its template rendering mode
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Store original image | ||
| let originalImage = button.image | ||
|
|
||
| // Flash to a highlighted state (use template rendering) | ||
| if let image = originalImage { | ||
| let highlightedImage = image.copy() as? NSImage | ||
| highlightedImage?.isTemplate = false // Make it appear "active" | ||
| button.image = highlightedImage | ||
| } | ||
|
|
||
| // Restore after a brief delay | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { | ||
| button.image = originalImage | ||
| } |
There was a problem hiding this comment.
The flash effect implementation has a fundamental issue: setting isTemplate = false on a copy of an already-template image doesn't produce a visible change. Template images are rendered using the system's accent color, and copying the image and changing this property doesn't make it appear different to the user.
For a visible flash effect, consider using an alpha animation (similar to the pattern in updateStatusIcon at lines 51-59) or changing the image to a different symbol temporarily.
| // Store original image | |
| let originalImage = button.image | |
| // Flash to a highlighted state (use template rendering) | |
| if let image = originalImage { | |
| let highlightedImage = image.copy() as? NSImage | |
| highlightedImage?.isTemplate = false // Make it appear "active" | |
| button.image = highlightedImage | |
| } | |
| // Restore after a brief delay | |
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { | |
| button.image = originalImage | |
| } | |
| let originalAlpha = button.alphaValue | |
| NSAnimationContext.runAnimationGroup({ context in | |
| context.duration = 0.1 | |
| button.animator().alphaValue = 0.3 | |
| }, completionHandler: { | |
| NSAnimationContext.runAnimationGroup({ context in | |
| context.duration = 0.1 | |
| button.animator().alphaValue = originalAlpha | |
| }, completionHandler: nil) | |
| }) |
| // Store original image | ||
| let originalImage = button.image | ||
|
|
||
| // Flash to a highlighted state (use template rendering) | ||
| if let image = originalImage { | ||
| let highlightedImage = image.copy() as? NSImage | ||
| highlightedImage?.isTemplate = false // Make it appear "active" | ||
| button.image = highlightedImage | ||
| } | ||
|
|
||
| // Restore after a brief delay | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { | ||
| button.image = originalImage | ||
| } |
There was a problem hiding this comment.
This implementation has a potential race condition if the user clicks the menu item multiple times rapidly. Each invocation captures a different originalImage, and the last async block to execute might restore the wrong image. The image reference could also become nil if the button is deallocated.
Consider storing the original image as a property to avoid capturing stale references, or use a cancellable timer/task pattern to cancel pending restoration when a new flash is triggered.
|
|
||
| // Store original image | ||
| let originalImage = button.image | ||
|
|
||
| // Flash to a highlighted state (use template rendering) | ||
| if let image = originalImage { | ||
| let highlightedImage = image.copy() as? NSImage | ||
| highlightedImage?.isTemplate = false // Make it appear "active" | ||
| button.image = highlightedImage | ||
| } | ||
|
|
||
| // Restore after a brief delay | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { | ||
| button.image = originalImage | ||
| } |
There was a problem hiding this comment.
The flash implementation pattern differs from the existing visual feedback pattern in this class. The updateStatusIcon method (lines 43-60) uses NSAnimationContext with alpha animation for visual feedback, which is more consistent with macOS UI conventions and provides smoother animations.
Consider using a similar animation approach here for consistency, such as briefly animating the button's alpha or scale.
| // Store original image | |
| let originalImage = button.image | |
| // Flash to a highlighted state (use template rendering) | |
| if let image = originalImage { | |
| let highlightedImage = image.copy() as? NSImage | |
| highlightedImage?.isTemplate = false // Make it appear "active" | |
| button.image = highlightedImage | |
| } | |
| // Restore after a brief delay | |
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { | |
| button.image = originalImage | |
| } | |
| // Use a brief alpha animation for visual feedback, consistent with updateStatusIcon | |
| NSAnimationContext.runAnimationGroup { context in | |
| context.duration = 0.15 | |
| button.animator().alphaValue = 0.3 | |
| } completionHandler: { | |
| NSAnimationContext.runAnimationGroup { context in | |
| context.duration = 0.15 | |
| button.animator().alphaValue = 1.0 | |
| } | |
| } |
| private func flashStatusBarIcon() { | ||
| guard let button = statusItem?.button else { return } | ||
|
|
||
| // Store original image | ||
| let originalImage = button.image | ||
|
|
||
| // Flash to a highlighted state (use template rendering) | ||
| if let image = originalImage { | ||
| let highlightedImage = image.copy() as? NSImage | ||
| highlightedImage?.isTemplate = false // Make it appear "active" | ||
| button.image = highlightedImage | ||
| } | ||
|
|
||
| // Restore after a brief delay | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { | ||
| button.image = originalImage | ||
| } |
There was a problem hiding this comment.
The new flashStatusBarIcon method lacks test coverage. The MenuBarControllerTests file contains tests for other UI methods like updateStatusIcon (lines 70-83), but there's no test for this new visual feedback functionality. Given that the repository has comprehensive test coverage for UI components, this method should have tests to verify it doesn't crash and handles edge cases like rapid clicks.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The menu item is a one-shot action (not a toggle), so there's no checkmark. Added a brief flash of the status bar icon to confirm the action was triggered.