Remote video tile keyboard accessible to open menu#2651
Conversation
mgamis-msft
requested review from
JamesBurnside,
PorterNan,
alkwa-msft,
anjulgarg,
carocao-msft,
dmceachernmsft,
edwardlee-msft and
prabhjot-msft
as code owners
Calling bundle size is increased❗.
|
Chat bundle size is increased❗.
|
CallWithChat bundle size is increased❗.
|
mgamis-msft
force-pushed
the
mgamis/remote-video-tile-menu-accessible
branch
from
0c53b35 to
8d1c291
Compare
| <Stack style={remoteVideoTileWrapperStyle}> | ||
| <Stack | ||
| /* @conditional-compile-remove(pinned-participants) */ | ||
| tabIndex={remoteVideoTileMenuKind === 'drawer' ? 0 : undefined} |
There was a problem hiding this comment.
why are we setting the tab order of the video tile to 0 when it will show a drawer? it shouldn't be any different for mobile incase the user uses a bluetooth keyboard.
There was a problem hiding this comment.
My assumption here is that when menuKind is contextmenu it has the three dots that can be focused and clicked instead?
There was a problem hiding this comment.
Yes James is right. When the menuKind is 'contextual' then ellipses button will be present and keyboard accessible so we don't need to focus on the remote video tile. I don't think we want the keyboard user to tab on a remote video tile and then have to tab again to focus on the ellipses button to open the contextual menu:
2023-01-13.10-52-32.mp4
But let me know if this is desired.
| /* @conditional-compile-remove(pinned-participants) */ | ||
| tabIndex={remoteVideoTileMenuKind === 'drawer' ? 0 : undefined} | ||
| /* @conditional-compile-remove(pinned-participants) */ | ||
| onKeyDown={remoteVideoTileMenuKind === 'drawer' ? onKeyDown : undefined} |
There was a problem hiding this comment.
should this handler only be used when its a drawer over a context menu? the handler you wrote will work for desktop no?
There was a problem hiding this comment.
Yes, the handler should only be assigned when menuKind = 'drawer'. The RemoteVideoTile will be assigned menuKind = 'drawer' when the VGL is on mobile and menuKind = 'contextual' when the VGL is on desktop. We don't want the handler to be active on Desktop like so:
2023-01-13.10-55-15.mp4
4 participants
What
Make RemoteVideoTile menu keyboard accessible to open menu
Why
https://skype.visualstudio.com/SPOOL/_workitems/edit/3091719
How Tested
Local sample testing
Desktop: https://user-images.githubusercontent.com/79475487/212334291-aa21dea2-2f03-4371-bf71-0faac8b367eb.mp4
Emulated mobile on browser: https://user-images.githubusercontent.com/79475487/212334405-209467be-5593-42d8-b9bd-e368b3e1ea22.mp4
Process & policy checklist
Is this a breaking change?