fix(detail): stop switch/load flicker — pin image height, letterbox blur, keep tone panel (on top of #535) - #536
Merged
Merged
Conversation
…letterbox blur, keep tone panel) Rebuilt on top of #535 (which removed the preview fade animation). #535 alone did not fix the dominant flicker because it kept `object-contain md:max-h-[90vh]` and didn't touch the info panel. This adds the remaining, verified fixes: - Image height: `<img>` (preview + high-res) `md:max-h-[90vh]` → `object-contain w-full sm:h-full`, and the ProgressiveImage root wrapper `relative` → `relative sm:h-full` so the height:100% chain resolves (container sm:h-[90vh] → viewport → slide → wrapper → img). The img element then stays a constant 90vh for every aspect ratio instead of resizing ~607↔1366px and overflowing on each switch. Verified live: injecting the wrapper height made the slide img go 607→810px. - Blur placeholder: `style={{ objectFit: 'contain' }}` on the preview image so next/image renders the blur with `background-size: contain` (it reads style.objectFit, not the className) — otherwise the blur filled the fixed box via `cover` and the real image shrank to letterbox on load = a load flash. - Tone analysis panel: `if (loading)` → `if (loading && !toneData)` so a switch keeps the previous tone values rendered instead of collapsing to a spinner and shifting the whole info panel's layout. Mirrors the histogram guard from #532 (which was never applied to tone analysis). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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
Rebuilt on top of #535 (which removed the preview fade animation). #535 alone didn't fix the dominant detail-view flicker because it kept
object-contain md:max-h-[90vh]and didn't touch the info panel. This adds the remaining verified fixes (supersedes #534, which was based on pre-#535 main and conflicted):Image height pinned —
<img>(preview + high-res)md:max-h-[90vh]→object-contain w-full sm:h-full, and the ProgressiveImage root wrapperrelative→relative sm:h-fullso theheight:100%chain resolves (containersm:h-[90vh]→ viewport → slide → wrapper → img). The img element stays a constant 90vh for every aspect instead of resizing ~607↔1366px and overflowing on each switch. Verified live: injecting the wrapper height made the slide img go 607 → 810px.Blur placeholder letterboxed —
style={{ objectFit: 'contain' }}on the preview image. next/image derives the blur'sbackground-sizefromstyle.objectFit(get-img-props.js:backgroundSize = imgStyle.objectFit), not the className — without it the blur filled the fixed box viacoverwhile the real image letterboxed smaller = a load flash (worst for portraits). Confirmed live (wasbackground-size: cover).Tone analysis kept on switch —
if (loading)→if (loading && !toneData). It returned a spinner unconditionally, so each switch collapsed the tone section and shifted the whole info panel's layout (rows below jumped up/back) = "info panel flashes on switch". Mirrors the histogram guard from fix(detail): stop the switch + zoom-load flicker #532, which was never applied to tone analysis.Not touched / not regressed
initial/animate/transitionre-added).createPortaland fix(detail): stop the switch + zoom-load flicker #532MemoWebGLImageViewerare intact (zoom-blank / zoom-load-flicker not regressed).Verification plan
Deploy this branch HEAD → I confirm deployed == HEAD on live (img constant 810 across aspects + tone doesn't collapse on switch) → Manjusaka real-device verifies switch + load → Impl walks the height chain + measures the panel on switch.