fix: handle middle mouse button click - #58
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds middle-click tracking by handling the browser's Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Plugin
participant PlausibleAPI
rect rgba(200,200,255,0.5)
User->>Browser: Click link (left or middle)
Browser->>Plugin: dispatch 'click' or 'auxclick' event
Plugin->>Plugin: check MouseEvent.button (0 or 1) & optional predicate
alt allowed
Plugin->>PlausibleAPI: POST /api/event (anchor text + props)
PlausibleAPI-->>Plugin: 200 OK
else ignored
Plugin-->>Browser: no action
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Comment |
|
@CodeRabbit could you generate the minimal test file for related changes. Only minimal unit test is needed. One-two test cases. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1230 🏁 Script executed: Length of output: 1872 🏁 Script executed: Length of output: 4624
// `@vitest-environment` jsdom
import { Plausible } from '../Plausible';
import { enableLinkClicksCapture } from './enableLinkClicksCapture';
const mockFetch = vi.fn();
globalThis.fetch = mockFetch;
const locationSpyon = vi.spyOn(window, 'location', 'get');
beforeEach(() => {
vi.clearAllMocks();
locationSpyon.mockReturnValue(new URL('https://example.org') as any as Location);
mockFetch.mockReturnValue(new Response('ok'));
document.body.innerHTML = `<a id="link" href="https://example.com">click me</a>`;
});
const plausible = new Plausible({
apiHost: 'https://plausible.io',
domain: 'example.org',
});
enableLinkClicksCapture(plausible);
test('Ignore right-click (button=2) on a link', () => {
const anchorElm = document.querySelector('a#link') as HTMLAnchorElement;
expect(anchorElm).toBeInstanceOf(HTMLAnchorElement);
anchorElm.dispatchEvent(new MouseEvent('click', { button: 2, bubbles: true }));
expect(mockFetch).not.toHaveBeenCalled();
});
test('Capture middle mouse button click via auxclick event', () => {
const anchorElm = document.querySelector('a#link') as HTMLAnchorElement;
expect(anchorElm).toBeInstanceOf(HTMLAnchorElement);
anchorElm.dispatchEvent(new MouseEvent('auxclick', { button: 1, bubbles: true }));
expect(mockFetch).toHaveBeenCalledTimes(1);
expect(mockFetch).toHaveBeenCalledWith(
'https://plausible.io/api/event',
expect.objectContaining({
method: 'POST',
}),
);
});Two test cases:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/plugins/enableLinkClicksCapture.ts`:
- Around line 55-60: The auxclick listener is registered in the bubbling phase
while click uses capture, causing asymmetric behavior; update the
addEventListener and removeEventListener calls for 'auxclick' to use the same
options object ({ capture: true }) as 'click' so that clickCallback is invoked
in the capture phase for both events (refer to the clickCallback registration
and the cleanup removal function).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b685a64c-6fa7-49a9-ad36-3e40fd08571a
📒 Files selected for processing (1)
src/plugins/enableLinkClicksCapture.ts
Fixes #27
Related to #57
Summary by CodeRabbit