VideoGallery UUID for its local video tile to dock to.#1111
VideoGallery UUID for its local video tile to dock to.#1111mgamis-msft merged 13 commits intomainfrom
Conversation
mgamis-msft
requested review from
JamesBurnside,
PorterNan,
alcail,
alkwa-msft,
anjulgarg and
prprabhu-ms
as code owners
| <span | ||
| className="ms-layer" | ||
| /> | ||
| > |
There was a problem hiding this comment.
This is the FluentUI Modal. Not sure why the old snapshot did not have it.
|
Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" 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 "ui change" label to the PR for updating the snapshot. |
|
|
||
| useEffect(() => { | ||
| if (containerRef.current) { | ||
| containerRef.current.id = `video-gallery-${uuidv4()}`; |
There was a problem hiding this comment.
The idea is that whenever our containerRef changes we want to use a new UUID so we don't have collisions between different video gallery instances?
There was a problem hiding this comment.
This useEffect only happens since containerRef is instantiated only once during the lifetime of the VideoGallery component. So the UUID is only created once so it should be the same for a VIdeoGallery's lifetime.
There was a problem hiding this comment.
Why not simply generate an id once.
containerId = useRef(`video-gallery-${uuidv4()}`)
...
return (<div id={containerId.current}> .... </div>)
There was a problem hiding this comment.
I like that better too. Changed. Thanks.
This reverts commit 0deee04.
|
Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" label to the PR for updating the snapshot. |
mgamis-msft
force-pushed
the
mgamis/videogallery-uuid
branch
from
2ccd019 to
ef5009d
Compare
|
Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" 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 "ui change" 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 "ui change" label to the PR for updating the snapshot. |
1 similar comment
|
Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" label to the PR for updating the snapshot. |
5 participants
What
Assigning VideoGallery containerRef.current.id using a UUID. Then using containerRef.current.id which is assigned to root div as the Modal's hostId.
Why
https://skype.visualstudio.com/SPOOL/_workitems/edit/2649260
When there are multiple VideoGallery's in the same page their id's will be the same since it was hardcoded and thus all local video tiles will dock to the first VideoGallery.
How Tested
Verified docs in storybook. Please check chromatic.
Process & policy checklist
Is this a breaking change?