Fix checkboxes/radios in context menus should only close on ENTER, not SPACE

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
This commit is contained in:
Michael Telatynski 2020-07-06 10:18:49 +01:00
parent d366ca12a0
commit 6cb0ac6a50
3 changed files with 51 additions and 7 deletions

View file

@ -424,7 +424,18 @@ MenuItemCheckbox.propTypes = {
};
// Semantic component for representing a styled role=menuitemcheckbox
export const StyledMenuItemCheckbox = ({children, label, checked=false, disabled=false, ...props}) => {
export const StyledMenuItemCheckbox = ({children, label, onChange, onClose, checked=false, disabled=false, ...props}) => {
const onKeyDown = (ev) => {
// Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12
if (ev.key === Key.ENTER || ev.key === Key.SPACE) {
ev.preventDefault();
ev.stopPropagation();
onChange(ev);
if (ev.key === Key.ENTER) {
onClose();
}
}
};
return (
<StyledCheckbox
{...props}
@ -434,18 +445,21 @@ export const StyledMenuItemCheckbox = ({children, label, checked=false, disabled
aria-disabled={disabled}
tabIndex={-1}
aria-label={label}
onChange={onChange}
onKeyDown={onKeyDown}
>
{ children }
</StyledCheckbox>
);
};
StyledMenuItemCheckbox.propTypes = {
...AccessibleButton.propTypes,
...StyledCheckbox.propTypes,
label: PropTypes.string, // optional
checked: PropTypes.bool.isRequired,
disabled: PropTypes.bool, // optional
className: PropTypes.string, // optional
onClick: PropTypes.func.isRequired,
onChange: PropTypes.func.isRequired,
onClose: PropTypes.func.isRequired, // gets called after onChange on Key.ENTER
};
// Semantic component for representing a role=menuitemradio
@ -467,7 +481,18 @@ MenuItemRadio.propTypes = {
};
// Semantic component for representing a styled role=menuitemradio
export const StyledMenuItemRadio = ({children, label, checked=false, disabled=false, ...props}) => {
export const StyledMenuItemRadio = ({children, label, onChange, onClose, checked=false, disabled=false, ...props}) => {
const onKeyDown = (ev) => {
// Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12
if (ev.key === Key.ENTER || ev.key === Key.SPACE) {
ev.preventDefault();
ev.stopPropagation();
onChange(ev);
if (ev.key === Key.ENTER) {
onClose();
}
}
};
return (
<StyledRadioButton
{...props}
@ -477,6 +502,8 @@ export const StyledMenuItemRadio = ({children, label, checked=false, disabled=fa
aria-disabled={disabled}
tabIndex={-1}
aria-label={label}
onChange={onChange}
onKeyDown={onKeyDown}
>
{ children }
</StyledRadioButton>
@ -488,7 +515,8 @@ StyledMenuItemRadio.propTypes = {
checked: PropTypes.bool.isRequired,
disabled: PropTypes.bool, // optional
className: PropTypes.string, // optional
onClick: PropTypes.func.isRequired,
onChange: PropTypes.func.isRequired,
onClose: PropTypes.func.isRequired, // gets called after onChange on Key.ENTER
};
// Placement method for <ContextMenu /> to position context menu to right of elementRect with chevronOffset

View file

@ -334,6 +334,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
<div>
<div className='mx_RoomSublist2_contextMenu_title'>{_t("Sort by")}</div>
<StyledMenuItemRadio
onClose={this.onCloseMenu}
onChange={() => this.onTagSortChanged(SortAlgorithm.Recent)}
checked={!isAlphabetical}
name={`mx_${this.props.tagId}_sortBy`}
@ -341,6 +342,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
{_t("Activity")}
</StyledMenuItemRadio>
<StyledMenuItemRadio
onClose={this.onCloseMenu}
onChange={() => this.onTagSortChanged(SortAlgorithm.Alphabetic)}
checked={isAlphabetical}
name={`mx_${this.props.tagId}_sortBy`}
@ -352,6 +354,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
<div>
<div className='mx_RoomSublist2_contextMenu_title'>{_t("Unread rooms")}</div>
<StyledMenuItemCheckbox
onClose={this.onCloseMenu}
onChange={this.onUnreadFirstChanged}
checked={isUnreadFirst}
>
@ -362,6 +365,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
<div>
<div className='mx_RoomSublist2_contextMenu_title'>{_t("Show")}</div>
<StyledMenuItemCheckbox
onClose={this.onCloseMenu}
onChange={this.onMessagePreviewChanged}
checked={this.props.layout.showPreviews}
>

View file

@ -80,6 +80,8 @@ interface IState {
generalMenuPosition: PartialDOMRect;
}
type Volume = ALL_MESSAGES_LOUD | ALL_MESSAGES | MENTIONS_ONLY | MUTE;
const messagePreviewId = (roomId: string) => `mx_RoomTile2_messagePreview_${roomId}`;
const contextMenuBelow = (elementRect: PartialDOMRect) => {
@ -213,6 +215,11 @@ export default class RoomTile2 extends React.Component<IProps, IState> {
// TODO: Support tagging: https://github.com/vector-im/riot-web/issues/14211
// TODO: XOR favourites and low priority: https://github.com/vector-im/riot-web/issues/14210
if ((ev as React.KeyboardEvent).key === Key.ENTER) {
// Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12
this.setState({generalMenuPosition: null}); // hide the menu
}
};
private onLeaveRoomClick = (ev: ButtonEvent) => {
@ -237,11 +244,13 @@ export default class RoomTile2 extends React.Component<IProps, IState> {
this.setState({generalMenuPosition: null}); // hide the menu
};
private async saveNotifState(ev: ButtonEvent, newState: ALL_MESSAGES_LOUD | ALL_MESSAGES | MENTIONS_ONLY | MUTE) {
private async saveNotifState(ev: ButtonEvent, newState: Volume) {
ev.preventDefault();
ev.stopPropagation();
if (MatrixClientPeg.get().isGuest()) return;
// get key before we go async and React discards the nativeEvent
const key = (ev as React.KeyboardEvent).key;
try {
// TODO add local echo - https://github.com/vector-im/riot-web/issues/14280
await setRoomNotifsState(this.props.room.roomId, newState);
@ -251,7 +260,10 @@ export default class RoomTile2 extends React.Component<IProps, IState> {
console.error(error);
}
this.setState({notificationsMenuPosition: null}); // Close the context menu
if (key === Key.ENTER) {
// Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12
this.setState({notificationsMenuPosition: null}); // hide the menu
}
}
private onClickAllNotifs = ev => this.saveNotifState(ev, ALL_MESSAGES);