fix: eliminate image flicker by gating opacity on fit-transform readiness#1370
fix: eliminate image flicker by gating opacity on fit-transform readiness#1370siddharthsai218 wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an ChangesisFitReady Flicker Fix
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/hooks/useZoomTransform.ts (1)
196-200: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winConsider a fallback for permanent invisibility when fit never succeeds.
If
applyFitTransformexhausts all retry frames without success (e.g., zero-size viewport, corrupted image with no dimensions),isFitReadystaysfalseand the image remains permanently invisible. The ResizeObserver provides a recovery path for viewport size changes, but not all scenarios are covered (e.g., no ResizeObserver support, or dimensions that never become available).Consider adding a fallback that sets
isFitReadytotrueafterMAX_FIT_RETRY_FRAMESexhausted, so the image is at least visible (even if not perfectly fit) rather than invisible.🛡️ Proposed fallback in scheduleFitTransform
fitFrameRef.current = scheduleFrame(() => { fitFrameRef.current = null; const applied = applyFitTransform(); if (!applied && fitRetryCountRef.current < MAX_FIT_RETRY_FRAMES) { fitRetryCountRef.current += 1; scheduleFitTransform(false); + } else if (!applied) { + // All retries exhausted; show image anyway to avoid permanent invisibility. + setIsFitReady(true); } });🤖 Prompt for 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. In `@frontend/src/hooks/useZoomTransform.ts` around lines 196 - 200, In useZoomTransform’s fit retry flow, `applyFitTransform` can leave `isFitReady` false forever when the viewport/image never becomes measurable, causing permanent invisibility. Update the retry handling around `scheduleFitTransform` and the `didApply` branch so that once `MAX_FIT_RETRY_FRAMES` is exhausted you still set `isFitReady` to true as a fallback, while keeping the existing successful-fit state updates in `isFitInitializedRef`, `hasUserInteractedRef`, and `fitRetryCountRef` intact.
🤖 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/ZoomableImage.tsx`:
- Around line 117-118: Add test coverage for the `isFitReady` opacity gate in
`ZoomableImage` by updating
`frontend/src/components/Media/__tests__/ZoomableImage.test.tsx` to verify the
image starts with `opacity: 0` and no transition before load, then changes to
`opacity: 1` with `transition: opacity 120ms ease-out` after the load flow
completes. Use the existing `ZoomableImage` render/load test setup and assert
against the rendered image style to cover this gating behavior.
---
Outside diff comments:
In `@frontend/src/hooks/useZoomTransform.ts`:
- Around line 196-200: In useZoomTransform’s fit retry flow, `applyFitTransform`
can leave `isFitReady` false forever when the viewport/image never becomes
measurable, causing permanent invisibility. Update the retry handling around
`scheduleFitTransform` and the `didApply` branch so that once
`MAX_FIT_RETRY_FRAMES` is exhausted you still set `isFitReady` to true as a
fallback, while keeping the existing successful-fit state updates in
`isFitInitializedRef`, `hasUserInteractedRef`, and `fitRetryCountRef` intact.
🪄 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: c899269a-99cb-4f70-9397-6f693d00e25c
📒 Files selected for processing (2)
frontend/src/components/Media/ZoomableImage.tsxfrontend/src/hooks/useZoomTransform.ts
1 participant
Addressed Issues:
Fixes #1366
Additional Notes:
Description
Fixes the image flicker at the top-left corner during slideshow/navigation transitions.
Root Cause
The image became visible before its fit/zoom transform had actually been applied. There were two gaps:
requestAnimationFrame, causing a visible flash at the untransformed (top-left) positionFix
Added an
isFitReadystate that only becomestrueonce the fit transform has actually been applied. Image opacity is now gated on this instead of justcontentDimensions, so the image stays hidden until it's fully positioned and scaled correctly — eliminating the top-left flicker.Changes
useZoomTransform.ts: addedisFitReadystate, set/reset logic, exposed from hookZoomableImage.tsx: gated image opacity/transition onisFitReadyMinimal, modular change — ~5 lines across 2 files, no unrelated modifications.
Testing
Confirmed the top-left flicker is eliminated across multiple transitions, rapid navigation, and slideshow mode.
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: Claude
Checklist
Summary by CodeRabbit
opacity: 0) until the initial fit-to-view step completes, then fade in toopacity: 1.opacity: 0with no opacity transition, then fade-in toopacity: 1withopacity 120ms ease-outafter image load.