Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runfix: address keyboard navigation issues in call ui (WPB-6075) #16538

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

V-Gira
Copy link
Contributor

@V-Gira V-Gira commented Jan 17, 2024

BugWPB-6075 [Web] Cannot use keyboard navigation to select devices in call ui

Description

Buttons should be able to be pressed with spacebar, to prevent typing in a conv in the background (see #16285), we add an onKeyDown event to buttons that need it
Additionnally, the select menus should be able to be quitted by pressing escape

Checklist

  • PR has been self reviewed by the author;
  • Hard-to-understand areas of the code have been commented;
  • If it is a core feature, unit tests have been added;

Important details for the reviewers

context for typing in a background conv #16285
previous iteration for allowing keyboard nav in call ui #16295

Comment on lines -252 to -262
const isSpaceOrEnterKey = (event: React.KeyboardEvent<HTMLButtonElement>) =>
[KEY.ENTER, KEY.SPACE].includes(event.key);

const handleToggleCameraKeydown = (event: React.KeyboardEvent<HTMLButtonElement>) => {
if (isSpaceOrEnterKey(event)) {
toggleCamera(call);
}

return true;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic can be achieved with the handleKeyDown() keyboard util
handleKeyDown() execute the callback in arguments on a key press of spacebar or enter

Comment on lines -270 to +257
if (!isEnterKey(event)) {
event.preventDefault();
}
event.preventDefault();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We preventDefault for all key presses, including enter, and add onKeyDown events to buttons affected

Copy link

codecov bot commented Jan 17, 2024

Codecov Report

Attention: 60 lines in your changes are missing coverage. Please review.

Comparison is base (2378bea) 45.42% compared to head (ce38019) 45.38%.
Report is 2 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #16538      +/-   ##
==========================================
- Coverage   45.42%   45.38%   -0.05%     
==========================================
  Files         740      740              
  Lines       24218    24257      +39     
  Branches     5504     5514      +10     
==========================================
+ Hits        11000    11008       +8     
- Misses      11806    11830      +24     
- Partials     1412     1419       +7     

Comment on lines -458 to +448
onClick={event => {
const target = event.target as Element;
// We want to ensure to only toggling the menu open or closed when clicking the icon, not the select menu
if (!target.closest('#select-microphone')) {
setAudioOptionsOpen(prev => !prev);
}
}}
onClick={() => setAudioOptionsOpen(prev => !prev)}
onKeyDown={event => handleKeyDown(event, () => setAudioOptionsOpen(prev => !prev))}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to filter out the select element in that iteration, both clicks and keypresses to select an option will toggle the menu off

@@ -206,7 +206,6 @@ const FullscreenVideoCall: React.FC<FullscreenVideoCallProps> = ({
setSelectedAudioOptions([microphone, speaker]);
switchMicrophoneInput(microphone.id);
switchSpeakerOutput(speaker.id);
setAudioOptionsOpen(false);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now rely on the onClick or onKeyDown event to toggle the menu off

@V-Gira V-Gira merged commit c204797 into dev Jan 17, 2024
10 checks passed
@V-Gira V-Gira deleted the v/call-ui-keyb-nav branch January 17, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants