✨ Implement video docking (minimize to corner) #14961
Conversation
src/static-template.js
Outdated
| const refs = map(); | ||
|
|
||
| if (root.hasAttribute('ref')) { |
There was a problem hiding this comment.
@jridgewell Added this as a convenience. The alternative would be to modify the result of htmlRefs when root + refs are meant to be referenced in the same object.
There was a problem hiding this comment.
You already have a reference to root, because you passed it in.
There was a problem hiding this comment.
Actually, nvm. I'm noticing that it was designed specifically not to set refs on root.
|
Love it! Will review when out of WIP! Thanks :) |
src/static-template.js
Outdated
| const refs = map(); | ||
|
|
||
| if (root.hasAttribute('ref')) { |
There was a problem hiding this comment.
You already have a reference to root, because you passed it in.
|
@jridgewell Yup, I had added that only as a convenience. Either way, removed. |
|
I think dragging shouldn't be enabled until the video is fully ducked because it causes some wired behavior if the user snaps the video to a different corner as it is docking. |
|
@wassgha Fixed. The demo is a little out of date. |
|
I was playing around with the demo page some. I found that if I selected the cheese slicing video and then scrolled back and forth rapidly with I was testing with Chrome 66 on a Mac, if it's relevant. |
|
@aghassemi PTAL |
src/service/video/docking.js
Outdated
| transitionDurationMs > 200 ? 'ease-in' : 'linear'; | ||
|
|
||
| const positioningStyles = { | ||
| 'width': px(width), |
There was a problem hiding this comment.
do we still need to set width/height given scale is being set now? would be nice to avoid if at all possible since it is causes relayout (although just relayout of video box) if not avoidable, let's add "will-change: width, height, transform" to the CSS of the docking container.
There was a problem hiding this comment.
Unfortunately yes, because we need to set the aspect ratio for each sized element.
Made a small optimization to set/reset dimensions only for first and last frames and added will-change.
|
|
||
| attribute: **`dock`** | ||
|
|
||
| **Experimental feature. Setting this attribute is not currently valid.** |
There was a problem hiding this comment.
so I assume the strategy is no experimental flag and using validation rules to keep this experimental until ready? (if so, thumbs up from me)
|
Is the demo page up to date? |
|
It does work but I am seeing two glitches, one that has to do with the chrome navigation bar showing/hiding on scroll. I remember working on this and not sure if there was a solution to it. The second bug is related to snapping (the video sometimes snaps to the middle of the screen or to the sides) |
|
Looks like this knocked 0.5KB off of v0.js. Nice! /cc @jridgewell |
Partial for #14061
Passed over some ideas/corner cases from original implementation (#14614, thanks @wassgha!), but defers to runtime APIs when possible and keeps less state for sanity/robustness.
Features
Demo
http://slicing-cheese.glitch.me
In follow-up PRs:
(in no particular order)
Don't show controls after dragging.(Done already.)Refresh on resize.(Done already.)Video should not switch positions when scrolling quickly past the component container.(Fixed.)