fix(toolbar): enable mouse wheel horizontal scroll#1246
Conversation
WalkthroughAdds a ref and test id to ToolbarContent to enable/use the wheel-to-horizontal scroll hook, and introduces a test verifying horizontal scrolling occurs on vertical wheel events via mocked dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant ToolbarContent
participant useWheelHorizontalScroll
participant ContainerDiv
User->>Browser: Wheel event (deltaY)
Browser->>ContainerDiv: dispatch wheel
ContainerDiv->>useWheelHorizontalScroll: event handled via ref
useWheelHorizontalScroll->>ContainerDiv: set scrollLeft (> 0)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Bundle size diff
|
|
Preview environments for this PR:
These previews are automatically updated with each commit. Note: After a new deployment, it may take a few minutes for the changes to propagate and for caches to update. During this time, you might experience temporary loading issues or see an older version of the app. If the app fails to load, please wait a few minutes and try again. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1246 +/- ##
==========================================
+ Coverage 12.03% 13.15% +1.12%
==========================================
Files 724 724
Lines 29798 29796 -2
Branches 1398 1387 -11
==========================================
+ Hits 3585 3921 +336
+ Misses 25657 25342 -315
+ Partials 556 533 -23
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 33 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: konturio/disaster-ninja-fe-esmAssets Changed:
Files in
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/features/toolbar/components/ToolbarContent/ToolbarContent.test.tsx(1 hunks)src/features/toolbar/components/ToolbarContent/ToolbarContent.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/features/toolbar/components/ToolbarContent/ToolbarContent.test.tsx (1)
src/features/toolbar/components/ToolbarContent/ToolbarContent.tsx (1)
ToolbarContent(8-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: deploy
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
src/features/toolbar/components/ToolbarContent/ToolbarContent.tsx (1)
14-14: Good wiring: ref + hook correctly enable wheel-to-horizontal scroll.The ref is attached to the correct container, and the hook is invoked with it. This should unlock horizontal scrolling as intended.
src/features/toolbar/components/ToolbarContent/ToolbarContent.test.tsx (1)
8-20: Solid isolation via useToolbarContent mock.The minimal mock for useToolbarContent keeps the test focused and deterministic.
| vi.mock('../ToolbarControl/ToolbarControl', () => ({ | ||
| ToolbarControl: ({ id }: { id: string }) => ( | ||
| <div data-testid={id} style={{ width: '300px' }} /> | ||
| ), | ||
| })); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove redundant inline width styling on mocked ToolbarControl.
You already force scrollable dimensions via scrollWidth/clientWidth. The inline width on the mock adds noise and could mislead future maintainers.
Apply this diff:
- ToolbarControl: ({ id }: { id: string }) => (
- <div data-testid={id} style={{ width: '300px' }} />
- ),
+ ToolbarControl: ({ id }: { id: string }) => <div data-testid={id} />,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| vi.mock('../ToolbarControl/ToolbarControl', () => ({ | |
| ToolbarControl: ({ id }: { id: string }) => ( | |
| <div data-testid={id} style={{ width: '300px' }} /> | |
| ), | |
| })); | |
| vi.mock('../ToolbarControl/ToolbarControl', () => ({ | |
| ToolbarControl: ({ id }: { id: string }) => <div data-testid={id} />, | |
| })); |
🤖 Prompt for AI Agents
In src/features/toolbar/components/ToolbarContent/ToolbarContent.test.tsx around
lines 22 to 26, the mocked ToolbarControl includes an unnecessary inline style
width which is redundant because tests override scrollWidth/clientWidth; remove
the style attribute from the mock so it becomes simply <div data-testid={id} />
to avoid misleading future maintainers and let test-controlled dimensions drive
scrolling behavior.
| vi.mock('@konturio/default-icons', () => ({})); | ||
| vi.mock('@konturio/default-icons/tslib/index.js', () => ({})); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm necessity of icon mocks; drop if unused.
If ToolbarContent + mocked ToolbarButton don’t transitively import @konturio/default-icons, these mocks can be removed to reduce test setup noise.
Run this to locate imports and confirm whether these mocks are actually hit in this test:
🏁 Script executed:
#!/bin/bash
# Check if ToolbarContent or its mocked children import the icons.
rg -n "@konturio/default-icons" -A 2 -B 2Length of output: 20260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- show test file ----"
sed -n '1,240p' src/features/toolbar/components/ToolbarContent/ToolbarContent.test.tsx || true
echo
echo "---- show ToolbarContent component ----"
sed -n '1,240p' src/features/toolbar/components/ToolbarContent/ToolbarContent.tsx || true
echo
echo "---- show ToolbarIcon ----"
sed -n '1,240p' src/features/toolbar/components/ToolbarIcon.tsx || true
echo
echo "---- list files in toolbar/components ----"
ls -la src/features/toolbar/components || true
echo
echo "---- show ToolbarButton if exists ----"
sed -n '1,240p' src/features/toolbar/components/ToolbarButton.tsx || true || trueLength of output: 4246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- locate useWheelHorizontalScroll ----"
rg -n "useWheelHorizontalScroll" || true
echo
echo "---- show hook file ----"
sed -n '1,240p' src/utils/hooks/useWheelHorizontalScroll.ts || sed -n '1,240p' src/utils/hooks/useWheelHorizontalScroll.tsx || true
echo
echo "---- list ToolbarControl files ----"
ls -la src/features/toolbar/components/ToolbarControl || true
echo
echo "---- show ToolbarControl implementation(s) ----"
sed -n '1,240p' src/features/toolbar/components/ToolbarControl/ToolbarControl.tsx || sed -n '1,240p' src/features/toolbar/components/ToolbarControl/index.tsx || true
echo
echo "---- list ToolbarButton files ----"
ls -la src/features/toolbar/components/ToolbarButton || true
echo
echo "---- show ToolbarButton implementation(s) ----"
sed -n '1,240p' src/features/toolbar/components/ToolbarButton/ToolbarButton.tsx || sed -n '1,240p' src/features/toolbar/components/ToolbarButton/index.tsx || trueLength of output: 6123
Remove unused @konturio/default-icons mocks from this test
The test already mocks ToolbarControl and ToolbarButton (and useToolbarContent), so importing ToolbarContent won't pull in ToolbarIcon/@konturio/default-icons — the two vi.mock calls are redundant and can be removed.
- File to edit:
- src/features/toolbar/components/ToolbarContent/ToolbarContent.test.tsx — remove the two mock lines (currently lines 32–33)
Remove these lines:
vi.mock('@konturio/default-icons', () => ({}));
vi.mock('@konturio/default-icons/tslib/index.js', () => ({}));
🤖 Prompt for AI Agents
In src/features/toolbar/components/ToolbarContent/ToolbarContent.test.tsx around
lines 32–33, remove the two redundant mock lines that import
@konturio/default-icons (vi.mock('@konturio/default-icons', () => ({})); and
vi.mock('@konturio/default-icons/tslib/index.js', () => ({}))); simply delete
those two lines so the test relies on the existing
ToolbarControl/ToolbarButton/useToolbarContent mocks.
| test('scrollLeft changes on vertical wheel events', async () => { | ||
| const { ToolbarContent } = await import('./ToolbarContent'); | ||
| const { getByTestId } = render(<ToolbarContent />); | ||
| const scroll = getByTestId('toolbar-content'); | ||
|
|
||
| Object.defineProperty(scroll, 'scrollWidth', { value: 300, configurable: true }); | ||
| Object.defineProperty(scroll, 'clientWidth', { value: 100, configurable: true }); | ||
|
|
||
| scroll.dispatchEvent(new WheelEvent('wheel', { deltaY: 50, bubbles: true })); | ||
|
|
||
| expect( | ||
| (scroll as HTMLDivElement).scrollLeft > 0, | ||
| `scrollLeft should change after wheel event, got ${(scroll as HTMLDivElement).scrollLeft}`, | ||
| ).toBe(true); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Tighten assertions and make the wheel event cancelable.
- Prefer toBeGreaterThan over boolean assertions for clarity.
- Make the WheelEvent cancelable to align with typical implementations that call preventDefault.
Apply this diff:
- scroll.dispatchEvent(new WheelEvent('wheel', { deltaY: 50, bubbles: true }));
+ scroll.dispatchEvent(new WheelEvent('wheel', { deltaY: 50, bubbles: true, cancelable: true }));
-
- expect(
- (scroll as HTMLDivElement).scrollLeft > 0,
- `scrollLeft should change after wheel event, got ${(scroll as HTMLDivElement).scrollLeft}`,
- ).toBe(true);
+ expect((scroll as HTMLDivElement).scrollLeft).toBeGreaterThan(0);Optional follow-ups:
- Add a precondition check to assert initial scrollLeft is 0, then assert it grows.
- Add a complementary test asserting no scroll occurs when scrollWidth <= clientWidth.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('scrollLeft changes on vertical wheel events', async () => { | |
| const { ToolbarContent } = await import('./ToolbarContent'); | |
| const { getByTestId } = render(<ToolbarContent />); | |
| const scroll = getByTestId('toolbar-content'); | |
| Object.defineProperty(scroll, 'scrollWidth', { value: 300, configurable: true }); | |
| Object.defineProperty(scroll, 'clientWidth', { value: 100, configurable: true }); | |
| scroll.dispatchEvent(new WheelEvent('wheel', { deltaY: 50, bubbles: true })); | |
| expect( | |
| (scroll as HTMLDivElement).scrollLeft > 0, | |
| `scrollLeft should change after wheel event, got ${(scroll as HTMLDivElement).scrollLeft}`, | |
| ).toBe(true); | |
| }); | |
| test('scrollLeft changes on vertical wheel events', async () => { | |
| const { ToolbarContent } = await import('./ToolbarContent'); | |
| const { getByTestId } = render(<ToolbarContent />); | |
| const scroll = getByTestId('toolbar-content'); | |
| Object.defineProperty(scroll, 'scrollWidth', { value: 300, configurable: true }); | |
| Object.defineProperty(scroll, 'clientWidth', { value: 100, configurable: true }); | |
| scroll.dispatchEvent(new WheelEvent('wheel', { deltaY: 50, bubbles: true, cancelable: true })); | |
| expect((scroll as HTMLDivElement).scrollLeft).toBeGreaterThan(0); | |
| }); |
🤖 Prompt for AI Agents
In src/features/toolbar/components/ToolbarContent/ToolbarContent.test.tsx around
lines 35 to 49, tighten the assertion and make the simulated wheel event
cancelable: replace the boolean assertion that checks scrollLeft > 0 with an
explicit expect((scroll as HTMLDivElement).scrollLeft).toBeGreaterThan(0) to
provide clearer failure messages, and construct the WheelEvent with the
cancelable flag (e.g., new WheelEvent('wheel', { deltaY: 50, bubbles: true,
cancelable: true })) so it matches typical implementations that call
preventDefault.
|
Komzpa
deleted the
codex/fix-toolbar-horizontal-scroll-mouse-wheel-issue
branch
2 participants
Summary
Testing
make precommit(fails: No rule to make target 'precommit')pnpm lintpnpm vitest runhttps://kontur.fibery.io/Tasks/Task/21890
https://chatgpt.com/codex/tasks/task_e_689e29c8f218832fb45b1840f7116992
Summary by CodeRabbit