Fix production zoom scroll behavior#1295
Conversation
WalkthroughZoomableImage refactors wheel-zoom handling by introducing a ChangesDynamic Zoom Wheel Interceptor Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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
🤖 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 `@frontend/src/components/Media/__tests__/ZoomableImage.test.tsx`:
- Around line 97-127: Add two new tests in ZoomableImage.test.tsx alongside the
existing "stable container intercept" case: (1) an axis-dependent anchoring test
that sets up mockElementRect so only one axis overflows (e.g., width larger than
container but height fits), fires a wheel event and asserts mockSetTransform was
called with the expected anchor coordinates and scale change (use the same
pattern as the existing test referencing wheelArea, image, fireEvent.wheel, and
mockSetTransform), and (2) a recenter-on-min-scale test that starts at >min
scale, simulates a zoom-out to the minimum scale via wheel events, and asserts
that the transform was reset to the centered coordinates (relying on
ZoomableImage, mockElementRect, and mockSetTransform to verify recentering).
🪄 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: cf69a53e-2307-4afe-88f9-84a1a37efe16
📒 Files selected for processing (2)
frontend/src/components/Media/ZoomableImage.tsxfrontend/src/components/Media/__tests__/ZoomableImage.test.tsx
|
@VanshajPoonia can you create a test release in your fork for me to test the implementation? |
|
Sure, I created a test release in my fork: https://github.com/VanshajPoonia/PictoPy/releases/tag/v1.1.0-pr1295-test.1 The release is for testing the zoom on scroll implementation from this PR |
Fixes #1293
Screenshots/Recordings:
This PR addresses the zoom-on-scroll behavior described in issue #1293 and the linked reference issue #656.
Before:
After:
Additional Notes:
The regression was caused by the custom wheel listener depending on
react-zoom-pan-pinch's internalwrapperComponentbeing available during the first effect run. In production builds, that timing could fail, causing the viewer to use the library's default wheel behavior.This PR attaches the custom wheel listener to a stable container owned by
ZoomableImageand disables the library's default wheel handling. A focused regression test was added to verify the custom wheel behavior still works when the internal transform wrapper is not ready.Local verification completed:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: Codex
Checklist
Summary by CodeRabbit
Refactor
Tests