fix: BROS-220: Fix resize handling in Video tag #8038
Conversation
Refactored `Frames.tsx` and `Minimap.tsx` to use the newly added `useResizeObserver` hook for timely width calculations. Added memoized references and hooks to have relevant callbacks. Introduced a new debounced `useResizeObserver` hook in the core library.
Gondragos
requested review from
a team,
hlomzik and
nick-skriabin
as code owners
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
✅ Deploy Preview for label-studio-playground ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for heartex-docs canceled.
|
✅ Deploy Preview for label-studio-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
yyassi-heartex
left a comment
yyassi-heartex
left a comment
There was a problem hiding this comment.
About time we got a hook for resize observer! great work!
|
/git merge
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8038 +/- ##
===========================================
- Coverage 70.21% 65.88% -4.33%
===========================================
Files 715 505 -210
Lines 50325 33493 -16832
Branches 8600 8621 +21
===========================================
- Hits 35334 22068 -13266
+ Misses 14988 11422 -3566
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| entries: ResizeObserverEntry[], | ||
| ): T => { | ||
| if (entries.length === 0) return {} as T; | ||
| const { width, height } = elements[0].getBoundingClientRect(); |
There was a problem hiding this comment.
why methods are different for 1 and for many? could you add a link to MDN or another resource?
There was a problem hiding this comment.
With one element we get in entries exactly the element I expect to get, and it already has the right sizes.
With more than one element it provides only entries that exactly had changes. So it's harder to get the first element size if there is no guarantee that the first entry will be related to the first element.
| } | ||
| }; | ||
| const handleAnimationFrameRef = useRef(handleAnimationFrame); | ||
| handleAnimationFrameRef.current = handleAnimationFrame; |
There was a problem hiding this comment.
this looks so confusing... are there other ways to have recursion in functional components?
There was a problem hiding this comment.
Recursion works fine. It just can't use the right values inside the function without moving them to ref if they are changing during execution.
It's the same trick as when we move handlers from props to ref to decrease the necessity to invalidate caches of useMemo/useCallback.
Co-authored-by: Andrew <[email protected]>
Refactored
Frames.tsxandMinimap.tsxto use the newly addeduseResizeObserverhook for timely width calculations. Added memoized references and hooks to have relevant callbacks. Introduced a new debounceduseResizeObserverhook in the core library.Before:
video_resize.mov
timeline_resize.mov
playing_zoom.mov