fix(photo-viewer): hide metadata sidepanel on mobile#1525
Merged
Conversation
…utton Fixes E2E test failures where PhotoMetadataSidepanel fixed bottom-sheet overlaid the photo viewer info bar on mobile, intercepting clicks on the annotate button. Changes: - Add toggle button (visible on mobile only) with ChevronUp icon that opens/closes the metadata sidepanel (aria-expanded, aria-controls) - Add isOpenMobile state (default: closed on mobile, always visible on desktop) - Sidepanel uses transform: translateY(100%) when closed to hide it without occupying space in layout - Toggle button z-index: 20 (above sidepanel z-index: 8), positioned bottom-right so it doesn't intercept clicks on the annotate button in the info bar - Add metadataToggle i18n key to photoViewer.json (English only) - Respect prefers-reduced-motion by disabling transform animations - Maintain desktop layout: sidepanel always visible, toggle button display: none The toggle button is 44x44 px (meets WCAG touch target minimum), uses primary color tokens, and includes proper focus indicators. Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
0bddaac to
75a0e00
Compare
…ift-snap test SVG <line> elements with y1 === y2 have a zero-height bounding box; Playwright's state:'visible' / toBeVisible() requires a non-zero rendered rect, so the check fails on CI even though the stroke is visible. Switching to state:'attached' validates shape commitment without depending on geometric height, and retains the y1/y2 attribute assertions for snap-angle verification. Fixes #1528 (Shift-snap line visibility false-negative on 2-vCPU CI shard) Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
…event touch interference On mobile, the metadata sidepanel toggle button (position: fixed, z-index: 20) was intercepting touch events on the annotation canvas during freehand drawing, causing strokes to fail. Hide both the toggle button and sidepanel by returning null from PhotoMetadataSidepanel when isAnnotating is true. Fixes failing E2E tests: - Freehand tool on mobile — pointer drag captures stroke - Measurement tool — inline input appears after drag on mobile/tablet - Multi-tool lifecycle — draw 3 shapes, save, view original, clear Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
…ng prop Update the renderSidepanel helper's type signature to accept the new isAnnotating prop that hides the sidepanel during annotation mode. Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
Move the early return-on-isAnnotating after all hook calls. React's Rules of Hooks require the same number of hook calls on every render, so the early return before useState/useEffect/useCallback was causing the entire PhotoViewer subtree to crash when entering annotation mode — resulting in 15+ failing photoAnnotation E2E tests across desktop, tablet, and mobile. Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
…asurement On WebKit viewports with hasTouch=true, page.mouse.* calls do not reliably propagate pointerdown/pointermove/pointerup to React's onPointer* handlers on SVG elements. Rewrite drawFreehandTouch() to dispatch synthetic PointerEvents directly via svgOverlay.evaluate(), and add a new drawLineTouch() helper for measurement/line drags. - drawFreehandTouch: replaces page.mouse.* with el.dispatchEvent(PointerEvent) for each segment, subdivided 3x to ensure FreehandTool accumulates ≥2 points through RDP simplification before COMMIT_DRAFT fires - drawLineTouch: new helper for two-point drags (measurement tool) with 5-step subdivision; used in Scenario 17 - Scenario 17 (measurement mobile): replace inline page.mouse.* with drawLineTouch - Scenario 21 (multi-tool lifecycle): replace drawFreehand with drawFreehandTouch so the freehand step works on iPhone 13 (WebKit/hasTouch) without breaking desktop No production code modified. Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.6) <noreply@anthropic.com>
…h helpers When all PointerEvents fire in a single synchronous JS task (one evaluate() call), React batches the SET_DRAFT update from pointerdown together with all pointermove updates. This means handlePointerMove sees stale state (draftShape === null) and returns early — so FreehandTool never captures intermediate points and COMMIT_DRAFT is skipped. Fix: split each helper into two evaluate() calls with a requestAnimationFrame yield between pointerdown and the pointermove/pointerup sequence. The rAF yield lets React flush the SET_DRAFT update so handlePointerMove sees a non-null draftShape in the second evaluate call. This resolves desktop Chromium failures introduced in the previous commit (which broke desktop by switching the multi-tool lifecycle test from page.mouse.* to drawFreehandTouch before the React batching issue was understood). Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.6) <noreply@anthropic.com>
…tate flush MeasurementTool reads state.draftShape.x2/y2 in onPointerUp (via React state) to compute the line length. Unlike FreehandTool (which uses module-level capturedPoints), the endpoint update arrives through React state from onPointerMove. If pointerup fires in the same synchronous batch as the pointermove events, React hasn't applied the final x2/y2 yet — onPointerUp reads x2=startX, distance=0, and discards the measurement. Add a second rAF yield between the pointermove batch and pointerup so React commits the final endpoint before onPointerUp executes. drawFreehandTouch remains two-phase (FreehandTool uses module-level capturedPoints, not React state, so the stale-state issue doesn't affect it). Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.6) <noreply@anthropic.com>
Capture the iPhone 13 / WebKit + React useReducer state-batching issue that surfaced while fixing the mobile photoAnnotation tests. Future runs of the e2e-test-engineer can reuse the multi-phase evaluate() pattern without re-deriving it. Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
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
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
Fixes E2E test failures where
PhotoMetadataSidepanelfixed bottom-sheet overlays the photo viewer info bar on mobile, intercepting pointer events on the annotate button.Changes
Toggle button: Visible on mobile only (44×44 touch target), opens/closes sidepanel with rotation animation
aria-expandedandaria-controlsfor accessibilitydata-testid="photo-metadata-toggle"for E2E selectionMobile behavior:
translateY(100%))Desktop behavior: No change — sidepanel always visible, toggle button hidden
Accessibility: Respects
prefers-reduced-motion(no transforms/animations when reduced motion is preferred)i18n: Added
metadataTogglekey toen/photoViewer.json(English only; translator handles non-English)Z-Index Hierarchy
The toggle button doesn't interfere with the annotate button because it's positioned in the bottom-right corner, away from the info bar.
Co-Authored-By: Claude frontend-developer (Haiku 4.5) noreply@anthropic.com