From 991257cbc363dd1f674cadc0bddd1aaf2deeb12c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 31 Jan 2022 16:05:05 +0000 Subject: [PATCH] Fix accessibility and consistency of MessageComposerButtons (#7679) --- res/css/views/rooms/_MessageComposer.scss | 12 ++-- src/components/structures/ContextMenu.tsx | 8 ++- .../views/location/LocationButton.tsx | 23 ++++---- .../views/rooms/CollapsibleButton.tsx | 28 ++++++---- .../views/rooms/MessageComposer.tsx | 12 +++- .../views/rooms/MessageComposerButtons.tsx | 55 ++++++++----------- src/i18n/strings/en_EN.json | 2 +- .../rooms/MessageComposerButtons-test.tsx | 29 +++++----- 8 files changed, 94 insertions(+), 75 deletions(-) diff --git a/res/css/views/rooms/_MessageComposer.scss b/res/css/views/rooms/_MessageComposer.scss index 27d28eb4e6..c55e803747 100644 --- a/res/css/views/rooms/_MessageComposer.scss +++ b/res/css/views/rooms/_MessageComposer.scss @@ -192,11 +192,14 @@ limitations under the License. line-height: var(--size); width: auto; padding-left: var(--size); - border-radius: 100%; - margin-right: 6px; - &:last-child { - margin-right: auto; + &:not(.mx_CallContextMenu_item) { + border-radius: 50%; + margin-right: 6px; + + &:last-child { + margin-right: auto; + } } &::before { @@ -407,6 +410,7 @@ limitations under the License. align-items: center; max-width: unset; width: 100%; + margin: 7px 7px 7px 16px; // space out the buttons } .mx_MessageComposer_Menu .mx_ContextualMenu { diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index c2ebbdc784..fa164bca17 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -453,7 +453,13 @@ export const alwaysAboveRightOf = (elementRect: DOMRect, chevronFace = ChevronFa return menuOptions; }; -type ContextMenuTuple = [boolean, RefObject, () => void, () => void, (val: boolean) => void]; +type ContextMenuTuple = [ + boolean, + RefObject, + (ev?: SyntheticEvent) => void, + (ev?: SyntheticEvent) => void, + (val: boolean) => void, +]; // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-constraint export const useContextMenu = (): ContextMenuTuple => { const button = useRef(null); diff --git a/src/components/views/location/LocationButton.tsx b/src/components/views/location/LocationButton.tsx index 4ef0e744b1..815e001717 100644 --- a/src/components/views/location/LocationButton.tsx +++ b/src/components/views/location/LocationButton.tsx @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { ReactElement, useContext } from 'react'; +import React, { ReactElement, SyntheticEvent, useContext } from 'react'; import classNames from 'classnames'; import { RoomMember } from 'matrix-js-sdk/src/models/room-member'; import { logger } from "matrix-js-sdk/src/logger"; @@ -23,25 +23,29 @@ import { makeLocationContent } from "matrix-js-sdk/src/content-helpers"; import { _t } from '../../../languageHandler'; import LocationPicker from './LocationPicker'; -import { CollapsibleButton, ICollapsibleButtonProps } from '../rooms/CollapsibleButton'; +import { CollapsibleButton } from '../rooms/CollapsibleButton'; import ContextMenu, { aboveLeftOf, useContextMenu, AboveLeftOf } from "../../structures/ContextMenu"; import Modal from '../../../Modal'; import QuestionDialog from '../dialogs/QuestionDialog'; import MatrixClientContext from '../../../contexts/MatrixClientContext'; +import { OverflowMenuContext } from "../rooms/MessageComposerButtons"; -interface IProps extends Pick { +interface IProps { roomId: string; sender: RoomMember; menuPosition: AboveLeftOf; - narrowMode: boolean; } -export const LocationButton: React.FC = ( - { roomId, sender, menuPosition, narrowMode }, -) => { +export const LocationButton: React.FC = ({ roomId, sender, menuPosition }) => { + const overflowMenuCloser = useContext(OverflowMenuContext); const [menuDisplayed, button, openMenu, closeMenu] = useContextMenu(); const matrixClient = useContext(MatrixClientContext); + const _onFinished = (ev?: SyntheticEvent) => { + closeMenu(ev); + overflowMenuCloser?.(); + }; + let contextMenu: ReactElement; if (menuDisplayed) { const position = menuPosition ?? aboveLeftOf( @@ -49,13 +53,13 @@ export const LocationButton: React.FC = ( contextMenu = ; } @@ -74,7 +78,6 @@ export const LocationButton: React.FC = ( diff --git a/src/components/views/rooms/CollapsibleButton.tsx b/src/components/views/rooms/CollapsibleButton.tsx index 13834c289d..a0a802d6b4 100644 --- a/src/components/views/rooms/CollapsibleButton.tsx +++ b/src/components/views/rooms/CollapsibleButton.tsx @@ -14,23 +14,29 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { ComponentProps } from 'react'; +import React, { ComponentProps, useContext } from 'react'; +import classNames from 'classnames'; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; +import { MenuItem } from "../../structures/ContextMenu"; +import { OverflowMenuContext } from './MessageComposerButtons'; -export interface ICollapsibleButtonProps - extends ComponentProps -{ - narrowMode: boolean; +interface ICollapsibleButtonProps extends ComponentProps { title: string; } -export const CollapsibleButton = ({ narrowMode, title, ...props }: ICollapsibleButtonProps) => { - return ; +export const CollapsibleButton = ({ title, className, ...props }: ICollapsibleButtonProps) => { + const inOverflowMenu = !!useContext(OverflowMenuContext); + if (inOverflowMenu) { + return + { title } + ; + } + + return ; }; export default CollapsibleButton; diff --git a/src/components/views/rooms/MessageComposer.tsx b/src/components/views/rooms/MessageComposer.tsx index d4ebf720b1..c0873e9903 100644 --- a/src/components/views/rooms/MessageComposer.tsx +++ b/src/components/views/rooms/MessageComposer.tsx @@ -330,7 +330,10 @@ export default class MessageComposer extends React.Component { }; private setStickerPickerOpen = (isStickerPickerOpen: boolean) => { - this.setState({ isStickerPickerOpen }); + this.setState({ + isStickerPickerOpen, + isMenuOpen: false, + }); }; private toggleButtonMenu = (): void => { @@ -453,7 +456,12 @@ export default class MessageComposer extends React.Component { menuPosition={menuPosition} narrowMode={this.state.narrowMode} relation={this.props.relation} - onRecordStartEndClick={() => this.voiceRecordingButton.current?.onRecordStartEndClick()} + onRecordStartEndClick={() => { + this.voiceRecordingButton.current?.onRecordStartEndClick(); + if (this.state.narrowMode) { + this.toggleButtonMenu(); + } + }} setStickerPickerOpen={this.setStickerPickerOpen} showLocationButton={this.state.showLocationButton} showStickersButton={this.state.showStickersButton} diff --git a/src/components/views/rooms/MessageComposerButtons.tsx b/src/components/views/rooms/MessageComposerButtons.tsx index bbabb689c1..a5ebddde89 100644 --- a/src/components/views/rooms/MessageComposerButtons.tsx +++ b/src/components/views/rooms/MessageComposerButtons.tsx @@ -17,14 +17,14 @@ limitations under the License. import classNames from 'classnames'; import { IEventRelation } from "matrix-js-sdk/src/models/event"; import { M_POLL_START } from "matrix-events-sdk"; -import React, { ReactElement, useContext } from 'react'; +import React, { createContext, ReactElement, useContext } from 'react'; import { Room } from 'matrix-js-sdk/src/models/room'; import { MatrixClient } from 'matrix-js-sdk/src/client'; import { _t } from '../../../languageHandler'; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; -import { CollapsibleButton, ICollapsibleButtonProps } from './CollapsibleButton'; -import ContextMenu, { aboveLeftOf, AboveLeftOf, MenuItem, useContextMenu } from '../../structures/ContextMenu'; +import { CollapsibleButton } from './CollapsibleButton'; +import ContextMenu, { aboveLeftOf, AboveLeftOf, useContextMenu } from '../../structures/ContextMenu'; import dis from '../../../dispatcher/dispatcher'; import EmojiPicker from '../emojipicker/EmojiPicker'; import ErrorDialog from "../dialogs/ErrorDialog"; @@ -52,6 +52,9 @@ interface IProps { toggleButtonMenu: () => void; } +type OverflowMenuCloser = () => void; +export const OverflowMenuContext = createContext(null); + const MessageComposerButtons: React.FC = (props: IProps) => { const matrixClient: MatrixClient = useContext(MatrixClientContext); const { room, roomId } = useContext(RoomContext); @@ -107,7 +110,6 @@ function narrowMode( className={moreOptionsClasses} onClick={props.toggleButtonMenu} title={_t("More options")} - tooltip={false} /> { props.isMenuOpen && ( - { moreButtons.map((button, index) => ( - - { button } - - )) } + + { moreButtons } + ) } ; @@ -134,18 +130,16 @@ function emojiButton(props: IProps): ReactElement { key="emoji_button" addEmoji={props.addEmoji} menuPosition={props.menuPosition} - narrowMode={props.narrowMode} />; } -interface IEmojiButtonProps extends Pick { +interface IEmojiButtonProps { addEmoji: (unicode: string) => boolean; menuPosition: AboveLeftOf; } -const EmojiButton: React.FC = ( - { addEmoji, menuPosition, narrowMode }, -) => { +const EmojiButton: React.FC = ({ addEmoji, menuPosition }) => { + const overflowMenuCloser = useContext(OverflowMenuContext); const [menuDisplayed, button, openMenu, closeMenu] = useContextMenu(); let contextMenu: React.ReactElement | null = null; @@ -156,7 +150,10 @@ const EmojiButton: React.FC = ( contextMenu = { + closeMenu(); + overflowMenuCloser?.(); + }} managed={false} > @@ -177,7 +174,6 @@ const EmojiButton: React.FC = ( @@ -274,19 +270,18 @@ class UploadButton extends React.Component { function showStickersButton(props: IProps): ReactElement { return ( props.showStickersButton - ? props.setStickerPickerOpen(!props.isStickerPickerOpen)} title={ props.narrowMode - ? null + ? _t("Send a sticker") : props.isStickerPickerOpen ? _t("Hide Stickers") : _t("Show Stickers") } - label={props.narrowMode ? _t("Send a sticker") : null} /> : null ); @@ -302,25 +297,23 @@ function voiceRecordingButton(props: IProps): ReactElement { className="mx_MessageComposer_button mx_MessageComposer_voiceMessage" onClick={props.onRecordStartEndClick} title={_t("Send voice message")} - narrowMode={props.narrowMode} /> ); } function pollButton(props: IProps, room: Room): ReactElement { - return ; + return ; } -interface IPollButtonProps extends Pick { +interface IPollButtonProps { room: Room; } class PollButton extends React.PureComponent { + static contextType = OverflowMenuContext; + private onCreateClick = () => { + this.context?.(); // close overflow menu const canSend = this.props.room.currentState.maySendEvent( M_POLL_START.name, MatrixClientPeg.get().getUserId(), @@ -357,7 +350,6 @@ class PollButton extends React.PureComponent { ); @@ -377,7 +369,6 @@ function showLocationButton( roomId={roomId} sender={room.getMember(matrixClient.getUserId())} menuPosition={props.menuPosition} - narrowMode={props.narrowMode} /> : null ); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index f31199482f..bff54532f9 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1696,9 +1696,9 @@ "Send voice message": "Send voice message", "Add emoji": "Add emoji", "Upload file": "Upload file", + "Send a sticker": "Send a sticker", "Hide Stickers": "Hide Stickers", "Show Stickers": "Show Stickers", - "Send a sticker": "Send a sticker", "You do not have permission to start polls in this room.": "You do not have permission to start polls in this room.", "Create poll": "Create poll", "Bold": "Bold", diff --git a/test/components/views/rooms/MessageComposerButtons-test.tsx b/test/components/views/rooms/MessageComposerButtons-test.tsx index 915c66eb48..1fc9323848 100644 --- a/test/components/views/rooms/MessageComposerButtons-test.tsx +++ b/test/components/views/rooms/MessageComposerButtons-test.tsx @@ -77,6 +77,7 @@ describe("MessageComposerButtons", () => { narrowMode={true} showLocationButton={true} showStickersButton={true} + toggleButtonMenu={() => {}} />, ); @@ -159,28 +160,28 @@ function createRoomState(room: Room): IRoomState { function buttonLabels(buttons: ReactWrapper): any[] { // Note: Depends on the fact that the mini buttons use aria-label - // and the labels under More options use label + // and the labels under More options use textContent const mainButtons = ( buttons - .find('div') - .map((button: ReactWrapper) => button.prop("aria-label")) + .find('div.mx_MessageComposer_button[aria-label]') + .map((button: ReactWrapper) => button.prop("aria-label") as string) .filter(x => x) ); - let extraButtons = ( + const extraButtons = ( buttons - .find('div') - .map((button: ReactWrapper) => button.prop("label")) + .find('.mx_MessageComposer_Menu div.mx_AccessibleButton[role="menuitem"]') + .map((button: ReactWrapper) => button.text()) .filter(x => x) ); - if (extraButtons.length === 0) { - extraButtons = []; - } else { - extraButtons = [extraButtons]; + + const list: any[] = [ + ...mainButtons, + ]; + + if (extraButtons.length > 0) { + list.push(extraButtons); } - return [ - ...mainButtons, - ...extraButtons, - ]; + return list; }