Conversation
🎭 Playwright Tests: ✅ PassedResults: 507 passed, 0 failed, 0 flaky, 8 skipped (Total: 515) 📊 Browser Reports
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/30/2026, 05:37:24 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 26 kB (baseline 26 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 974 kB (baseline 974 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 471 kB (baseline 471 kB) • 🟢 -8 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.89 kB (baseline 2.89 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 33.7 kB (baseline 33.7 kB) • ⚪ 0 BReusable component library chunks
Status: 4 added / 4 removed Data & Services — 2.71 MB (baseline 2.71 MB) • 🔴 +384 BStores, services, APIs, and repositories
Status: 8 added / 8 removed Utilities & Hooks — 25.3 kB (baseline 25.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 7 added / 7 removed Vendor & Third-Party — 10.7 MB (baseline 10.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.1 MB (baseline 7.1 MB) • 🟢 -132 BBundles that do not match a named category
Status: 34 added / 34 removed |
DrJKL
force-pushed
the
bl-visual-impala-358
branch
from
b32e777 to
02424d9
Compare
Member
Author
|
@DrJKL did you want to take over this? I can open if needed, or feel free to let me know if you opened your own PR I didn't open this because I wanted to add some tests for this since it touches a pretty hot path, but I tested manually and it works. |
## Summary Adds test coverage for the fix in #6597 (GitHub issue #6443). ## Changes Adds 2 new tests to `useCanvasInteractions.test.ts`: - **should forward wheel events to canvas when capture element is NOT focused** - verifies zoom works when hovering over unfocused inputs - **should NOT forward wheel events when capture element IS focused** - verifies focused text areas still capture scroll events ## Testing All 9 tests pass: ``` ✓ src/renderer/core/canvas/useCanvasInteractions.test.ts (9 tests) 9ms ``` ## Context PR #6597 by @benceruleanlu implements the fix but is marked as draft due to missing test coverage. This PR adds that coverage to help get the fix merged. Fixes #6443 (together with #6597) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8458-test-add-tests-for-focus-dependent-wheel-capture-behavior-2f86d73d365081ee937ed4c9f3f6532f) by [Unito](https://www.unito.io) Co-authored-by: Subagent 5 <subagent@example.com> Co-authored-by: Amp <amp@ampcode.com>
Contributor
📝 WalkthroughWalkthroughWheel event forwarding was made conditional: wheel events from elements marked data-capture-wheel are only forwarded to the canvas when they are not focused, or when in "standard" navigation mode with Ctrl/Meta held; focused capture elements block forwarding. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CaptureElement
participant Interactions as useCanvasInteractions
participant Settings as useSettingStore
participant Canvas
User->>CaptureElement: wheel event
CaptureElement->>Interactions: emit WheelEvent
Interactions->>Interactions: wheelCapturedByFocusedElement(event)?
alt wheel captured by focused element
Interactions->>Settings: get navigation mode
Settings-->>Interactions: "legacy" / "standard"
alt standard + Ctrl/Meta held
Interactions->>Canvas: forward wheel (preventDefault & stopPropagation)
else legacy or no modifier
Interactions-->>CaptureElement: do not forward
end
else not captured by focused element
Interactions->>Canvas: forward wheel (preventDefault & stopPropagation)
end
Suggested reviewers
✨ 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 |
Contributor
|
Did you want someone else to take this still? |
Member
Author
I got it |
dosubot
bot
added
the
size:L
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the two-finger panning/scroll UX over input widgets by forwarding wheel events to the canvas unless the widget element is actively focused. Previously, any element with data-capture-wheel="true" would always capture wheel events regardless of focus state.
Changes:
- Introduced
wheelCapturedByFocusedElement()to check both the presence of a capture element AND its focus state - Refactored
handleWheel()andforwardEventToCanvas()to respect the new focus-based capture logic - Added comprehensive test coverage for the new focus-based wheel event forwarding behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/renderer/core/canvas/useCanvasInteractions.ts | Implements focus-aware wheel event capture logic with new helper functions and updated event forwarding |
| src/renderer/core/canvas/useCanvasInteractions.test.ts | Adds test cases for focus-based wheel event forwarding in both legacy and standard navigation modes |
Comments suppressed due to low confidence (1)
src/renderer/core/canvas/useCanvasInteractions.ts:68
- There appears to be a logic issue in handleWheel for standard navigation mode. When an element with data-capture-wheel is NOT focused, the shouldForwardWheelEvent check passes (line 53), but then the event is not forwarded because it doesn't match either the Ctrl+wheel condition (line 56) or the legacy mode condition (line 62). This means regular wheel events over unfocused capture elements won't be forwarded to the canvas in standard mode, which contradicts the PR's goal of forwarding wheel events when the element is not focused. Consider adding logic to forward the event in standard mode when shouldForwardWheelEvent returns true but Ctrl/Meta is not pressed.
const handleWheel = (event: WheelEvent) => {
if (!shouldForwardWheelEvent(event)) return
// In standard mode, Ctrl+wheel should go to canvas for zoom
if (isStandardNavMode.value && (event.ctrlKey || event.metaKey)) {
forwardEventToCanvas(event)
return
}
// In legacy mode, all wheel events go to canvas for zoom
if (!isStandardNavMode.value) {
forwardEventToCanvas(event)
return
}
// Otherwise, let the component handle it normally
}
Comment on lines
+203
to
+224
| it('should forward ctrl+wheel to canvas when capture element IS focused in standard mode', () => { | ||
| const { get } = useSettingStore() | ||
| vi.mocked(get).mockReturnValue('standard') | ||
|
|
||
| const captureElement = document.createElement('div') | ||
| captureElement.setAttribute('data-capture-wheel', 'true') | ||
| const textarea = document.createElement('textarea') | ||
| captureElement.appendChild(textarea) | ||
| document.body.appendChild(captureElement) | ||
| textarea.focus() | ||
|
|
||
| const { handleWheel } = useCanvasInteractions() | ||
| const mockEvent = createMockWheelEvent(true) | ||
| Object.defineProperty(mockEvent, 'target', { value: textarea }) | ||
|
|
||
| handleWheel(mockEvent) | ||
|
|
||
| expect(mockEvent.preventDefault).toHaveBeenCalled() | ||
| expect(mockEvent.stopPropagation).toHaveBeenCalled() | ||
|
|
||
| document.body.removeChild(captureElement) | ||
| }) |
There was a problem hiding this comment.
The test only checks ctrlKey but the implementation also supports metaKey (for macOS Command key). Consider adding a test case for metaKey to ensure both keyboard modifiers work correctly on different platforms.
Comment on lines
+102
to
+103
| // Honor wheel capture only when the element is focused | ||
| if (event instanceof WheelEvent && !shouldForwardWheelEvent(event)) return |
There was a problem hiding this comment.
The forwardEventToCanvas function has new wheel event capture logic but lacks dedicated test coverage. While handleWheel tests indirectly exercise this path, consider adding explicit tests for forwardEventToCanvas to verify it properly respects the focus-based wheel capture when called directly (not just through handleWheel).
Comment on lines
+158
to
+224
| it('should forward wheel events to canvas when capture element is NOT focused', () => { | ||
| const { get } = useSettingStore() | ||
| vi.mocked(get).mockReturnValue('legacy') | ||
|
|
||
| const captureElement = document.createElement('div') | ||
| captureElement.setAttribute('data-capture-wheel', 'true') | ||
| const textarea = document.createElement('textarea') | ||
| captureElement.appendChild(textarea) | ||
| document.body.appendChild(captureElement) | ||
|
|
||
| const { handleWheel } = useCanvasInteractions() | ||
| const mockEvent = createMockWheelEvent() | ||
| Object.defineProperty(mockEvent, 'target', { value: textarea }) | ||
|
|
||
| handleWheel(mockEvent) | ||
|
|
||
| expect(mockEvent.preventDefault).toHaveBeenCalled() | ||
| expect(mockEvent.stopPropagation).toHaveBeenCalled() | ||
|
|
||
| document.body.removeChild(captureElement) | ||
| }) | ||
|
|
||
| it('should NOT forward wheel events when capture element IS focused', () => { | ||
| const { get } = useSettingStore() | ||
| vi.mocked(get).mockReturnValue('legacy') | ||
|
|
||
| const captureElement = document.createElement('div') | ||
| captureElement.setAttribute('data-capture-wheel', 'true') | ||
| const textarea = document.createElement('textarea') | ||
| captureElement.appendChild(textarea) | ||
| document.body.appendChild(captureElement) | ||
| textarea.focus() | ||
|
|
||
| const { handleWheel } = useCanvasInteractions() | ||
| const mockEvent = createMockWheelEvent() | ||
| Object.defineProperty(mockEvent, 'target', { value: textarea }) | ||
|
|
||
| handleWheel(mockEvent) | ||
|
|
||
| expect(mockEvent.preventDefault).not.toHaveBeenCalled() | ||
| expect(mockEvent.stopPropagation).not.toHaveBeenCalled() | ||
|
|
||
| document.body.removeChild(captureElement) | ||
| }) | ||
|
|
||
| it('should forward ctrl+wheel to canvas when capture element IS focused in standard mode', () => { | ||
| const { get } = useSettingStore() | ||
| vi.mocked(get).mockReturnValue('standard') | ||
|
|
||
| const captureElement = document.createElement('div') | ||
| captureElement.setAttribute('data-capture-wheel', 'true') | ||
| const textarea = document.createElement('textarea') | ||
| captureElement.appendChild(textarea) | ||
| document.body.appendChild(captureElement) | ||
| textarea.focus() | ||
|
|
||
| const { handleWheel } = useCanvasInteractions() | ||
| const mockEvent = createMockWheelEvent(true) | ||
| Object.defineProperty(mockEvent, 'target', { value: textarea }) | ||
|
|
||
| handleWheel(mockEvent) | ||
|
|
||
| expect(mockEvent.preventDefault).toHaveBeenCalled() | ||
| expect(mockEvent.stopPropagation).toHaveBeenCalled() | ||
|
|
||
| document.body.removeChild(captureElement) | ||
| }) |
There was a problem hiding this comment.
Missing test coverage for standard navigation mode with a regular wheel event (no Ctrl/Meta) when the capture element is NOT focused. This scenario should test whether the wheel event is properly forwarded to the canvas for panning, which is a key part of the PR's goal.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae84b22e43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+44
to
+46
| const shouldForwardWheelEvent = (event: WheelEvent): boolean => | ||
| !wheelCapturedByFocusedElement(event) || | ||
| (isStandardNavMode.value && (event.ctrlKey || event.metaKey)) |
There was a problem hiding this comment.
Because shouldForwardWheelEvent now returns true whenever the capture element is not focused, any data-capture-wheel element that can’t receive focus will always have its wheel events intercepted by the capture-phase listener (@wheel.capture="canvasInteractions.forwardEventToCanvas" in GraphCanvas.vue). That means the original target never sees the wheel event (preventDefault + stopPropagation happen in capture), so widgets that rely on wheel without focus will stop working. A concrete example is Load3DScene.vue, which marks the container with data-capture-wheel but doesn’t make it focusable; after this change, wheel zoom/pan in the 3D viewer will never fire unless the element is made focusable or the capture logic treats non-focusable elements as captured.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
I think it's actually incredible that ChatGPT was the one that came up with the best review comment here. This is actually something that's insane. I don't know how it managed to catch this one where like CodeRabbit and Copilot both missed it
Member
Author
There was a problem hiding this comment.
So basically the behavior I'm leaning towards right now is that the user would need to click on "Load3D Scene" for them to be able to pan it. I think that's perfectly reasonable because that's really the only solution that lets us let the user pan across the graph without having their pan be stopped in his tracks the second it manages to go over the "Load3D Scene" widget. I think that's reasonable and I made that change
Contributor
There was a problem hiding this comment.
Hmm, how does it feel to use? Maybe we can focus the 3D scene when clicking the node.
Member
Author
There was a problem hiding this comment.
It feels fine? Then deselection would open another discussion. And I think that complicates things by introducing widget specific exceptions to a global rule.
![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
🤖 Fix all issues with AI agents
In `@src/renderer/core/canvas/useCanvasInteractions.test.ts`:
- Around line 158-224: The tests in useCanvasInteractions.test.ts are creating
plain objects cast to WheelEvent so event instanceof WheelEvent is false and the
wheel-specific code path in useCanvasInteractions.handleWheel isn't exercised;
update the createMockWheelEvent helper to construct real WheelEvent instances
(e.g., new WheelEvent('wheel', { ctrlKey, metaKey })) and spy on
preventDefault/stopPropagation (or assert defaultPrevented/cancelBubble) so
handleWheel behavior is actually covered when calling
useCanvasInteractions().handleWheel.
In `@src/renderer/core/canvas/useCanvasInteractions.ts`:
- Around line 29-46: Convert the two helper function expressions to function
declarations: change the const wheelCapturedByFocusedElement = (event:
WheelEvent): boolean => { ... } into function
wheelCapturedByFocusedElement(event: WheelEvent): boolean { ... } and change the
const shouldForwardWheelEvent = (event: WheelEvent): boolean => ... into
function shouldForwardWheelEvent(event: WheelEvent): boolean { return ... } so
they follow the repo guideline for pure helpers while preserving existing
behavior and references.
Comment on lines
+158
to
+224
| it('should forward wheel events to canvas when capture element is NOT focused', () => { | ||
| const { get } = useSettingStore() | ||
| vi.mocked(get).mockReturnValue('legacy') | ||
|
|
||
| const captureElement = document.createElement('div') | ||
| captureElement.setAttribute('data-capture-wheel', 'true') | ||
| const textarea = document.createElement('textarea') | ||
| captureElement.appendChild(textarea) | ||
| document.body.appendChild(captureElement) | ||
|
|
||
| const { handleWheel } = useCanvasInteractions() | ||
| const mockEvent = createMockWheelEvent() | ||
| Object.defineProperty(mockEvent, 'target', { value: textarea }) | ||
|
|
||
| handleWheel(mockEvent) | ||
|
|
||
| expect(mockEvent.preventDefault).toHaveBeenCalled() | ||
| expect(mockEvent.stopPropagation).toHaveBeenCalled() | ||
|
|
||
| document.body.removeChild(captureElement) | ||
| }) | ||
|
|
||
| it('should NOT forward wheel events when capture element IS focused', () => { | ||
| const { get } = useSettingStore() | ||
| vi.mocked(get).mockReturnValue('legacy') | ||
|
|
||
| const captureElement = document.createElement('div') | ||
| captureElement.setAttribute('data-capture-wheel', 'true') | ||
| const textarea = document.createElement('textarea') | ||
| captureElement.appendChild(textarea) | ||
| document.body.appendChild(captureElement) | ||
| textarea.focus() | ||
|
|
||
| const { handleWheel } = useCanvasInteractions() | ||
| const mockEvent = createMockWheelEvent() | ||
| Object.defineProperty(mockEvent, 'target', { value: textarea }) | ||
|
|
||
| handleWheel(mockEvent) | ||
|
|
||
| expect(mockEvent.preventDefault).not.toHaveBeenCalled() | ||
| expect(mockEvent.stopPropagation).not.toHaveBeenCalled() | ||
|
|
||
| document.body.removeChild(captureElement) | ||
| }) | ||
|
|
||
| it('should forward ctrl+wheel to canvas when capture element IS focused in standard mode', () => { | ||
| const { get } = useSettingStore() | ||
| vi.mocked(get).mockReturnValue('standard') | ||
|
|
||
| const captureElement = document.createElement('div') | ||
| captureElement.setAttribute('data-capture-wheel', 'true') | ||
| const textarea = document.createElement('textarea') | ||
| captureElement.appendChild(textarea) | ||
| document.body.appendChild(captureElement) | ||
| textarea.focus() | ||
|
|
||
| const { handleWheel } = useCanvasInteractions() | ||
| const mockEvent = createMockWheelEvent(true) | ||
| Object.defineProperty(mockEvent, 'target', { value: textarea }) | ||
|
|
||
| handleWheel(mockEvent) | ||
|
|
||
| expect(mockEvent.preventDefault).toHaveBeenCalled() | ||
| expect(mockEvent.stopPropagation).toHaveBeenCalled() | ||
|
|
||
| document.body.removeChild(captureElement) | ||
| }) |
Contributor
There was a problem hiding this comment.
Use real WheelEvent instances so the instanceof paths are exercised.
The new tests use a plain object cast to WheelEvent, so event instanceof WheelEvent is false and the wheel-specific forwarding path isn’t actually exercised. Consider constructing real WheelEvent instances and spying on preventDefault/stopPropagation (or assert defaultPrevented/cancelBubble) to ensure the behavior is covered.
✅ Example helper update (uses real WheelEvent)
function createMockWheelEvent(ctrlKey = false, metaKey = false): WheelEvent {
const event = new WheelEvent('wheel', { ctrlKey, metaKey })
vi.spyOn(event, 'preventDefault')
vi.spyOn(event, 'stopPropagation')
return event
}Based on learnings: Aim for behavioral coverage of critical and new features.
🤖 Prompt for AI Agents
In `@src/renderer/core/canvas/useCanvasInteractions.test.ts` around lines 158 -
224, The tests in useCanvasInteractions.test.ts are creating plain objects cast
to WheelEvent so event instanceof WheelEvent is false and the wheel-specific
code path in useCanvasInteractions.handleWheel isn't exercised; update the
createMockWheelEvent helper to construct real WheelEvent instances (e.g., new
WheelEvent('wheel', { ctrlKey, metaKey })) and spy on
preventDefault/stopPropagation (or assert defaultPrevented/cancelBubble) so
handleWheel behavior is actually covered when calling
useCanvasInteractions().handleWheel.
Comment on lines
+29
to
+46
| /** | ||
| * Returns true if the wheel event target is inside an element that should | ||
| * capture wheel events AND that element (or a descendant) currently has focus. | ||
| * | ||
| * This allows two-finger panning to continue over inputs until the user has | ||
| * actively focused the widget, at which point the widget can consume scroll. | ||
| */ | ||
| const wheelCapturedByFocusedElement = (event: WheelEvent): boolean => { | ||
| const target = event.target as HTMLElement | null | ||
| const captureElement = target?.closest('[data-capture-wheel="true"]') | ||
| const active = document.activeElement as Element | null | ||
|
|
||
| return !!(captureElement && active && captureElement.contains(active)) | ||
| } | ||
|
|
||
| const shouldForwardWheelEvent = (event: WheelEvent): boolean => | ||
| !wheelCapturedByFocusedElement(event) || | ||
| (isStandardNavMode.value && (event.ctrlKey || event.metaKey)) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer function declarations for the new pure helpers.
Both helpers are pure and can be declared as functions to match the repo guideline.
[replacements below keep behavior unchanged]
♻️ Suggested refactor
- const wheelCapturedByFocusedElement = (event: WheelEvent): boolean => {
+ function wheelCapturedByFocusedElement(event: WheelEvent): boolean {
const target = event.target as HTMLElement | null
const captureElement = target?.closest('[data-capture-wheel="true"]')
const active = document.activeElement as Element | null
return !!(captureElement && active && captureElement.contains(active))
}
- const shouldForwardWheelEvent = (event: WheelEvent): boolean =>
- !wheelCapturedByFocusedElement(event) ||
- (isStandardNavMode.value && (event.ctrlKey || event.metaKey))
+ function shouldForwardWheelEvent(event: WheelEvent): boolean {
+ return (
+ !wheelCapturedByFocusedElement(event) ||
+ (isStandardNavMode.value && (event.ctrlKey || event.metaKey))
+ )
+ }As per coding guidelines: Do not use function expressions if it is possible to use function declarations instead.
📝 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.
Suggested change
| /** | |
| * Returns true if the wheel event target is inside an element that should | |
| * capture wheel events AND that element (or a descendant) currently has focus. | |
| * | |
| * This allows two-finger panning to continue over inputs until the user has | |
| * actively focused the widget, at which point the widget can consume scroll. | |
| */ | |
| const wheelCapturedByFocusedElement = (event: WheelEvent): boolean => { | |
| const target = event.target as HTMLElement | null | |
| const captureElement = target?.closest('[data-capture-wheel="true"]') | |
| const active = document.activeElement as Element | null | |
| return !!(captureElement && active && captureElement.contains(active)) | |
| } | |
| const shouldForwardWheelEvent = (event: WheelEvent): boolean => | |
| !wheelCapturedByFocusedElement(event) || | |
| (isStandardNavMode.value && (event.ctrlKey || event.metaKey)) | |
| /** | |
| * Returns true if the wheel event target is inside an element that should | |
| * capture wheel events AND that element (or a descendant) currently has focus. | |
| * | |
| * This allows two-finger panning to continue over inputs until the user has | |
| * actively focused the widget, at which point the widget can consume scroll. | |
| */ | |
| function wheelCapturedByFocusedElement(event: WheelEvent): boolean { | |
| const target = event.target as HTMLElement | null | |
| const captureElement = target?.closest('[data-capture-wheel="true"]') | |
| const active = document.activeElement as Element | null | |
| return !!(captureElement && active && captureElement.contains(active)) | |
| } | |
| function shouldForwardWheelEvent(event: WheelEvent): boolean { | |
| return ( | |
| !wheelCapturedByFocusedElement(event) || | |
| (isStandardNavMode.value && (event.ctrlKey || event.metaKey)) | |
| ) | |
| } |
🤖 Prompt for AI Agents
In `@src/renderer/core/canvas/useCanvasInteractions.ts` around lines 29 - 46,
Convert the two helper function expressions to function declarations: change the
const wheelCapturedByFocusedElement = (event: WheelEvent): boolean => { ... }
into function wheelCapturedByFocusedElement(event: WheelEvent): boolean { ... }
and change the const shouldForwardWheelEvent = (event: WheelEvent): boolean =>
... into function shouldForwardWheelEvent(event: WheelEvent): boolean { return
... } so they follow the repo guideline for pure helpers while preserving
existing behavior and references.
christian-byrne
approved these changes
Feb 1, 2026
jtydhr88
approved these changes
Feb 1, 2026
|
Related Documentation No published documentation to review for changes on this repository. Write your first living document How did I do? Any feedback? |
christian-byrne
added a commit
that referenced
this pull request
Feb 14, 2026
Remove `disabled` attribute from read-only preview textareas — disabled elements cannot receive focus, so the focus-gated wheel capture logic (#6597) always forwards wheel events to the canvas instead of allowing scroll. Add `data-capture-wheel` and `tabindex` to WidgetMarkdown display div so rendered markdown content can also capture wheel events when focused. Fixes COM-14812 Amp-Thread-ID: https://ampcode.com/threads/T-019c5a22-0e4c-7217-b022-49f9c86b8790
christian-byrne
added a commit
that referenced
this pull request
Feb 20, 2026
Remove `disabled` attribute from read-only preview textareas — disabled elements cannot receive focus, so the focus-gated wheel capture logic (#6597) always forwards wheel events to the canvas instead of allowing scroll. Add `data-capture-wheel` and `tabindex` to WidgetMarkdown display div so rendered markdown content can also capture wheel events when focused. Fixes COM-14812 Amp-Thread-ID: https://ampcode.com/threads/T-019c5a22-0e4c-7217-b022-49f9c86b8790
christian-byrne
added a commit
that referenced
this pull request
Feb 20, 2026
## Summary Restore mouse-wheel scrolling for read-only preview widgets (PreviewAny plaintext and markdown modes), broken by the focus-gated wheel capture in #6597. ## Changes - **What**: Remove `disabled` attribute from read-only textareas (keep `readonly`) so they can receive focus and capture wheel events. Add `data-capture-wheel` and `tabindex` to WidgetMarkdown display div. - **Root cause**: `disabled` elements cannot receive focus in browsers. The focus-gated `wheelCapturedByFocusedElement()` from #6597 always evaluated to false for disabled textareas, forwarding all wheel events to the canvas. ## Review Focus - Verify that removing `disabled` while keeping `readonly` does not allow unintended editing - Confirm `tabindex="0"` on the markdown display div does not cause unexpected tab-order issues - Ensure trackpad panning over unfocused widgets (#6523) still works correctly Fixes COM-14812 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8863-fix-restore-mouse-wheel-scrolling-in-preview-as-text-outputs-3076d73d365081719bf5e453235bb2b5) by [Unito](https://www.unito.io)
github-actions bot
pushed a commit
that referenced
this pull request
Feb 20, 2026
## Summary Restore mouse-wheel scrolling for read-only preview widgets (PreviewAny plaintext and markdown modes), broken by the focus-gated wheel capture in #6597. ## Changes - **What**: Remove `disabled` attribute from read-only textareas (keep `readonly`) so they can receive focus and capture wheel events. Add `data-capture-wheel` and `tabindex` to WidgetMarkdown display div. - **Root cause**: `disabled` elements cannot receive focus in browsers. The focus-gated `wheelCapturedByFocusedElement()` from #6597 always evaluated to false for disabled textareas, forwarding all wheel events to the canvas. ## Review Focus - Verify that removing `disabled` while keeping `readonly` does not allow unintended editing - Confirm `tabindex="0"` on the markdown display div does not cause unexpected tab-order issues - Ensure trackpad panning over unfocused widgets (#6523) still works correctly Fixes COM-14812 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8863-fix-restore-mouse-wheel-scrolling-in-preview-as-text-outputs-3076d73d365081719bf5e453235bb2b5) by [Unito](https://www.unito.io)
github-actions bot
pushed a commit
that referenced
this pull request
Feb 20, 2026
## Summary Restore mouse-wheel scrolling for read-only preview widgets (PreviewAny plaintext and markdown modes), broken by the focus-gated wheel capture in #6597. ## Changes - **What**: Remove `disabled` attribute from read-only textareas (keep `readonly`) so they can receive focus and capture wheel events. Add `data-capture-wheel` and `tabindex` to WidgetMarkdown display div. - **Root cause**: `disabled` elements cannot receive focus in browsers. The focus-gated `wheelCapturedByFocusedElement()` from #6597 always evaluated to false for disabled textareas, forwarding all wheel events to the canvas. ## Review Focus - Verify that removing `disabled` while keeping `readonly` does not allow unintended editing - Confirm `tabindex="0"` on the markdown display div does not cause unexpected tab-order issues - Ensure trackpad panning over unfocused widgets (#6523) still works correctly Fixes COM-14812 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8863-fix-restore-mouse-wheel-scrolling-in-preview-as-text-outputs-3076d73d365081719bf5e453235bb2b5) by [Unito](https://www.unito.io)
huntcsg
pushed a commit
that referenced
this pull request
Feb 21, 2026
## Summary Restore mouse-wheel scrolling for read-only preview widgets (PreviewAny plaintext and markdown modes), broken by the focus-gated wheel capture in #6597. ## Changes - **What**: Remove `disabled` attribute from read-only textareas (keep `readonly`) so they can receive focus and capture wheel events. Add `data-capture-wheel` and `tabindex` to WidgetMarkdown display div. - **Root cause**: `disabled` elements cannot receive focus in browsers. The focus-gated `wheelCapturedByFocusedElement()` from #6597 always evaluated to false for disabled textareas, forwarding all wheel events to the canvas. ## Review Focus - Verify that removing `disabled` while keeping `readonly` does not allow unintended editing - Confirm `tabindex="0"` on the markdown display div does not cause unexpected tab-order issues - Ensure trackpad panning over unfocused widgets (#6523) still works correctly Fixes COM-14812 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8863-fix-restore-mouse-wheel-scrolling-in-preview-as-text-outputs-3076d73d365081719bf5e453235bb2b5) by [Unito](https://www.unito.io)
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
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.
Summary
Forward wheel events to the canvas unless a wheel-capturing element is focused, and ensure the Load3D scene becomes focusable on pointer interaction so its wheel zoom/pan works after the user clicks into it.
Changes
Review Focus
Screenshots (if applicable)
N/A