Skip to content

review(morph): morph Flickable non-interactive, proxy wheel scroll#583

Merged
fernandotonon merged 1 commit into
masterfrom
fix/morph-flickable-noninteractive
May 17, 2026
Merged

review(morph): morph Flickable non-interactive, proxy wheel scroll#583
fernandotonon merged 1 commit into
masterfrom
fix/morph-flickable-noninteractive

Conversation

@fernandotonon
Copy link
Copy Markdown
Owner

@fernandotonon fernandotonon commented May 17, 2026 •
edited by coderabbitai Bot
Loading

Codex follow-up on PR #582 (merged):

The new Flickable in the morph band defaulted to interactive: true, which consumed left-button drags in that region. timelineArea (the root marquee-selection MouseArea) expects empty-row drags to fall through to it — and they did before the Flickable was added. Starting a marquee drag over morph lanes was therefore silently dropped.

Fix: match rowsView's interactive: false so left-drags pass through. Wheel scrolling still works because scrollByPixels (invoked by the root WheelHandler) now also drives the morph list's contentY when its content overflows — same dy as the bone list so one wheel notch advances both consistently.

Test plan

  • CI green
  • Manual: drag left-click over morph lanes → marquee selection on bone keys still activates.
  • Manual: high-blendshape character → wheel scrolls the morph list when it overflows.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed wheel scrolling in the dope sheet to properly scroll both the bone row list and morph-target band (when visible and overflowing).
    • Improved interaction handling for morph-target selections to ensure smooth mouse drag operations within the animation timeline.

Review Change Stack

@fernandotonon @claude
review(morph): morph Flickable non-interactive, proxy wheel scroll
7cd35c8 
Codex follow-up on PR #582 (merged):

The new Flickable in the morph band defaulted to `interactive: true`,
which consumed left-button drags in that region. `timelineArea` (the
root marquee-selection MouseArea) expects empty-row drags to fall
through to it — and they did before the Flickable was added.
Starting a marquee drag over morph lanes was therefore silently
dropped.

Fix: match `rowsView`'s `interactive: false` so left-drags pass
through. Wheel scrolling still works because `scrollByPixels`
(invoked by the root WheelHandler) now also drives the morph list's
`contentY` when its content overflows — same dy as the bone list so
one wheel notch advances both consistently.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ee4d9071-7db7-45bf-8ede-892c6a6a4076

📥 Commits

Reviewing files that changed from the base of the PR and between be84c10 and 7cd35c8.

📒 Files selected for processing (1)
  • qml/AnimationDopeSheet.qml
📝 Walkthrough

Walkthrough

Wheel scrolling in the animation dope sheet now scrolls both the bone row list and the morph-target band through a unified scroll helper, with the morph band disabled for independent scrolling to delegate all wheel and drag interactions to the root timeline's handling.

Changes

Unified wheel scrolling for bone rows and morph band

Layer / File(s) Summary
Unified scroll helper and morph band configuration
qml/AnimationDopeSheet.qml		 The root.scrollByPixels(dy) helper conditionally updates both rowsView.contentY and morphList.contentY based on visibility and overflow state. The morphList Flickable's interactive property is set to false so that marquee drags and wheel events route through the root timeline's unified scroll behavior rather than Flickable's own handlers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • fernandotonon/QtMeshEditor#576: Introduces the morph-target band (morphList) in the dope sheet; this PR refines its scroll and interaction behavior.

Poem

🐰 Wheel-scroll unity prevails,
Bone rows and morphs in tandem sails,
No dual interaction fray,
Just harmonious scroll's ballet! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main changes: setting the morph Flickable to non-interactive and proxying wheel scroll events.
Description check ✅ Passed The description covers the problem context, the solution approach, and includes a test plan with manual verification steps. While not following the template sections exactly, it provides comprehensive technical details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/morph-flickable-noninteractive

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 17, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@fernandotonon fernandotonon merged commit 1b73e53 into master May 17, 2026
20 checks passed
@fernandotonon fernandotonon deleted the fix/morph-flickable-noninteractive branch May 17, 2026 22:14
@fernandotonon fernandotonon mentioned this pull request May 17, 2026
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@fernandotonon