Fix VideoEffects Pane reseting focus when an item is selected #4618
Conversation
JamesBurnside
requested review from
alkwa-msft,
dmceachernmsft,
carocao-msft,
edwardlee-msft,
PorterNan,
mgamis-msft,
prabhjot-msft,
AmyL219,
jimchou-dev,
vhuseinova-msft,
Leah-Xia-Microsoft,
jpeng-ms,
waynemo,
emlynmac,
JoshuaLai and
palatter
Calling bundle size is increased❗.
|
Chat bundle size is increased❗.
|
CallWithChat bundle size is increased❗.
|
|
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. |
@azure/communication-react jest test coverage for stable.
|
@azure/communication-react jest test coverage for beta.
|
JamesBurnside
added
the
update_snapshots
github-actions
bot
removed
the
update_snapshots
There was a problem hiding this comment.
Hmm some of these looking a little flakey potentially
There was a problem hiding this comment.
Ah they're not! Every other test performs a click on a specific tile, hence why the focus rect does not show. In this test it does not perform a subsequent click and hence the focus rect shows.
| */ | ||
| focusOnMount?: boolean; | ||
| componentRef?: React.RefObject<{ |
There was a problem hiding this comment.
Interesting so we can set a action to take when the component reference is set?
There was a problem hiding this comment.
It's the concept of an imperative handle. You can do this today with regular html elements, e.g. document.getElementById("myTextField").focus();
(ref: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus).
In that example we get the handle by doing document.getElementById. In react world we can't do that, we have to do one render pass that renders the component and updates the ref, then once its been rendered (i.e. added to the DOM), we can define functions that can be executed on it. The action won't happen when the component reference is set, it happen anytime after its set when the owner of the handle calls the function
What
Instead of trying to change focus on mount, instead use an imperative handle to call
focus()when openPane is called. We can't use focus on mount, because if the pane is already open its already mounted, but we still need to shift focus.Why
Fix bug where whenever an item was selected, the focus would shift back to the
nonevideo effects item.How Tested
Locally verified in both configuration page and call page that when the video effects pane is first opened the focus shifts to the first item, confirmed when the pane is already open and the effects button is clicked the focus shifts to the none item, and confirmed when an item is selected the focus does not shift back to the first item.