fix: close tabs on middle-click over close overlay#8379
Conversation
The tab close affordance uses a wider hover overlay than the visible icon. That overlay intercepted middle-click mouseup events before they reach the tab label, so middle-click close sotpped working on the right side of the tab.
WalkthroughGradientCloseButton now intercepts middle-mouse down and up events, prevents the browser autoscroll behavior, stops propagation on mouse up, and invokes the optional click handler through the wrapper. ChangesMiddle-click close behavior
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js (1)
14-18: 🎯 Functional Correctness | 🔵 TrivialConfirm the
handleCloseClickimplementation remains agnostic to event type.The upstream handlers (
handleCloseClickinRequestTab/index.js,ExampleTab/index.js, etc.) correctly treat the received event as a trigger token rather than inspectinge.typeore.button. This makes the current implementation of forwarding themouseupevent safe.Ensure no future logic inside
handleCloseClickor its callees branches one.type === 'click'. The function should rely solely on the side effect (closing the tab) rather than the source event type.🤖 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 `@packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js` around lines 14 - 18, The close-button event flow should stay event-agnostic: `GradientCloseButton.handleMouseUp` forwards a `mouseup` event into `onClick`, and the upstream `handleCloseClick` handlers in `RequestTab/index.js`, `ExampleTab/index.js`, and similar should treat that event only as a trigger to close, not inspect `e.type` or `e.button`. Review `handleCloseClick` and any callees to remove or avoid branches based on the event source, keeping the behavior driven solely by the close action.
🤖 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
`@packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js`:
- Around line 23-27: The GradientCloseButton overlay behavior needs regression
coverage for middle-click on the close affordance area. Add a test around the
GradientCloseButton component that exercises the close overlay via middle mouse
button while the pointer is over the overlay and verifies the tab still closes;
reference the StyledWrapper handlers and handleMouseDown/handleMouseUp path so
the test stays aligned if the implementation moves.
---
Nitpick comments:
In
`@packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js`:
- Around line 14-18: The close-button event flow should stay event-agnostic:
`GradientCloseButton.handleMouseUp` forwards a `mouseup` event into `onClick`,
and the upstream `handleCloseClick` handlers in `RequestTab/index.js`,
`ExampleTab/index.js`, and similar should treat that event only as a trigger to
close, not inspect `e.type` or `e.button`. Review `handleCloseClick` and any
callees to remove or avoid branches based on the event source, keeping the
behavior driven solely by the close action.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 70737098-b8a6-41ce-bbd2-0de146966165
📒 Files selected for processing (1)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js
| <StyledWrapper | ||
| className={`close-gradient ${hasChanges ? 'has-changes' : ''}`} | ||
| onMouseDown={handleMouseDown} | ||
| onMouseUp={handleMouseUp} | ||
| > |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a regression test for middle-click over the close overlay.
This is a user-visible behavior fix, so it needs coverage proving middle-click still closes when the pointer is over the overlay area near the close affordance. As per coding guidelines, "Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created."
🤖 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
`@packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js`
around lines 23 - 27, The GradientCloseButton overlay behavior needs regression
coverage for middle-click on the close affordance area. Add a test around the
GradientCloseButton component that exercises the close overlay via middle mouse
button while the pointer is over the overlay and verifies the tab still closes;
reference the StyledWrapper handlers and handleMouseDown/handleMouseUp path so
the test stays aligned if the implementation moves.
Source: Coding guidelines
Description
The tab close affordance uses a wider hover overlay than the visible icon. That overlay intercepted middle-click mouseup events before they reach the tab label, so middle-click close sotpped working on the right side of the tab.
This fixes #8378
Contribution Checklist:
Summary by CodeRabbit