[Bugfix] Fix video tile strict mode findDOMNode warning#2621
[Bugfix] Fix video tile strict mode findDOMNode warning#2621dmceachernmsft merged 7 commits intomainfrom
Conversation
dmceachernmsft
requested review from
alkwa-msft,
anjulgarg,
carocao-msft and
edwardlee-msft
as code owners
dmceachernmsft
requested review from
JamesBurnside,
PorterNan,
mgamis-msft and
prabhjot-msft
as code owners
CallWithChat bundle size is increased❗.
|
Calling bundle size is increased❗.
|
Chat bundle size is increased❗.
|
|
Failed to pass the composite UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot. |
|
Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot. |
|
Failed to pass the composite UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot. |
|
Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot. |
| )} | ||
| {...longPressHandlersTrampoline} | ||
| > | ||
| <div ref={videoTileRef} style={{ width: '100%', height: '100%' }}> |
There was a problem hiding this comment.
Nice, we should do get rid of dependencies of northstar more and more
Btw, we do we not wrap the Stack element anymore?
There was a problem hiding this comment.
so the stack that is wrapping this div is what is setting the height for the tile. I moved the div that is setting the ref inside so that the tile renders as normal. For some reason when the div was outside it would break the video tile and it wouldn't have a height being passed to the reference. When I moved the div inside the stack the reference suddenly had a height again.
There was a problem hiding this comment.
nit: put style { width: '100%', height: '100%' } in VideoTile.styles.ts 😅
There was a problem hiding this comment.
I guess Ref doesn't create any new element but div does? And that could be the reason, LGTM if this would not create any regressions, but move the style tag into a separated file (./styles/VideoTile.styles)
| /* @conditional-compile-remove(teams-identity-support) */ | ||
| isTeamsIdentityCall={isTeamsCall} | ||
| /> | ||
| <React.StrictMode> |
There was a problem hiding this comment.
I guess this StrictMode element was added for testing purposes? Remove it if there is no specific reason to add this
There was a problem hiding this comment.
yup this was added so that should this come up again on the calling side we will know.
There was a problem hiding this comment.
Nice looks like it will check some other warnings and bad practice, and not impact production mode
6 participants
What
Remove use of
<Ref>tag from fluent nothstarWhy
fixes findDOMNode warning caused by strict mode in the video tile. Under the hood the
<Ref>tag probably usesfindDOMNode(). According to react's documentation about the function it should not be used with React.FunctionalComponents.<Ref>is a React.FunctionalComponent according the the api file provided by fluent-northstar.https://skype.visualstudio.com/SPOOL/_workitems/edit/3090254
How Tested
Ran app locally with new reference structure to make sure the video gallery still works