fix(desktop): make lightbox zoom controls interactive - #6710
Conversation
ac47987 to
57f76e6
Compare
Replace decorative lightbox zoom icons with accessible buttons wired to the existing clamped zoom state. Keep controls from closing the dialog, including SVG icon targets, and cover the regression in the gallery E2E spec. Signed-off-by: sanic <c57bf9b4275088b2b33db7f746975407210f159fbd8bd733c0e532375f69aa80@buzz.block.builderlab.xyz> Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
57f76e6 to
d3484c9
Compare
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Combined review — two independent source reviews plus live headless E2E, all at head 57f76e6. No blocking findings.
MINOR — repeated 5% steps miss the displayed endpoint by one click due to floating-point accumulation. The button handlers add/subtract IMAGE_LIGHTBOX_ZOOM_STEP and rely on updateZoom → clampImageLightboxZoom, which clamps but never rounds. 40 Zoom in clicks from 1 yield 2.9999999999999973: the label renders 300% while disabled={zoom >= IMAGE_LIGHTBOX_MAX_ZOOM} stays false until click 41; the mirror case holds for Zoom out from an exact 3. Fix by snapping stepped values to the 5% grid before clamping (integer-percent math is simplest); that also lets the exact-endpoint disabled state be asserted after exactly 40 steps in the spec.
Everything else checked out under live E2E: buttons step exactly 5% without dismissing the dialog, direct clicks on nested SVG descendants work (the Element widening in the capture guards is the right call — SVG targets are not HTMLElement), keyboard Enter/ArrowRight behave, 100-click runs clamp and disable at both bounds, and tab order skips the disabled button. The extraction preserves slider behavior mechanically and keeps markdown.tsx under the size gate.
Cross-PR: textually conflicts with #6705 in desktop/src/shared/ui/markdown.tsx; whichever lands second must keep both ImageLightboxZoomControls and #6705's ImageGalleryStatus/openThreadImageGallery, then rerun the gallery smoke row.
Scale the stored zoom offset by the zoom ratio in imageLightboxZoomStateAtZoom so the image point at the frame center stays anchored when the slider or wheel changes zoom, instead of leaving a fixed pixel offset that drifted and exposed empty frame gutter. Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
Summary
markdown.tsxremains below the repository file-size threshold.Testing
pnpm build:e2epnpm exec playwright test image-attachment-gallery.spec.ts --project=smoke— 10 passedpnpm typecheckgit diff --checkUI evidence
The rendered lightbox flow is covered by the gallery smoke test: Zoom out is disabled at 100%, Zoom in changes the value to 105%, Zoom out returns it to 100%, and the dialog remains open throughout. Existing gallery, keyboard, spoiler, context-menu, and close-path assertions also pass.