Repository navigation
feat(ui): open videos like images and zoom into both - #57924
Conversation
There was a problem hiding this comment.
In Chromium this works as described. The new component test (first Escape zooms back to fit, second closes) and the updated Review-pane video e2e test both pass: the video moves into the preview and comes back as the same element. The image-zoom unit tests and the type checks of packages/ui and packages/gui-extensions pass too. I didn't try the video zoom gestures by hand.
Public API: unchanged. This is UI-internal; the only removal is the app's own view.exitFullscreen i18n string.
Two things worth fixing, both inline:
- Borrowing the video calls
Element.moveBeforewith no fallback. Where the web app runs in a browser without it, the preview gets stuck invisible.object-view-boxis Chromium-only as well. - The capture-phase Space handler takes Space away from the preview's focused buttons.
| borderRadius: getComputedStyle(video).borderTopLeftRadius, | ||
| }) | ||
| video.before(holder) | ||
| root.moveBefore(video, null) |
There was a problem hiding this comment.
moveBefore is Chromium-only for now. That's fine in Electron, but the web app can be opened in browsers without it. There this call throws inside reveal() after holder has already gone into the pane. The preview stays in pending: an open, invisible modal over the page, with a stray placeholder in the pane and the full-screen button hidden (open() stays true). Could ArtifactVideo feature-detect moveBefore (and CSS.supports("object-view-box", "none"), since otherwise the zoom pill changes but the video doesn't zoom)? Where they're missing, it could skip the preview or fall back to the old behaviour.
| const action = shortcut(event.key) | ||
| const bare = !event.metaKey && !event.ctrlKey && !event.altKey | ||
|
|
||
| if (bare && event.key === " " && props.video && holder) { |
There was a problem hiding this comment.
This listens in capture on window, so while a video is open Space always toggles playback. Keyboard users can then no longer press the focused close or zoom buttons with Space, and the close button gets focus when the preview opens. Maybe skip this when event.target is a button (or anything other than the video or the preview body)?
…ng layout per glide frame
Kit was watching a screen recording in the Review pane and wanted to zoom into it with ⌘-scroll, and wanted its window-filling view to animate open like images do.
What changed
<video>(moveBefore) and returns it when the close lands, so playback, time and controls carry straight through. A placeholder of its size holds its place in the pane.0, or a first Escape, glides back to fit. The existing level pill shows the zoom.object-view-box, so its native controls stay put and keep working (play/pause, scrub, mute). Space plays and pauses. A drag in the bottom 56 px is left to the scrubber.motion'sanimate, so a new target mid-glide keeps its velocity.data-expanded) and its now-unused "Exit full screen" string.Not done: ⌘-scroll on the inline pane video itself. Opening the preview is the way to zoom.
Proof
Fictional test-pattern video in the mocked app's Review pane, and the storybook poster image.
Video, real time:
video-1x.mp4
Video, 0.25×:
video-025x.mp4
Image, real time:
image-1x.mp4
Image, 0.25×:
image-025x.mp4
Tests
image-zoom.test.ts:viewBoxmath.component-tests/image-preview.spec.ts: the first Escape zooms back to fit and the second one closes.e2e/regression/review.spec.ts: the video opens into the preview and comes back to the pane as the same element.