From 98dd07cddba4baef18e995e6c3f473644e9555cb Mon Sep 17 00:00:00 2001 From: Alex Kirk Date: Fri, 1 Dec 2023 12:24:30 +0100 Subject: [PATCH 01/36] Override aria-label for whole tile --- src/components/views/rooms/EventTile.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index cb414173387..026f9aad618 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -1210,6 +1210,7 @@ export class UnwrappedEventTile extends React.Component "ref": this.ref, "className": classes, "aria-live": ariaLive, + "aria-label": this.props.mxEvent.sender.name + '. ' + (this.props.mxEvent.clearEvent || this.props.mxEvent.event).content.body, "aria-atomic": true, "data-scroll-tokens": scrollToken, "data-has-reply": !!replyChain, @@ -1399,6 +1400,7 @@ export class UnwrappedEventTile extends React.Component "className": classes, "tabIndex": -1, "aria-live": ariaLive, + "aria-label": this.props.mxEvent.sender.name + '. ' + (this.props.mxEvent.clearEvent || this.props.mxEvent.event).content.body, "aria-atomic": "true", "data-scroll-tokens": scrollToken, "data-layout": this.props.layout, From 6e6696e385debbbc79b645d817ca96c03d67496d Mon Sep 17 00:00:00 2001 From: Alex Kirk Date: Tue, 12 Dec 2023 14:35:04 +0100 Subject: [PATCH 02/36] Message navigation --- src/accessibility/KeyboardShortcuts.ts | 19 ++++++++ src/components/structures/LoggedInView.tsx | 11 +++++ src/components/structures/MessagePanel.tsx | 44 +++++++++++++++++++ .../views/rooms/SendMessageComposer.tsx | 9 ++++ src/i18n/strings/en_EN.json | 2 + 5 files changed, 85 insertions(+) diff --git a/src/accessibility/KeyboardShortcuts.ts b/src/accessibility/KeyboardShortcuts.ts index 398dc270088..4a87e2f521c 100644 --- a/src/accessibility/KeyboardShortcuts.ts +++ b/src/accessibility/KeyboardShortcuts.ts @@ -120,6 +120,10 @@ export enum KeyBindingAction { SelectPrevUnreadRoom = "KeyBinding.previousUnreadRoom", /** Select next room with unread messages */ SelectNextUnreadRoom = "KeyBinding.nextUnreadRoom", + /** Select prev message */ + SelectPrevMessage = "KeyBinding.previousMessage", + /** Select next message */ + SelectNextMessage = "KeyBinding.nextMessage", /** Switches to a space by number */ SwitchToSpaceByNumber = "KeyBinding.switchToSpaceByNumber", @@ -286,6 +290,8 @@ export const CATEGORIES: Record = { KeyBindingAction.SelectPrevUnreadRoom, KeyBindingAction.SelectNextRoom, KeyBindingAction.SelectPrevRoom, + KeyBindingAction.SelectNextMessage, + KeyBindingAction.SelectPrevMessage, KeyBindingAction.OpenUserSettings, KeyBindingAction.SwitchToSpaceByNumber, KeyBindingAction.PreviousVisitedRoomOrSpace, @@ -373,6 +379,7 @@ export const KEYBOARD_SHORTCUTS: IKeyboardShortcuts = { }, [KeyBindingAction.EditPrevMessage]: { default: { + ctrlOrCmdKey: true, key: Key.ARROW_UP, }, displayName: _td("keyboard|navigate_prev_message_edit"), @@ -558,6 +565,18 @@ export const KEYBOARD_SHORTCUTS: IKeyboardShortcuts = { }, displayName: _td("keyboard|prev_room"), }, + [KeyBindingAction.SelectNextMessage]: { + default: { + key: Key.ARROW_DOWN, + }, + displayName: _td("keyboard|next_message"), + }, + [KeyBindingAction.SelectPrevMessage]: { + default: { + key: Key.ARROW_UP, + }, + displayName: _td("keyboard|prev_message"), + }, [KeyBindingAction.CancelAutocomplete]: { default: { key: Key.ESCAPE, diff --git a/src/components/structures/LoggedInView.tsx b/src/components/structures/LoggedInView.tsx index c0707fba260..5d3f5f5f6b5 100644 --- a/src/components/structures/LoggedInView.tsx +++ b/src/components/structures/LoggedInView.tsx @@ -449,6 +449,8 @@ class LoggedInView extends React.Component { const roomAction = getKeyBindingsManager().getRoomAction(ev); switch (roomAction) { + case KeyBindingAction.SelectPrevMessage: + case KeyBindingAction.SelectNextMessage: case KeyBindingAction.ScrollUp: case KeyBindingAction.ScrollDown: case KeyBindingAction.JumpToFirstMessage: @@ -472,6 +474,13 @@ class LoggedInView extends React.Component { const navAction = getKeyBindingsManager().getNavigationAction(ev); switch (navAction) { + case KeyBindingAction.SelectPrevMessage: + case KeyBindingAction.SelectNextMessage: + // pass the event down to the scroll panel + this.onScrollKeyPressed(ev); + handled = true; + break; + case KeyBindingAction.FilterRooms: dis.dispatch({ action: "focus_room_filter", @@ -544,6 +553,8 @@ class LoggedInView extends React.Component { PlatformPeg.get()?.navigateForwardBack(false); handled = true; break; + case KeyBindingAction.SelectPrevMessage: + handled = true; } // Handle labs actions here, as they apply within the same scope diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 3a97f27b381..c381ae6fc4a 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -56,6 +56,9 @@ import { MainGrouper } from "./grouper/MainGrouper"; import { CreationGrouper } from "./grouper/CreationGrouper"; import { _t } from "../../languageHandler"; import { getLateEventInfo } from "./grouper/LateEventGrouper"; +import { getKeyBindingsManager } from "../../KeyBindingsManager"; +import { KeyBindingAction } from "../../accessibility/KeyboardShortcuts"; +import { highlightEvent } from "../../utils/EventUtils"; const CONTINUATION_MAX_INTERVAL = 5 * 60 * 1000; // 5 minutes const continuedTypes = [EventType.Sticker, EventType.RoomMessage]; @@ -205,6 +208,7 @@ interface IReadReceiptForUser { */ export default class MessagePanel extends React.Component { public static contextType = RoomContext; + public focusedEventId?: string; public context!: React.ContextType; public static defaultProps = { @@ -422,6 +426,46 @@ export default class MessagePanel extends React.Component { * @param {KeyboardEvent} ev: the keyboard event to handle */ public handleScrollKey(ev: React.KeyboardEvent | KeyboardEvent): void { + const navAction = getKeyBindingsManager().getNavigationAction(ev); + switch (navAction) { + case KeyBindingAction.SelectPrevMessage: + case KeyBindingAction.SelectNextMessage: + const events: WrappedEvent[] = this.props.events.map((event) => { + return { event, shouldShow: this.shouldShowEvent(event) }; + }); + const currentEventId = this.focusedEventId || this.props.highlightedEventId || events[events.length-1]?.event.getId()!; + if ( navAction === KeyBindingAction.SelectPrevMessage ) { + events.reverse(); + } + let previousEventId = null; + for (let i = events.length - 1; i >= 0; i--) { + const { event, shouldShow } = events[i]; + if (!shouldShow) { + continue; + } + const eventId = event.getId()!; + if ( previousEventId && eventId === currentEventId ) { + console.log('.mx_EventTile[data-event-id="' + previousEventId + '"]'); + + document.querySelector('.mx_EventTile[data-event-id="' + previousEventId + '"]')?.focus(); + this.focusedEventId = previousEventId; + ev.preventDefault(); + return; + } + previousEventId = eventId; + } + if ( navAction === KeyBindingAction.SelectNextMessage ) { + defaultDispatcher.dispatch( + { + action: Action.FocusSendMessageComposer, + context: TimelineRenderingType.Room, + }, + true, + ); + } + break; + } + this.scrollPanel.current?.handleScrollKey(ev); } diff --git a/src/components/views/rooms/SendMessageComposer.tsx b/src/components/views/rooms/SendMessageComposer.tsx index b371d81b729..840ff147324 100644 --- a/src/components/views/rooms/SendMessageComposer.tsx +++ b/src/components/views/rooms/SendMessageComposer.tsx @@ -308,6 +308,15 @@ export class SendMessageComposer extends React.Component Date: Tue, 12 Dec 2023 16:28:21 +0100 Subject: [PATCH 03/36] Change hotkeys to use cmd/ctrl --- src/accessibility/KeyboardShortcuts.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/accessibility/KeyboardShortcuts.ts b/src/accessibility/KeyboardShortcuts.ts index 4a87e2f521c..9a1441238ff 100644 --- a/src/accessibility/KeyboardShortcuts.ts +++ b/src/accessibility/KeyboardShortcuts.ts @@ -379,7 +379,6 @@ export const KEYBOARD_SHORTCUTS: IKeyboardShortcuts = { }, [KeyBindingAction.EditPrevMessage]: { default: { - ctrlOrCmdKey: true, key: Key.ARROW_UP, }, displayName: _td("keyboard|navigate_prev_message_edit"), @@ -567,12 +566,14 @@ export const KEYBOARD_SHORTCUTS: IKeyboardShortcuts = { }, [KeyBindingAction.SelectNextMessage]: { default: { + ctrlOrCmdKey: true, key: Key.ARROW_DOWN, }, displayName: _td("keyboard|next_message"), }, [KeyBindingAction.SelectPrevMessage]: { default: { + ctrlOrCmdKey: true, key: Key.ARROW_UP, }, displayName: _td("keyboard|prev_message"), From 3bb1bb43dd5369ac924207a2c955d53f526ff71a Mon Sep 17 00:00:00 2001 From: Alex Kirk Date: Fri, 8 Dec 2023 08:06:50 +0100 Subject: [PATCH 04/36] Add landmark navigation --- src/Keyboard.ts | 1 + src/accessibility/KeyboardShortcuts.ts | 23 ++++++++++++++- src/components/structures/LeftPanel.tsx | 13 +++++++++ src/components/structures/LoggedInView.tsx | 4 +++ src/components/structures/MatrixChat.tsx | 14 +++++++++ .../views/rooms/BasicMessageComposer.tsx | 11 +++++++ src/components/views/rooms/RoomList.tsx | 28 +++++++++++++++++- src/components/views/spaces/SpacePanel.tsx | 29 +++++++++++++++++-- src/i18n/strings/en_EN.json | 2 ++ 9 files changed, 121 insertions(+), 4 deletions(-) diff --git a/src/Keyboard.ts b/src/Keyboard.ts index 7b1ea4031be..5f2c82c65ef 100644 --- a/src/Keyboard.ts +++ b/src/Keyboard.ts @@ -29,6 +29,7 @@ export const Key = { ARROW_DOWN: "ArrowDown", ARROW_LEFT: "ArrowLeft", ARROW_RIGHT: "ArrowRight", + F6: "F6", TAB: "Tab", ESCAPE: "Escape", ENTER: "Enter", diff --git a/src/accessibility/KeyboardShortcuts.ts b/src/accessibility/KeyboardShortcuts.ts index 9a1441238ff..18059543b58 100644 --- a/src/accessibility/KeyboardShortcuts.ts +++ b/src/accessibility/KeyboardShortcuts.ts @@ -133,6 +133,10 @@ export enum KeyBindingAction { PreviousVisitedRoomOrSpace = "KeyBinding.PreviousVisitedRoomOrSpace", /** Navigates forward */ NextVisitedRoomOrSpace = "KeyBinding.NextVisitedRoomOrSpace", + /** Navigates to the next Landmark */ + NextLandmark = "KeyBinding.nextLandmark", + /** Navigates to the next Landmark */ + PreviousLandmark = "KeyBinding.previousLandmark", /** Toggles microphone while on a call */ ToggleMicInCall = "KeyBinding.toggleMicInCall", @@ -258,7 +262,7 @@ export const CATEGORIES: Record = { KeyBindingAction.ExpandRoomListSection, KeyBindingAction.NextRoom, KeyBindingAction.PrevRoom, - ], + ], }, [CategoryName.ACCESSIBILITY]: { categoryLabel: _td("common|accessibility"), @@ -296,6 +300,8 @@ export const CATEGORIES: Record = { KeyBindingAction.SwitchToSpaceByNumber, KeyBindingAction.PreviousVisitedRoomOrSpace, KeyBindingAction.NextVisitedRoomOrSpace, + KeyBindingAction.NextLandmark, + KeyBindingAction.PreviousLandmark, ], }, [CategoryName.AUTOCOMPLETE]: { @@ -733,4 +739,19 @@ export const KEYBOARD_SHORTCUTS: IKeyboardShortcuts = { key: Key.COMMA, }, }, + [KeyBindingAction.NextLandmark]: { + default: { + ctrlOrCmdKey: true, + key: Key.F6, + }, + displayName: _td("keyboard|next_landmark"), + }, + [KeyBindingAction.PreviousLandmark]: { + default: { + ctrlOrCmdKey: true, + key: Key.F6, + shiftKey: true, + }, + displayName: _td("keyboard|prev_landmark"), + }, }; diff --git a/src/components/structures/LeftPanel.tsx b/src/components/structures/LeftPanel.tsx index c2454e04fb0..132d6b150d3 100644 --- a/src/components/structures/LeftPanel.tsx +++ b/src/components/structures/LeftPanel.tsx @@ -309,6 +309,19 @@ export default class LeftPanel extends React.Component { } break; } + const navAction = getKeyBindingsManager().getNavigationAction(ev); + switch (navAction) { + case KeyBindingAction.NextLandmark: + ev.stopPropagation(); + ev.preventDefault(); + document.querySelector('.mx_RoomTile_selected')?.focus(); + break; + case KeyBindingAction.PreviousLandmark: + ev.stopPropagation(); + ev.preventDefault(); + document.querySelector('.mx_SpaceButton_active')?.focus(); + break; + } }; private renderBreadcrumbs(): React.ReactNode { diff --git a/src/components/structures/LoggedInView.tsx b/src/components/structures/LoggedInView.tsx index 5d3f5f5f6b5..d314e3cb13f 100644 --- a/src/components/structures/LoggedInView.tsx +++ b/src/components/structures/LoggedInView.tsx @@ -481,6 +481,10 @@ class LoggedInView extends React.Component { handled = true; break; + case KeyBindingAction.NextLandmark: + document.querySelector('.mx_SpaceButton_active')?.focus(); + handled = true; + break; case KeyBindingAction.FilterRooms: dis.dispatch({ action: "focus_room_filter", diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index b9ac2c71d72..2a32b04fdc8 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -314,6 +314,20 @@ export default class MatrixChat extends React.PureComponent { } } + private onKeyDown = (e: KeyboardEvent | React.KeyboardEvent): void => { + this.props.onKeyDown?.(e); + + const action = getKeyBindingsManager().getAccessibilityAction(e); + switch (action) { + case KeyBindingAction.Escape: + if (!this.props.hasCancel) break; + + e.stopPropagation(); + e.preventDefault(); + this.props.onFinished(); + break; + } + }; /** * Kick off a call to {@link initSession}, and handle any errors */ diff --git a/src/components/views/rooms/BasicMessageComposer.tsx b/src/components/views/rooms/BasicMessageComposer.tsx index 061dfe27039..740cd4d9207 100644 --- a/src/components/views/rooms/BasicMessageComposer.tsx +++ b/src/components/views/rooms/BasicMessageComposer.tsx @@ -536,6 +536,17 @@ export default class BasicMessageEditor extends React.Component } } + const navAction = getKeyBindingsManager().getNavigationAction(event); + switch (navAction) { + case KeyBindingAction.PreviousLandmark: + document.querySelector('.mx_RoomTile_selected')?.focus(); + handled = true; + break; + case KeyBindingAction.NextLandmark: + document.querySelector('.mx_SpaceButton_active')?.focus(); + handled = true; + break; + } const autocompleteAction = getKeyBindingsManager().getAutocompleteAction(event); const accessibilityAction = getKeyBindingsManager().getAccessibilityAction(event); if (model.autoComplete?.hasCompletions()) { diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 28154c40d65..18c96caa7f5 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -19,6 +19,7 @@ import React, { ComponentType, createRef, ReactComponentElement, SyntheticEvent import { IState as IRovingTabIndexState, RovingTabIndexProvider } from "../../../accessibility/RovingTabIndex"; import MatrixClientContext from "../../../contexts/MatrixClientContext"; +import { TimelineRenderingType } from "../../../contexts/RoomContext"; import { shouldShowComponent } from "../../../customisations/helpers/UIComponents"; import { Action } from "../../../dispatcher/actions"; import defaultDispatcher from "../../../dispatcher/dispatcher"; @@ -61,6 +62,8 @@ import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; import ExtraTile from "./ExtraTile"; import RoomSublist, { IAuxButtonProps } from "./RoomSublist"; import { SdkContextClass } from "../../../contexts/SDKContext"; +import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts"; +import { getKeyBindingsManager } from "../../../KeyBindingsManager"; interface IProps { onKeyDown: (ev: React.KeyboardEvent, state: IRovingTabIndexState) => void; @@ -645,7 +648,30 @@ export default class RoomList extends React.PureComponent {
{ + const navAction = getKeyBindingsManager().getNavigationAction(ev); + switch (navAction) { + case KeyBindingAction.NextLandmark: + const inThread = !!document.activeElement?.closest(".mx_ThreadView"); + defaultDispatcher.dispatch( + { + action: Action.FocusSendMessageComposer, + context: inThread ? TimelineRenderingType.Thread : TimelineRenderingType.Room, + }, + true, + ); + ev.stopPropagation(); + ev.preventDefault(); + return; + case KeyBindingAction.PreviousLandmark: + document.querySelector('.mx_RoomSearch')?.focus(); + ev.stopPropagation(); + ev.preventDefault(); + return; + } + + onKeyDownHandler(ev) + }} className="mx_RoomList" role="tree" aria-label={_t("common|rooms")} diff --git a/src/components/views/spaces/SpacePanel.tsx b/src/components/views/spaces/SpacePanel.tsx index 5b1756244b8..e91ab461dfd 100644 --- a/src/components/views/spaces/SpacePanel.tsx +++ b/src/components/views/spaces/SpacePanel.tsx @@ -52,6 +52,7 @@ import { UPDATE_STATUS_INDICATOR, } from "../../../stores/notifications/RoomNotificationStateStore"; import SpaceContextMenu from "../context_menus/SpaceContextMenu"; +import { TimelineRenderingType } from "../../../contexts/RoomContext"; import IconizedContextMenu, { IconizedContextMenuCheckbox, IconizedContextMenuOptionList, @@ -69,7 +70,8 @@ import defaultDispatcher from "../../../dispatcher/dispatcher"; import { ActionPayload } from "../../../dispatcher/payloads"; import { Action } from "../../../dispatcher/actions"; import { NotificationState } from "../../../stores/notifications/NotificationState"; -import { ALTERNATE_KEY_NAME } from "../../../accessibility/KeyboardShortcuts"; +import { ALTERNATE_KEY_NAME, KeyBindingAction } from "../../../accessibility/KeyboardShortcuts"; +import { getKeyBindingsManager } from "../../../KeyBindingsManager"; import { shouldShowComponent } from "../../../customisations/helpers/UIComponents"; import { UIComponent } from "../../../settings/UIFeature"; import { ThreadsActivityCentre } from "./threads-activity-centre/"; @@ -369,7 +371,30 @@ const SpacePanel: React.FC = () => { >
+
  • + Go to next landmark +
    + + + Ctrl + + + + + + + F6 + + +
    +
    • +
  • + Go to previous landmark +
    + + + Ctrl + + + + + + + Shift + + + + + + + F6 + + +
    +
    • From e3ebda61e9a966bd98885ea19569b9c88bc4e194 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 14 May 2024 20:28:47 +0530 Subject: [PATCH 20/36] Fix lint --- src/components/views/rooms/RoomList.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 1426dd81d98..dbf76ed5387 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -65,7 +65,6 @@ import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts"; import { getKeyBindingsManager } from "../../../KeyBindingsManager"; import AccessibleButton from "../elements/AccessibleButton"; - interface IProps { onKeyDown: (ev: React.KeyboardEvent, state: IRovingTabIndexState) => void; onFocus: (ev: React.FocusEvent) => void; From 7fba563ef54613c946709f833d4a22bbf177e558 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Wed, 15 May 2024 18:02:36 +0530 Subject: [PATCH 21/36] Move code to a single file --- src/accessibility/KeyboardLandmarkUtils.ts | 129 ++++++++++++++++++ src/components/structures/LeftPanel.tsx | 10 +- src/components/structures/LoggedInView.tsx | 9 +- .../views/rooms/BasicMessageComposer.tsx | 7 +- src/components/views/rooms/RoomList.tsx | 15 +- src/components/views/spaces/SpacePanel.tsx | 15 +- 6 files changed, 147 insertions(+), 38 deletions(-) create mode 100644 src/accessibility/KeyboardLandmarkUtils.ts diff --git a/src/accessibility/KeyboardLandmarkUtils.ts b/src/accessibility/KeyboardLandmarkUtils.ts new file mode 100644 index 00000000000..e851a225af5 --- /dev/null +++ b/src/accessibility/KeyboardLandmarkUtils.ts @@ -0,0 +1,129 @@ +/* + * + * Copyright 2024 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { TimelineRenderingType } from "../contexts/RoomContext"; +import { Action } from "../dispatcher/actions"; +import defaultDispatcher from "../dispatcher/dispatcher"; + +export const enum Landmark { + // This is the space/home button in the left panel. + ACTIVE_SPACE_BUTTON, + // This is the room filter in the left panel. + ROOM_SEARCH, + // This is the currently opened room/first room in the room list in the left panel. + ROOM_LIST, + // This is the message composer within the room. + MESSAGE_COMPOSER, + // This is the welcome screen shown when no room is selected + HOME, +} + +/** + * The landmarks are cycled through in the following order: + * ACTIVE_SPACE_BUTTON <-> ROOM_SEARCH <-> ROOM_LIST <-> MESSAGE_COMPOSER/HOME <-> ACTIVE_SPACE_BUTTON + */ +class LandmarkCollections { + /** + * Find the next landmark from a given landmark + * @param currentLandmark The current landmark + * @returns The next landmark in sequence + */ + public static nextLandmarkFrom(currentLandmark: Landmark): Landmark { + switch (currentLandmark) { + case Landmark.ACTIVE_SPACE_BUTTON: + return Landmark.ROOM_SEARCH; + case Landmark.ROOM_SEARCH: + return Landmark.ROOM_LIST; + case Landmark.ROOM_LIST: { + // Switch to composer if available otherwise the home component + const isComposerOpen = !!document.querySelector(".mx_MessageComposer"); + return isComposerOpen ? Landmark.MESSAGE_COMPOSER : Landmark.HOME; + } + case Landmark.MESSAGE_COMPOSER: + return Landmark.ACTIVE_SPACE_BUTTON; + case Landmark.HOME: + return Landmark.ACTIVE_SPACE_BUTTON; + } + } + + /** + * Find the previous landmark from a given landmark + * @param currentLandmark The current landmark + * @returns The previous landmark + */ + public static previousLandmarkFrom(currentLandmark: Landmark): Landmark { + switch (currentLandmark) { + case Landmark.ACTIVE_SPACE_BUTTON: { + const isComposerOpen = !!document.querySelector(".mx_MessageComposer"); + // Switch to composer if available otherwise the home component + return isComposerOpen ? Landmark.MESSAGE_COMPOSER : Landmark.HOME; + } + case Landmark.ROOM_SEARCH: + return Landmark.ACTIVE_SPACE_BUTTON; + case Landmark.ROOM_LIST: + return Landmark.ROOM_SEARCH; + case Landmark.MESSAGE_COMPOSER: + return Landmark.ROOM_LIST; + case Landmark.HOME: + return Landmark.ROOM_LIST; + } + } +} + +/** + * Keys are the different landmarks and the values are function which when invoked + * gives focus to the corresponding landmark. + */ +const focusFunctions = { + [Landmark.ACTIVE_SPACE_BUTTON]: () => { + document.querySelector(".mx_SpaceButton_active")?.focus(); + }, + [Landmark.ROOM_SEARCH]: () => { + document.querySelector(".mx_RoomSearch")?.focus(); + }, + [Landmark.ROOM_LIST]: () => { + ( + document.querySelector(".mx_RoomTile_selected") || + document.querySelector(".mx_RoomTile") + )?.focus(); + }, + [Landmark.MESSAGE_COMPOSER]: () => { + const inThread = !!document.activeElement?.closest(".mx_ThreadView"); + defaultDispatcher.dispatch( + { + action: Action.FocusSendMessageComposer, + context: inThread ? TimelineRenderingType.Thread : TimelineRenderingType.Room, + }, + true, + ); + }, + [Landmark.HOME]: () => { + document.querySelector(".mx_HomePage")?.focus(); + }, +}; + +/** + * Focus on the next/previous landmark + * @param currentLandmark The landmark from which this function is called + * @param backward If true, the previous landmark is focused. Otherwise the next landmark is focused. + */ +export function navigateLandmark(currentLandmark: Landmark, backward: boolean = false): void { + const newLandmark = backward + ? LandmarkCollections.previousLandmarkFrom(currentLandmark) + : LandmarkCollections.nextLandmarkFrom(currentLandmark); + focusFunctions[newLandmark](); +} diff --git a/src/components/structures/LeftPanel.tsx b/src/components/structures/LeftPanel.tsx index 4cbd62b53b6..9c6463843aa 100644 --- a/src/components/structures/LeftPanel.tsx +++ b/src/components/structures/LeftPanel.tsx @@ -44,6 +44,7 @@ import AccessibleButton, { ButtonEvent } from "../views/elements/AccessibleButto import PosthogTrackers from "../../PosthogTrackers"; import PageType from "../../PageTypes"; import { UserOnboardingButton } from "../views/user-onboarding/UserOnboardingButton"; +import { Landmark, navigateLandmark } from "../../accessibility/KeyboardLandmarkUtils"; interface IProps { isMinimized: boolean; @@ -313,17 +314,12 @@ export default class LeftPanel extends React.Component { case KeyBindingAction.NextLandmark: ev.stopPropagation(); ev.preventDefault(); - // The next landmark is the selected room or the room first room, if none selected. - ( - document.querySelector(".mx_RoomTile_selected") || - document.querySelector(".mx_RoomTile") - )?.focus(); + navigateLandmark(Landmark.ROOM_SEARCH); break; case KeyBindingAction.PreviousLandmark: ev.stopPropagation(); ev.preventDefault(); - // The previous landmark is the active space button. - document.querySelector(".mx_SpaceButton_active")?.focus(); + navigateLandmark(Landmark.ROOM_SEARCH, true); break; } }; diff --git a/src/components/structures/LoggedInView.tsx b/src/components/structures/LoggedInView.tsx index eed50f02205..d49f22cd413 100644 --- a/src/components/structures/LoggedInView.tsx +++ b/src/components/structures/LoggedInView.tsx @@ -75,6 +75,7 @@ import { PipContainer } from "./PipContainer"; import { monitorSyncedPushRules } from "../../utils/pushRules/monitorSyncedPushRules"; import { ConfigOptions } from "../../SdkConfig"; import { MatrixClientContextProvider } from "./MatrixClientContextProvider"; +import { Landmark, navigateLandmark } from "../../accessibility/KeyboardLandmarkUtils"; // We need to fetch each pinned message individually (if we don't already have it) // so each pinned message may trigger a request. Limit the number per room for sanity. @@ -473,8 +474,11 @@ class LoggedInView extends React.Component { const navAction = getKeyBindingsManager().getNavigationAction(ev); switch (navAction) { case KeyBindingAction.NextLandmark: - // This is reached when no landmark area is selected, so we start at the first landmark. - document.querySelector(".mx_SpaceButton_active")?.focus(); + navigateLandmark(Landmark.HOME); + handled = true; + break; + case KeyBindingAction.PreviousLandmark: + navigateLandmark(Landmark.HOME, true); handled = true; break; case KeyBindingAction.FilterRooms: @@ -633,7 +637,6 @@ class LoggedInView extends React.Component { public render(): React.ReactNode { let pageElement; - switch (this.props.page_type) { case PageTypes.RoomView: pageElement = ( diff --git a/src/components/views/rooms/BasicMessageComposer.tsx b/src/components/views/rooms/BasicMessageComposer.tsx index 84677af0f18..d9d8c3582bc 100644 --- a/src/components/views/rooms/BasicMessageComposer.tsx +++ b/src/components/views/rooms/BasicMessageComposer.tsx @@ -51,6 +51,7 @@ import { _t } from "../../../languageHandler"; import { linkify } from "../../../linkify-matrix"; import { SdkContextClass } from "../../../contexts/SDKContext"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; +import { Landmark, navigateLandmark } from "../../../accessibility/KeyboardLandmarkUtils"; // matches emoticons which follow the start of a line or whitespace const REGEX_EMOTICON_WHITESPACE = new RegExp("(?:^|\\s)(" + EMOTICON_REGEX.source + ")\\s|:^$"); @@ -539,13 +540,11 @@ export default class BasicMessageEditor extends React.Component const navAction = getKeyBindingsManager().getNavigationAction(event); switch (navAction) { case KeyBindingAction.PreviousLandmark: - // The previous landmark is the selected room (we know a room is selected because we're in its message composer). - document.querySelector(".mx_RoomTile_selected")?.focus(); + navigateLandmark(Landmark.MESSAGE_COMPOSER, true); handled = true; break; case KeyBindingAction.NextLandmark: - // The next landmark completes the cycle, back to the active space button. - document.querySelector(".mx_SpaceButton_active")?.focus(); + navigateLandmark(Landmark.MESSAGE_COMPOSER); handled = true; break; } diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index dbf76ed5387..47d9d273e5b 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -19,7 +19,6 @@ import React, { ComponentType, createRef, ReactComponentElement, SyntheticEvent import { IState as IRovingTabIndexState, RovingTabIndexProvider } from "../../../accessibility/RovingTabIndex"; import MatrixClientContext from "../../../contexts/MatrixClientContext"; -import { TimelineRenderingType } from "../../../contexts/RoomContext"; import { shouldShowComponent } from "../../../customisations/helpers/UIComponents"; import { Action } from "../../../dispatcher/actions"; import defaultDispatcher from "../../../dispatcher/dispatcher"; @@ -64,6 +63,7 @@ import { SdkContextClass } from "../../../contexts/SDKContext"; import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts"; import { getKeyBindingsManager } from "../../../KeyBindingsManager"; import AccessibleButton from "../elements/AccessibleButton"; +import { Landmark, navigateLandmark } from "../../../accessibility/KeyboardLandmarkUtils"; interface IProps { onKeyDown: (ev: React.KeyboardEvent, state: IRovingTabIndexState) => void; @@ -658,23 +658,14 @@ export default class RoomList extends React.PureComponent { onKeyDown={(ev) => { const navAction = getKeyBindingsManager().getNavigationAction(ev); if (navAction === KeyBindingAction.NextLandmark) { - // The next landmark is the message composer or the thread message composer. - const inThread = !!document.activeElement?.closest(".mx_ThreadView"); - defaultDispatcher.dispatch( - { - action: Action.FocusSendMessageComposer, - context: inThread ? TimelineRenderingType.Thread : TimelineRenderingType.Room, - }, - true, - ); + navigateLandmark(Landmark.ROOM_LIST); ev.stopPropagation(); ev.preventDefault(); return; } if (navAction === KeyBindingAction.PreviousLandmark) { - // The previous landmark is the room search input. - document.querySelector(".mx_RoomSearch")?.focus(); + navigateLandmark(Landmark.ROOM_LIST, true); ev.stopPropagation(); ev.preventDefault(); return; diff --git a/src/components/views/spaces/SpacePanel.tsx b/src/components/views/spaces/SpacePanel.tsx index 240c519a69e..784af285fa2 100644 --- a/src/components/views/spaces/SpacePanel.tsx +++ b/src/components/views/spaces/SpacePanel.tsx @@ -51,7 +51,6 @@ import { UPDATE_STATUS_INDICATOR, } from "../../../stores/notifications/RoomNotificationStateStore"; import SpaceContextMenu from "../context_menus/SpaceContextMenu"; -import { TimelineRenderingType } from "../../../contexts/RoomContext"; import IconizedContextMenu, { IconizedContextMenuCheckbox, IconizedContextMenuOptionList, @@ -75,6 +74,7 @@ import { shouldShowComponent } from "../../../customisations/helpers/UIComponent import { UIComponent } from "../../../settings/UIFeature"; import { ThreadsActivityCentre } from "./threads-activity-centre/"; import AccessibleButton from "../elements/AccessibleButton"; +import { Landmark, navigateLandmark } from "../../../accessibility/KeyboardLandmarkUtils"; const useSpaces = (): [Room[], MetaSpace[], Room[], SpaceKey] => { const invites = useEventEmitterState(SpaceStore.instance, UPDATE_INVITED_SPACES, () => { @@ -389,22 +389,13 @@ const SpacePanel: React.FC = () => { onKeyDown={(ev) => { const navAction = getKeyBindingsManager().getNavigationAction(ev); if (navAction === KeyBindingAction.PreviousLandmark) { - // The previous landmark is the message composer or the thread message composer. - const inThread = !!document.activeElement?.closest(".mx_ThreadView"); - defaultDispatcher.dispatch( - { - action: Action.FocusSendMessageComposer, - context: inThread ? TimelineRenderingType.Thread : TimelineRenderingType.Room, - }, - true, - ); + navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON, true); ev.stopPropagation(); ev.preventDefault(); return; } if (navAction === KeyBindingAction.NextLandmark) { - // The next landmark is the room search input. - document.querySelector(".mx_RoomSearch")?.focus(); + navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON); ev.stopPropagation(); ev.preventDefault(); return; From 7eb4dc60140edbc386117d6aa4f847d1e64ae669 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Wed, 15 May 2024 21:37:08 +0530 Subject: [PATCH 22/36] Add jest test --- .../KeyboardLandmarkUtils-test.tsx | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 test/accessibility/KeyboardLandmarkUtils-test.tsx diff --git a/test/accessibility/KeyboardLandmarkUtils-test.tsx b/test/accessibility/KeyboardLandmarkUtils-test.tsx new file mode 100644 index 00000000000..61218fb53d9 --- /dev/null +++ b/test/accessibility/KeyboardLandmarkUtils-test.tsx @@ -0,0 +1,131 @@ +/* + * + * Copyright 2024 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { render, screen, waitFor } from "@testing-library/react"; +import React from "react"; + +import { Landmark, navigateLandmark } from "../../src/accessibility/KeyboardLandmarkUtils"; +import defaultDispatcher from "../../src/dispatcher/dispatcher"; + +describe("KeyboardLandmarkUtils", () => { + it("Landmarks are cycled through correctly without an opened room", () => { + render( +
    +
    + SPACE_BUTTON +
    +
    + ROOM_SEARCH +
    +
    + ROOM_TILE +
    +
    + HOME_PAGE +
    +
    , + ); + // ACTIVE_SPACE_BUTTON <-> ROOM_SEARCH <-> ROOM_LIST <-> HOME <-> ACTIVE_SPACE_BUTTON + // ACTIVE_SPACE_BUTTON -> ROOM_SEARCH + navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON); + expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); + + // ROOM_SEARCH -> ROOM_LIST + navigateLandmark(Landmark.ROOM_SEARCH); + expect(screen.getByTestId("mx_RoomTile")).toHaveFocus(); + + // ROOM_LIST -> HOME_PAGE + navigateLandmark(Landmark.ROOM_LIST); + expect(screen.getByTestId("mx_HomePage")).toHaveFocus(); + + // HOME_PAGE -> ACTIVE_SPACE_BUTTOn + navigateLandmark(Landmark.HOME); + expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); + + // HOME_PAGE <- ACTIVE_SPACE_BUTTON + navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON, true); + expect(screen.getByTestId("mx_HomePage")).toHaveFocus(); + + // ROOM_LIST <- HOME_PAGE + navigateLandmark(Landmark.HOME, true); + expect(screen.getByTestId("mx_RoomTile")).toHaveFocus(); + + // ROOM_SEARCH <- ROOM_LIST + navigateLandmark(Landmark.ROOM_LIST, true); + expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); + + // ACTIVE_SPACE_BUTTON <- ROOM_SEARCH + navigateLandmark(Landmark.ROOM_SEARCH, true); + expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); + }); + + it("Landmarks are cycled through correctly with an opened room", async () => { + const callback = jest.fn(); + defaultDispatcher.register(callback); + render( +
    +
    + SPACE_BUTTON +
    +
    + ROOM_SEARCH +
    +
    + ROOM_TILE +
    +
    + ROOM +
    + COMPOSER +
    +
    +
    , + ); + // ACTIVE_SPACE_BUTTON <-> ROOM_SEARCH <-> ROOM_LIST <-> MESSAGE_COMPOSER <-> ACTIVE_SPACE_BUTTON + // ACTIVE_SPACE_BUTTON -> ROOM_SEARCH + navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON); + expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); + + // ROOM_SEARCH -> ROOM_LIST + navigateLandmark(Landmark.ROOM_SEARCH); + expect(screen.getByTestId("mx_RoomTile_selected")).toHaveFocus(); + + // ROOM_LIST -> MESSAGE_COMPOSER + navigateLandmark(Landmark.ROOM_LIST); + await waitFor(() => expect(callback).toHaveBeenCalledTimes(1)); + + // MESSAGE_COMPOSER -> ACTIVE_SPACE_BUTTOn + navigateLandmark(Landmark.HOME); + expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); + + // MESSAGE_COMPOSER <- ACTIVE_SPACE_BUTTON + navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON, true); + await waitFor(() => expect(callback).toHaveBeenCalledTimes(2)); + + // ROOM_LIST <- MESSAGE_COMPOSER + navigateLandmark(Landmark.HOME, true); + expect(screen.getByTestId("mx_RoomTile_selected")).toHaveFocus(); + + // ROOM_SEARCH <- ROOM_LIST + navigateLandmark(Landmark.ROOM_LIST, true); + expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); + + // ACTIVE_SPACE_BUTTON <- ROOM_SEARCH + navigateLandmark(Landmark.ROOM_SEARCH, true); + expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); + }); +}); From d043ae959ad98f87edccce2830d7f896bd5c5545 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Mon, 15 Jul 2024 00:35:56 +0530 Subject: [PATCH 23/36] Use a circular array for storing the order of landmarks --- src/accessibility/KeyboardLandmarkUtils.ts | 129 ------------------ src/accessibility/LandmarkNavigation.ts | 107 +++++++++++++++ src/components/structures/LeftPanel.tsx | 6 +- src/components/structures/LoggedInView.tsx | 7 +- .../views/rooms/BasicMessageComposer.tsx | 6 +- src/components/views/rooms/RoomList.tsx | 6 +- src/components/views/spaces/SpacePanel.tsx | 6 +- 7 files changed, 123 insertions(+), 144 deletions(-) delete mode 100644 src/accessibility/KeyboardLandmarkUtils.ts create mode 100644 src/accessibility/LandmarkNavigation.ts diff --git a/src/accessibility/KeyboardLandmarkUtils.ts b/src/accessibility/KeyboardLandmarkUtils.ts deleted file mode 100644 index e851a225af5..00000000000 --- a/src/accessibility/KeyboardLandmarkUtils.ts +++ /dev/null @@ -1,129 +0,0 @@ -/* - * - * Copyright 2024 The Matrix.org Foundation C.I.C. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { TimelineRenderingType } from "../contexts/RoomContext"; -import { Action } from "../dispatcher/actions"; -import defaultDispatcher from "../dispatcher/dispatcher"; - -export const enum Landmark { - // This is the space/home button in the left panel. - ACTIVE_SPACE_BUTTON, - // This is the room filter in the left panel. - ROOM_SEARCH, - // This is the currently opened room/first room in the room list in the left panel. - ROOM_LIST, - // This is the message composer within the room. - MESSAGE_COMPOSER, - // This is the welcome screen shown when no room is selected - HOME, -} - -/** - * The landmarks are cycled through in the following order: - * ACTIVE_SPACE_BUTTON <-> ROOM_SEARCH <-> ROOM_LIST <-> MESSAGE_COMPOSER/HOME <-> ACTIVE_SPACE_BUTTON - */ -class LandmarkCollections { - /** - * Find the next landmark from a given landmark - * @param currentLandmark The current landmark - * @returns The next landmark in sequence - */ - public static nextLandmarkFrom(currentLandmark: Landmark): Landmark { - switch (currentLandmark) { - case Landmark.ACTIVE_SPACE_BUTTON: - return Landmark.ROOM_SEARCH; - case Landmark.ROOM_SEARCH: - return Landmark.ROOM_LIST; - case Landmark.ROOM_LIST: { - // Switch to composer if available otherwise the home component - const isComposerOpen = !!document.querySelector(".mx_MessageComposer"); - return isComposerOpen ? Landmark.MESSAGE_COMPOSER : Landmark.HOME; - } - case Landmark.MESSAGE_COMPOSER: - return Landmark.ACTIVE_SPACE_BUTTON; - case Landmark.HOME: - return Landmark.ACTIVE_SPACE_BUTTON; - } - } - - /** - * Find the previous landmark from a given landmark - * @param currentLandmark The current landmark - * @returns The previous landmark - */ - public static previousLandmarkFrom(currentLandmark: Landmark): Landmark { - switch (currentLandmark) { - case Landmark.ACTIVE_SPACE_BUTTON: { - const isComposerOpen = !!document.querySelector(".mx_MessageComposer"); - // Switch to composer if available otherwise the home component - return isComposerOpen ? Landmark.MESSAGE_COMPOSER : Landmark.HOME; - } - case Landmark.ROOM_SEARCH: - return Landmark.ACTIVE_SPACE_BUTTON; - case Landmark.ROOM_LIST: - return Landmark.ROOM_SEARCH; - case Landmark.MESSAGE_COMPOSER: - return Landmark.ROOM_LIST; - case Landmark.HOME: - return Landmark.ROOM_LIST; - } - } -} - -/** - * Keys are the different landmarks and the values are function which when invoked - * gives focus to the corresponding landmark. - */ -const focusFunctions = { - [Landmark.ACTIVE_SPACE_BUTTON]: () => { - document.querySelector(".mx_SpaceButton_active")?.focus(); - }, - [Landmark.ROOM_SEARCH]: () => { - document.querySelector(".mx_RoomSearch")?.focus(); - }, - [Landmark.ROOM_LIST]: () => { - ( - document.querySelector(".mx_RoomTile_selected") || - document.querySelector(".mx_RoomTile") - )?.focus(); - }, - [Landmark.MESSAGE_COMPOSER]: () => { - const inThread = !!document.activeElement?.closest(".mx_ThreadView"); - defaultDispatcher.dispatch( - { - action: Action.FocusSendMessageComposer, - context: inThread ? TimelineRenderingType.Thread : TimelineRenderingType.Room, - }, - true, - ); - }, - [Landmark.HOME]: () => { - document.querySelector(".mx_HomePage")?.focus(); - }, -}; - -/** - * Focus on the next/previous landmark - * @param currentLandmark The landmark from which this function is called - * @param backward If true, the previous landmark is focused. Otherwise the next landmark is focused. - */ -export function navigateLandmark(currentLandmark: Landmark, backward: boolean = false): void { - const newLandmark = backward - ? LandmarkCollections.previousLandmarkFrom(currentLandmark) - : LandmarkCollections.nextLandmarkFrom(currentLandmark); - focusFunctions[newLandmark](); -} diff --git a/src/accessibility/LandmarkNavigation.ts b/src/accessibility/LandmarkNavigation.ts new file mode 100644 index 00000000000..54ac6177641 --- /dev/null +++ b/src/accessibility/LandmarkNavigation.ts @@ -0,0 +1,107 @@ +/* + * Copyright 2024 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { TimelineRenderingType } from "../contexts/RoomContext"; +import { Action } from "../dispatcher/actions"; +import defaultDispatcher from "../dispatcher/dispatcher"; + +export const enum Landmark { + // This is the space/home button in the left panel. + ACTIVE_SPACE_BUTTON, + // This is the room filter in the left panel. + ROOM_SEARCH, + // This is the currently opened room/first room in the room list in the left panel. + ROOM_LIST, + // This is the message composer within the room if available or it is the welcome screen shown when no room is selected + MESSAGE_COMPOSER_OR_HOME, +} + +const ORDERED_LANDMARKS = [ + Landmark.ACTIVE_SPACE_BUTTON, + Landmark.ROOM_SEARCH, + Landmark.ROOM_LIST, + Landmark.MESSAGE_COMPOSER_OR_HOME, +]; + +/** + * The landmarks are cycled through in the following order: + * ACTIVE_SPACE_BUTTON <-> ROOM_SEARCH <-> ROOM_LIST <-> MESSAGE_COMPOSER/HOME <-> ACTIVE_SPACE_BUTTON + */ +export class LandmarkNavigation { + private static getIndexInArray(i: number): number { + const n = ORDERED_LANDMARKS.length; + // calculating index in the circular array + const nextIndex = ((i % n) + n) % n; + return nextIndex; + } + + /** + * Find the next landmark from a given landmark + * @param currentLandmark The current landmark + * @returns The next landmark in sequence + */ + public static navigateToNextLandmarkFrom(currentLandmark: Landmark): void { + const currentIndex = ORDERED_LANDMARKS.findIndex((l) => l === currentLandmark); + const nextIndex = LandmarkNavigation.getIndexInArray(currentIndex + 1); + const newLandmark = ORDERED_LANDMARKS[nextIndex]; + focusFunctions[newLandmark](); + } + + /** + * Find the previous landmark from a given landmark + * @param currentLandmark The current landmark + * @returns The previous landmark + */ + public static navigateToPreviousLandmarkFrom(currentLandmark: Landmark): void { + const currentIndex = ORDERED_LANDMARKS.findIndex((l) => l === currentLandmark); + const nextIndex = LandmarkNavigation.getIndexInArray(currentIndex - 1); + const newLandmark = ORDERED_LANDMARKS[nextIndex]; + focusFunctions[newLandmark](); + } +} +/** + * Keys are the different landmarks and the values are function which when invoked + * gives focus to the corresponding landmark. + */ +const focusFunctions = { + [Landmark.ACTIVE_SPACE_BUTTON]: () => { + document.querySelector(".mx_SpaceButton_active")?.focus(); + }, + [Landmark.ROOM_SEARCH]: () => { + document.querySelector(".mx_RoomSearch")?.focus(); + }, + [Landmark.ROOM_LIST]: () => { + ( + document.querySelector(".mx_RoomTile_selected") || + document.querySelector(".mx_RoomTile") + )?.focus(); + }, + [Landmark.MESSAGE_COMPOSER_OR_HOME]: () => { + const isComposerOpen = !!document.querySelector(".mx_MessageComposer"); + if (isComposerOpen) { + const inThread = !!document.activeElement?.closest(".mx_ThreadView"); + defaultDispatcher.dispatch( + { + action: Action.FocusSendMessageComposer, + context: inThread ? TimelineRenderingType.Thread : TimelineRenderingType.Room, + }, + true, + ); + } else { + document.querySelector(".mx_HomePage")?.focus(); + } + }, +}; diff --git a/src/components/structures/LeftPanel.tsx b/src/components/structures/LeftPanel.tsx index 9c6463843aa..5e97d4d1d44 100644 --- a/src/components/structures/LeftPanel.tsx +++ b/src/components/structures/LeftPanel.tsx @@ -44,7 +44,7 @@ import AccessibleButton, { ButtonEvent } from "../views/elements/AccessibleButto import PosthogTrackers from "../../PosthogTrackers"; import PageType from "../../PageTypes"; import { UserOnboardingButton } from "../views/user-onboarding/UserOnboardingButton"; -import { Landmark, navigateLandmark } from "../../accessibility/KeyboardLandmarkUtils"; +import { Landmark, LandmarkNavigation } from "../../accessibility/LandmarkNavigation"; interface IProps { isMinimized: boolean; @@ -314,12 +314,12 @@ export default class LeftPanel extends React.Component { case KeyBindingAction.NextLandmark: ev.stopPropagation(); ev.preventDefault(); - navigateLandmark(Landmark.ROOM_SEARCH); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_SEARCH); break; case KeyBindingAction.PreviousLandmark: ev.stopPropagation(); ev.preventDefault(); - navigateLandmark(Landmark.ROOM_SEARCH, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_SEARCH); break; } }; diff --git a/src/components/structures/LoggedInView.tsx b/src/components/structures/LoggedInView.tsx index 6b3989989aa..2be0f5c8fa4 100644 --- a/src/components/structures/LoggedInView.tsx +++ b/src/components/structures/LoggedInView.tsx @@ -75,7 +75,7 @@ import { PipContainer } from "./PipContainer"; import { monitorSyncedPushRules } from "../../utils/pushRules/monitorSyncedPushRules"; import { ConfigOptions } from "../../SdkConfig"; import { MatrixClientContextProvider } from "./MatrixClientContextProvider"; -import { Landmark, navigateLandmark } from "../../accessibility/KeyboardLandmarkUtils"; +import { Landmark, LandmarkNavigation } from "../../accessibility/LandmarkNavigation"; // We need to fetch each pinned message individually (if we don't already have it) // so each pinned message may trigger a request. Limit the number per room for sanity. @@ -472,11 +472,11 @@ class LoggedInView extends React.Component { const navAction = getKeyBindingsManager().getNavigationAction(ev); switch (navAction) { case KeyBindingAction.NextLandmark: - navigateLandmark(Landmark.HOME); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); handled = true; break; case KeyBindingAction.PreviousLandmark: - navigateLandmark(Landmark.HOME, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); handled = true; break; case KeyBindingAction.FilterRooms: @@ -639,6 +639,7 @@ class LoggedInView extends React.Component { public render(): React.ReactNode { let pageElement; + switch (this.props.page_type) { case PageTypes.RoomView: pageElement = ( diff --git a/src/components/views/rooms/BasicMessageComposer.tsx b/src/components/views/rooms/BasicMessageComposer.tsx index d9d8c3582bc..c2d6205bf05 100644 --- a/src/components/views/rooms/BasicMessageComposer.tsx +++ b/src/components/views/rooms/BasicMessageComposer.tsx @@ -51,7 +51,7 @@ import { _t } from "../../../languageHandler"; import { linkify } from "../../../linkify-matrix"; import { SdkContextClass } from "../../../contexts/SDKContext"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; -import { Landmark, navigateLandmark } from "../../../accessibility/KeyboardLandmarkUtils"; +import { Landmark, LandmarkNavigation } from "../../../accessibility/LandmarkNavigation"; // matches emoticons which follow the start of a line or whitespace const REGEX_EMOTICON_WHITESPACE = new RegExp("(?:^|\\s)(" + EMOTICON_REGEX.source + ")\\s|:^$"); @@ -540,11 +540,11 @@ export default class BasicMessageEditor extends React.Component const navAction = getKeyBindingsManager().getNavigationAction(event); switch (navAction) { case KeyBindingAction.PreviousLandmark: - navigateLandmark(Landmark.MESSAGE_COMPOSER, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); handled = true; break; case KeyBindingAction.NextLandmark: - navigateLandmark(Landmark.MESSAGE_COMPOSER); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); handled = true; break; } diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 47d9d273e5b..8b2ee88d230 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -63,7 +63,7 @@ import { SdkContextClass } from "../../../contexts/SDKContext"; import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts"; import { getKeyBindingsManager } from "../../../KeyBindingsManager"; import AccessibleButton from "../elements/AccessibleButton"; -import { Landmark, navigateLandmark } from "../../../accessibility/KeyboardLandmarkUtils"; +import { Landmark, LandmarkNavigation } from "../../../accessibility/LandmarkNavigation"; interface IProps { onKeyDown: (ev: React.KeyboardEvent, state: IRovingTabIndexState) => void; @@ -658,14 +658,14 @@ export default class RoomList extends React.PureComponent { onKeyDown={(ev) => { const navAction = getKeyBindingsManager().getNavigationAction(ev); if (navAction === KeyBindingAction.NextLandmark) { - navigateLandmark(Landmark.ROOM_LIST); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_LIST); ev.stopPropagation(); ev.preventDefault(); return; } if (navAction === KeyBindingAction.PreviousLandmark) { - navigateLandmark(Landmark.ROOM_LIST, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_LIST); ev.stopPropagation(); ev.preventDefault(); return; diff --git a/src/components/views/spaces/SpacePanel.tsx b/src/components/views/spaces/SpacePanel.tsx index bba308fdb96..f6c7ce05a2b 100644 --- a/src/components/views/spaces/SpacePanel.tsx +++ b/src/components/views/spaces/SpacePanel.tsx @@ -73,7 +73,7 @@ import { shouldShowComponent } from "../../../customisations/helpers/UIComponent import { UIComponent } from "../../../settings/UIFeature"; import { ThreadsActivityCentre } from "./threads-activity-centre/"; import AccessibleButton from "../elements/AccessibleButton"; -import { Landmark, navigateLandmark } from "../../../accessibility/KeyboardLandmarkUtils"; +import { Landmark, LandmarkNavigation } from "../../../accessibility/LandmarkNavigation"; import { KeyboardShortcut } from "../settings/KeyboardShortcut"; const useSpaces = (): [Room[], MetaSpace[], Room[], SpaceKey] => { @@ -389,13 +389,13 @@ const SpacePanel: React.FC = () => { onKeyDown={(ev) => { const navAction = getKeyBindingsManager().getNavigationAction(ev); if (navAction === KeyBindingAction.PreviousLandmark) { - navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); ev.stopPropagation(); ev.preventDefault(); return; } if (navAction === KeyBindingAction.NextLandmark) { - navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); ev.stopPropagation(); ev.preventDefault(); return; From e38849ab82a0117cab17aa28d14d018dc7d0897a Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Mon, 15 Jul 2024 00:37:04 +0530 Subject: [PATCH 24/36] Fix test --- .../KeyboardLandmarkUtils-test.tsx | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/test/accessibility/KeyboardLandmarkUtils-test.tsx b/test/accessibility/KeyboardLandmarkUtils-test.tsx index 61218fb53d9..4b0375f6680 100644 --- a/test/accessibility/KeyboardLandmarkUtils-test.tsx +++ b/test/accessibility/KeyboardLandmarkUtils-test.tsx @@ -1,5 +1,4 @@ /* - * * Copyright 2024 The Matrix.org Foundation C.I.C. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -18,7 +17,7 @@ import { render, screen, waitFor } from "@testing-library/react"; import React from "react"; -import { Landmark, navigateLandmark } from "../../src/accessibility/KeyboardLandmarkUtils"; +import { Landmark, LandmarkNavigation } from "../../src/accessibility/LandmarkNavigation"; import defaultDispatcher from "../../src/dispatcher/dispatcher"; describe("KeyboardLandmarkUtils", () => { @@ -41,35 +40,35 @@ describe("KeyboardLandmarkUtils", () => { ); // ACTIVE_SPACE_BUTTON <-> ROOM_SEARCH <-> ROOM_LIST <-> HOME <-> ACTIVE_SPACE_BUTTON // ACTIVE_SPACE_BUTTON -> ROOM_SEARCH - navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); // ROOM_SEARCH -> ROOM_LIST - navigateLandmark(Landmark.ROOM_SEARCH); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_SEARCH); expect(screen.getByTestId("mx_RoomTile")).toHaveFocus(); // ROOM_LIST -> HOME_PAGE - navigateLandmark(Landmark.ROOM_LIST); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_LIST); expect(screen.getByTestId("mx_HomePage")).toHaveFocus(); - // HOME_PAGE -> ACTIVE_SPACE_BUTTOn - navigateLandmark(Landmark.HOME); + // HOME_PAGE -> ACTIVE_SPACE_BUTTON + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); // HOME_PAGE <- ACTIVE_SPACE_BUTTON - navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); expect(screen.getByTestId("mx_HomePage")).toHaveFocus(); // ROOM_LIST <- HOME_PAGE - navigateLandmark(Landmark.HOME, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); expect(screen.getByTestId("mx_RoomTile")).toHaveFocus(); // ROOM_SEARCH <- ROOM_LIST - navigateLandmark(Landmark.ROOM_LIST, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_LIST); expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); // ACTIVE_SPACE_BUTTON <- ROOM_SEARCH - navigateLandmark(Landmark.ROOM_SEARCH, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_SEARCH); expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); }); @@ -97,35 +96,35 @@ describe("KeyboardLandmarkUtils", () => { ); // ACTIVE_SPACE_BUTTON <-> ROOM_SEARCH <-> ROOM_LIST <-> MESSAGE_COMPOSER <-> ACTIVE_SPACE_BUTTON // ACTIVE_SPACE_BUTTON -> ROOM_SEARCH - navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); // ROOM_SEARCH -> ROOM_LIST - navigateLandmark(Landmark.ROOM_SEARCH); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_SEARCH); expect(screen.getByTestId("mx_RoomTile_selected")).toHaveFocus(); // ROOM_LIST -> MESSAGE_COMPOSER - navigateLandmark(Landmark.ROOM_LIST); + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_LIST); await waitFor(() => expect(callback).toHaveBeenCalledTimes(1)); - // MESSAGE_COMPOSER -> ACTIVE_SPACE_BUTTOn - navigateLandmark(Landmark.HOME); + // MESSAGE_COMPOSER -> ACTIVE_SPACE_BUTTON + LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); // MESSAGE_COMPOSER <- ACTIVE_SPACE_BUTTON - navigateLandmark(Landmark.ACTIVE_SPACE_BUTTON, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); await waitFor(() => expect(callback).toHaveBeenCalledTimes(2)); // ROOM_LIST <- MESSAGE_COMPOSER - navigateLandmark(Landmark.HOME, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); expect(screen.getByTestId("mx_RoomTile_selected")).toHaveFocus(); // ROOM_SEARCH <- ROOM_LIST - navigateLandmark(Landmark.ROOM_LIST, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_LIST); expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); // ACTIVE_SPACE_BUTTON <- ROOM_SEARCH - navigateLandmark(Landmark.ROOM_SEARCH, true); + LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_SEARCH); expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); }); }); From 7edb9d4703aed94da00c8e7500b2b91655aaebde Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 16 Jul 2024 13:24:05 +0530 Subject: [PATCH 25/36] Rename test --- ...KeyboardLandmarkUtils-test.tsx => LandmarkNavigation-test.tsx} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/accessibility/{KeyboardLandmarkUtils-test.tsx => LandmarkNavigation-test.tsx} (100%) diff --git a/test/accessibility/KeyboardLandmarkUtils-test.tsx b/test/accessibility/LandmarkNavigation-test.tsx similarity index 100% rename from test/accessibility/KeyboardLandmarkUtils-test.tsx rename to test/accessibility/LandmarkNavigation-test.tsx From b493ac28f167858d40ec6f1b3b8e409e7129625d Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 16 Jul 2024 13:24:24 +0530 Subject: [PATCH 26/36] Change implementation - Use a circular array to store the order of the landmarks - Support skipping over missing landmarks --- src/accessibility/LandmarkNavigation.ts | 66 ++++++++++++++++++------- 1 file changed, 48 insertions(+), 18 deletions(-) diff --git a/src/accessibility/LandmarkNavigation.ts b/src/accessibility/LandmarkNavigation.ts index 54ac6177641..5c76be0bc99 100644 --- a/src/accessibility/LandmarkNavigation.ts +++ b/src/accessibility/LandmarkNavigation.ts @@ -49,45 +49,72 @@ export class LandmarkNavigation { } /** - * Find the next landmark from a given landmark + * Get the next/previous landmark that must be focused from a given landmark * @param currentLandmark The current landmark - * @returns The next landmark in sequence + * @param backwards If true, the landmark before currentLandmark in ORDERED_LANDMARKS is returned + * @returns The next landmark to focus */ - public static navigateToNextLandmarkFrom(currentLandmark: Landmark): void { + private static getLandmark(currentLandmark: Landmark, backwards = false): Landmark { const currentIndex = ORDERED_LANDMARKS.findIndex((l) => l === currentLandmark); - const nextIndex = LandmarkNavigation.getIndexInArray(currentIndex + 1); + const offset = backwards ? -1 : 1; + const nextIndex = LandmarkNavigation.getIndexInArray(currentIndex + offset); const newLandmark = ORDERED_LANDMARKS[nextIndex]; - focusFunctions[newLandmark](); + return newLandmark; + } + + /** + * + * @param currentLandmark The current landmark + * @param backwards If true, search the next landmark to the left in ORDERED_LANDMARKS + */ + private static findAndFocusLandmark(currentLandmark: Landmark, backwards = false): void { + let somethingWasFocused = false; + let landmark = currentLandmark; + while (!somethingWasFocused) { + landmark = LandmarkNavigation.getLandmark(landmark, backwards); + somethingWasFocused = focusFunctions[landmark](); + } + } + + /** + * Find and focus the next landmark from a given landmark + * @param currentLandmark The current landmark + */ + public static navigateToNextLandmarkFrom(currentLandmark: Landmark): void { + LandmarkNavigation.findAndFocusLandmark(currentLandmark); } /** - * Find the previous landmark from a given landmark + * Find and focus the previous landmark from a given landmark * @param currentLandmark The current landmark - * @returns The previous landmark */ public static navigateToPreviousLandmarkFrom(currentLandmark: Landmark): void { - const currentIndex = ORDERED_LANDMARKS.findIndex((l) => l === currentLandmark); - const nextIndex = LandmarkNavigation.getIndexInArray(currentIndex - 1); - const newLandmark = ORDERED_LANDMARKS[nextIndex]; - focusFunctions[newLandmark](); + LandmarkNavigation.findAndFocusLandmark(currentLandmark, true); } } + /** * Keys are the different landmarks and the values are function which when invoked - * gives focus to the corresponding landmark. + * gives focus to the corresponding landmark. These functions return a boolean + * indicating whether focus was successfully given. */ const focusFunctions = { [Landmark.ACTIVE_SPACE_BUTTON]: () => { - document.querySelector(".mx_SpaceButton_active")?.focus(); + const e = document.querySelector(".mx_SpaceButton_active"); + e?.focus(); + return !!e; }, [Landmark.ROOM_SEARCH]: () => { - document.querySelector(".mx_RoomSearch")?.focus(); + const e = document.querySelector(".mx_RoomSearch"); + e?.focus(); + return !!e; }, [Landmark.ROOM_LIST]: () => { - ( + const e = document.querySelector(".mx_RoomTile_selected") || - document.querySelector(".mx_RoomTile") - )?.focus(); + document.querySelector(".mx_RoomTile"); + e?.focus(); + return !!e; }, [Landmark.MESSAGE_COMPOSER_OR_HOME]: () => { const isComposerOpen = !!document.querySelector(".mx_MessageComposer"); @@ -100,8 +127,11 @@ const focusFunctions = { }, true, ); + return true; } else { - document.querySelector(".mx_HomePage")?.focus(); + const e = document.querySelector(".mx_HomePage"); + e?.focus(); + return !!e; } }, }; From 13968620ebb1ede453f08138ae00215e30b29b13 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 16 Jul 2024 13:25:28 +0530 Subject: [PATCH 27/36] Add playwright test --- .../accessibility/keyboard-navigation.spec.ts | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 playwright/e2e/accessibility/keyboard-navigation.spec.ts diff --git a/playwright/e2e/accessibility/keyboard-navigation.spec.ts b/playwright/e2e/accessibility/keyboard-navigation.spec.ts new file mode 100644 index 00000000000..0464a420298 --- /dev/null +++ b/playwright/e2e/accessibility/keyboard-navigation.spec.ts @@ -0,0 +1,149 @@ +/* +Copyright 2024 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { test, expect } from "../../element-web-test"; +import { Bot } from "../../pages/bot"; + +test.describe("Landmark navigation tests", () => { + test.use({ + displayName: "Alice", + }); + + test("without any rooms", async ({ page, homeserver, app, user }) => { + /** + * Without any rooms, there is no tile in the roomlist to be focused. + * So the next landmark in the list should be focused instead. + */ + + // Pressing Control+F6 will first focus the space button + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); + + // Pressing Control+F6 again will focus room search + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_RoomSearch")).toBeFocused(); + + // Pressing Control+F6 again will focus the message composer + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_HomePage")).toBeFocused(); + + // Pressing Control+F6 again will bring focus back to the space button + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); + + // Now go back in the same order + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_HomePage")).toBeFocused(); + + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_RoomSearch")).toBeFocused(); + + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); + }); + + test("in a room", async ({ page, homeserver, app, user }) => { + const bob = new Bot(page, homeserver, { displayName: "Bob" }); + await bob.prepareClient(); + + // create dms with bob and charlie + await app.client.evaluate( + async (cli, { bob }) => { + const bobRoom = await cli.createRoom({ is_direct: true }); + await cli.invite(bobRoom.room_id, bob); + }, + { + bob: bob.credentials.userId, + }, + ); + + await app.viewRoomByName("Bob"); + // confirm the room was loaded + await expect(page.getByText("Bob joined the room")).toBeVisible(); + + // Pressing Control+F6 will first focus the space button + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); + + // Pressing Control+F6 again will focus room search + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_RoomSearch")).toBeFocused(); + + // Pressing Control+F6 again will focus the room tile in the room list + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_RoomTile_selected")).toBeFocused(); + + // Pressing Control+F6 again will focus the message composer + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_BasicMessageComposer_input")).toBeFocused(); + + // Pressing Control+F6 again will bring focus back to the space button + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); + + // Now go back in the same order + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_BasicMessageComposer_input")).toBeFocused(); + + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_RoomTile_selected")).toBeFocused(); + + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_RoomSearch")).toBeFocused(); + + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); + }); + + test("without a room", async ({ page, homeserver, app, user }) => { + const bob = new Bot(page, homeserver, { displayName: "Bob" }); + await bob.prepareClient(); + + // create dms with bob and charlie + await app.client.evaluate( + async (cli, { bob }) => { + const bobRoom = await cli.createRoom({ is_direct: true }); + await cli.invite(bobRoom.room_id, bob); + }, + { + bob: bob.credentials.userId, + }, + ); + + await app.viewRoomByName("Bob"); + // confirm the room was loaded + await expect(page.getByText("Bob joined the room")).toBeVisible(); + + // Close the room + page.goto("/#/home"); + + // Pressing Control+F6 will first focus the space button + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); + + // Pressing Control+F6 again will focus room search + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_RoomSearch")).toBeFocused(); + + // Pressing Control+F6 again will focus the room tile in the room list + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_RoomTile")).toBeFocused(); + + // Pressing Control+F6 again will focus the home section + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_HomePage")).toBeFocused(); + }); +}); From 477f58ed9e621b2ed5dc29486fb5d9d8c9d03074 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 16 Jul 2024 16:14:52 +0530 Subject: [PATCH 28/36] Add more tests --- .../accessibility/keyboard-navigation.spec.ts | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/playwright/e2e/accessibility/keyboard-navigation.spec.ts b/playwright/e2e/accessibility/keyboard-navigation.spec.ts index 0464a420298..b4b74f51877 100644 --- a/playwright/e2e/accessibility/keyboard-navigation.spec.ts +++ b/playwright/e2e/accessibility/keyboard-navigation.spec.ts @@ -55,11 +55,11 @@ test.describe("Landmark navigation tests", () => { await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); }); - test("in a room", async ({ page, homeserver, app, user }) => { + test("with an open room", async ({ page, homeserver, app, user }) => { const bob = new Bot(page, homeserver, { displayName: "Bob" }); await bob.prepareClient(); - // create dms with bob and charlie + // create dm with bob await app.client.evaluate( async (cli, { bob }) => { const bobRoom = await cli.createRoom({ is_direct: true }); @@ -108,11 +108,11 @@ test.describe("Landmark navigation tests", () => { await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); }); - test("without a room", async ({ page, homeserver, app, user }) => { + test("without an open room", async ({ page, homeserver, app, user }) => { const bob = new Bot(page, homeserver, { displayName: "Bob" }); await bob.prepareClient(); - // create dms with bob and charlie + // create a dm with bob await app.client.evaluate( async (cli, { bob }) => { const bobRoom = await cli.createRoom({ is_direct: true }); @@ -145,5 +145,22 @@ test.describe("Landmark navigation tests", () => { // Pressing Control+F6 again will focus the home section await page.keyboard.press("ControlOrMeta+F6"); await expect(page.locator(".mx_HomePage")).toBeFocused(); + + // Pressing Control+F6 will bring focus back to the space button + await page.keyboard.press("ControlOrMeta+F6"); + await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); + + // Now go back in same order + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_HomePage")).toBeFocused(); + + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_RoomTile")).toBeFocused(); + + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_RoomSearch")).toBeFocused(); + + await page.keyboard.press("ControlOrMeta+Shift+F6"); + await expect(page.locator(".mx_SpaceButton_active")).toBeFocused(); }); }); From 6c8e19131ef6e8c661e4db2ca4f0cdff318646e5 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 16 Jul 2024 16:15:44 +0530 Subject: [PATCH 29/36] Fix comments and name --- src/accessibility/LandmarkNavigation.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/accessibility/LandmarkNavigation.ts b/src/accessibility/LandmarkNavigation.ts index 5c76be0bc99..162061ebb7f 100644 --- a/src/accessibility/LandmarkNavigation.ts +++ b/src/accessibility/LandmarkNavigation.ts @@ -63,11 +63,12 @@ export class LandmarkNavigation { } /** - * + * Focus the next landmark from a given landmark. + * This method will skip over any missing landmarks. * @param currentLandmark The current landmark * @param backwards If true, search the next landmark to the left in ORDERED_LANDMARKS */ - private static findAndFocusLandmark(currentLandmark: Landmark, backwards = false): void { + private static findAndFocusNextLandmark(currentLandmark: Landmark, backwards = false): void { let somethingWasFocused = false; let landmark = currentLandmark; while (!somethingWasFocused) { @@ -81,7 +82,7 @@ export class LandmarkNavigation { * @param currentLandmark The current landmark */ public static navigateToNextLandmarkFrom(currentLandmark: Landmark): void { - LandmarkNavigation.findAndFocusLandmark(currentLandmark); + LandmarkNavigation.findAndFocusNextLandmark(currentLandmark); } /** @@ -89,7 +90,7 @@ export class LandmarkNavigation { * @param currentLandmark The current landmark */ public static navigateToPreviousLandmarkFrom(currentLandmark: Landmark): void { - LandmarkNavigation.findAndFocusLandmark(currentLandmark, true); + LandmarkNavigation.findAndFocusNextLandmark(currentLandmark, true); } } From b2ce73b04ce066b4d325afcee4ca36d6ab56c74f Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 16 Jul 2024 18:35:40 +0530 Subject: [PATCH 30/36] Replacee method with Array.at --- src/accessibility/LandmarkNavigation.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/accessibility/LandmarkNavigation.ts b/src/accessibility/LandmarkNavigation.ts index 162061ebb7f..985bba7004b 100644 --- a/src/accessibility/LandmarkNavigation.ts +++ b/src/accessibility/LandmarkNavigation.ts @@ -41,13 +41,6 @@ const ORDERED_LANDMARKS = [ * ACTIVE_SPACE_BUTTON <-> ROOM_SEARCH <-> ROOM_LIST <-> MESSAGE_COMPOSER/HOME <-> ACTIVE_SPACE_BUTTON */ export class LandmarkNavigation { - private static getIndexInArray(i: number): number { - const n = ORDERED_LANDMARKS.length; - // calculating index in the circular array - const nextIndex = ((i % n) + n) % n; - return nextIndex; - } - /** * Get the next/previous landmark that must be focused from a given landmark * @param currentLandmark The current landmark @@ -57,8 +50,7 @@ export class LandmarkNavigation { private static getLandmark(currentLandmark: Landmark, backwards = false): Landmark { const currentIndex = ORDERED_LANDMARKS.findIndex((l) => l === currentLandmark); const offset = backwards ? -1 : 1; - const nextIndex = LandmarkNavigation.getIndexInArray(currentIndex + offset); - const newLandmark = ORDERED_LANDMARKS[nextIndex]; + const newLandmark = ORDERED_LANDMARKS.at((currentIndex + offset) % ORDERED_LANDMARKS.length)!; return newLandmark; } From ea4fea8b96af0c010ce4598b04459d7bc3046549 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 16 Jul 2024 19:09:18 +0530 Subject: [PATCH 31/36] Make changes from review --- src/accessibility/LandmarkNavigation.ts | 65 ++++++------------- src/components/structures/LeftPanel.tsx | 19 +++--- src/components/structures/LoggedInView.tsx | 8 +-- .../views/rooms/BasicMessageComposer.tsx | 17 +++-- src/components/views/rooms/RoomList.tsx | 15 ++--- src/components/views/spaces/SpacePanel.tsx | 15 ++--- .../accessibility/LandmarkNavigation-test.tsx | 32 ++++----- 7 files changed, 67 insertions(+), 104 deletions(-) diff --git a/src/accessibility/LandmarkNavigation.ts b/src/accessibility/LandmarkNavigation.ts index 985bba7004b..0c35cade80f 100644 --- a/src/accessibility/LandmarkNavigation.ts +++ b/src/accessibility/LandmarkNavigation.ts @@ -60,55 +60,31 @@ export class LandmarkNavigation { * @param currentLandmark The current landmark * @param backwards If true, search the next landmark to the left in ORDERED_LANDMARKS */ - private static findAndFocusNextLandmark(currentLandmark: Landmark, backwards = false): void { - let somethingWasFocused = false; + public static findAndFocusNextLandmark(currentLandmark: Landmark, backwards = false): void { let landmark = currentLandmark; - while (!somethingWasFocused) { + let element: HTMLElement | null | undefined = null; + while (element === null) { landmark = LandmarkNavigation.getLandmark(landmark, backwards); - somethingWasFocused = focusFunctions[landmark](); + element = landmarkToDOMElementMap[landmark](); } - } - - /** - * Find and focus the next landmark from a given landmark - * @param currentLandmark The current landmark - */ - public static navigateToNextLandmarkFrom(currentLandmark: Landmark): void { - LandmarkNavigation.findAndFocusNextLandmark(currentLandmark); - } - - /** - * Find and focus the previous landmark from a given landmark - * @param currentLandmark The current landmark - */ - public static navigateToPreviousLandmarkFrom(currentLandmark: Landmark): void { - LandmarkNavigation.findAndFocusNextLandmark(currentLandmark, true); + element?.focus(); } } /** - * Keys are the different landmarks and the values are function which when invoked - * gives focus to the corresponding landmark. These functions return a boolean - * indicating whether focus was successfully given. + * The functions return: + * - The DOM element of the landmark if it exists + * - undefined if the DOM element exists but focus is given through an action + * - null if the landmark does not exist */ -const focusFunctions = { - [Landmark.ACTIVE_SPACE_BUTTON]: () => { - const e = document.querySelector(".mx_SpaceButton_active"); - e?.focus(); - return !!e; - }, - [Landmark.ROOM_SEARCH]: () => { - const e = document.querySelector(".mx_RoomSearch"); - e?.focus(); - return !!e; - }, - [Landmark.ROOM_LIST]: () => { - const e = - document.querySelector(".mx_RoomTile_selected") || - document.querySelector(".mx_RoomTile"); - e?.focus(); - return !!e; - }, +const landmarkToDOMElementMap = { + [Landmark.ACTIVE_SPACE_BUTTON]: () => document.querySelector(".mx_SpaceButton_active"), + + [Landmark.ROOM_SEARCH]: () => document.querySelector(".mx_RoomSearch"), + [Landmark.ROOM_LIST]: () => + document.querySelector(".mx_RoomTile_selected") || + document.querySelector(".mx_RoomTile"), + [Landmark.MESSAGE_COMPOSER_OR_HOME]: () => { const isComposerOpen = !!document.querySelector(".mx_MessageComposer"); if (isComposerOpen) { @@ -120,11 +96,10 @@ const focusFunctions = { }, true, ); - return true; + // Special case where the element does exist but we focus it through an action. + return undefined; } else { - const e = document.querySelector(".mx_HomePage"); - e?.focus(); - return !!e; + return document.querySelector(".mx_HomePage"); } }, }; diff --git a/src/components/structures/LeftPanel.tsx b/src/components/structures/LeftPanel.tsx index 5e97d4d1d44..fe941a1a367 100644 --- a/src/components/structures/LeftPanel.tsx +++ b/src/components/structures/LeftPanel.tsx @@ -309,18 +309,15 @@ export default class LeftPanel extends React.Component { } break; } + const navAction = getKeyBindingsManager().getNavigationAction(ev); - switch (navAction) { - case KeyBindingAction.NextLandmark: - ev.stopPropagation(); - ev.preventDefault(); - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_SEARCH); - break; - case KeyBindingAction.PreviousLandmark: - ev.stopPropagation(); - ev.preventDefault(); - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_SEARCH); - break; + if (navAction) { + ev.stopPropagation(); + ev.preventDefault(); + LandmarkNavigation.findAndFocusNextLandmark( + Landmark.ROOM_SEARCH, + navAction === KeyBindingAction.PreviousLandmark, + ); } }; diff --git a/src/components/structures/LoggedInView.tsx b/src/components/structures/LoggedInView.tsx index 2be0f5c8fa4..755d2c11568 100644 --- a/src/components/structures/LoggedInView.tsx +++ b/src/components/structures/LoggedInView.tsx @@ -472,11 +472,11 @@ class LoggedInView extends React.Component { const navAction = getKeyBindingsManager().getNavigationAction(ev); switch (navAction) { case KeyBindingAction.NextLandmark: - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); - handled = true; - break; case KeyBindingAction.PreviousLandmark: - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); + LandmarkNavigation.findAndFocusNextLandmark( + Landmark.MESSAGE_COMPOSER_OR_HOME, + navAction === KeyBindingAction.PreviousLandmark, + ); handled = true; break; case KeyBindingAction.FilterRooms: diff --git a/src/components/views/rooms/BasicMessageComposer.tsx b/src/components/views/rooms/BasicMessageComposer.tsx index c2d6205bf05..ef6a3b73812 100644 --- a/src/components/views/rooms/BasicMessageComposer.tsx +++ b/src/components/views/rooms/BasicMessageComposer.tsx @@ -538,16 +538,15 @@ export default class BasicMessageEditor extends React.Component } const navAction = getKeyBindingsManager().getNavigationAction(event); - switch (navAction) { - case KeyBindingAction.PreviousLandmark: - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); - handled = true; - break; - case KeyBindingAction.NextLandmark: - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); - handled = true; - break; + + if (navAction) { + LandmarkNavigation.findAndFocusNextLandmark( + Landmark.MESSAGE_COMPOSER_OR_HOME, + navAction === KeyBindingAction.PreviousLandmark, + ); + handled = true; } + const autocompleteAction = getKeyBindingsManager().getAutocompleteAction(event); const accessibilityAction = getKeyBindingsManager().getAccessibilityAction(event); if (model.autoComplete?.hasCompletions()) { diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 8b2ee88d230..376b545b5a1 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -657,20 +657,15 @@ export default class RoomList extends React.PureComponent { onBlur={this.props.onBlur} onKeyDown={(ev) => { const navAction = getKeyBindingsManager().getNavigationAction(ev); - if (navAction === KeyBindingAction.NextLandmark) { - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_LIST); + if (navAction) { + LandmarkNavigation.findAndFocusNextLandmark( + Landmark.ROOM_LIST, + navAction === KeyBindingAction.PreviousLandmark, + ); ev.stopPropagation(); ev.preventDefault(); return; } - - if (navAction === KeyBindingAction.PreviousLandmark) { - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_LIST); - ev.stopPropagation(); - ev.preventDefault(); - return; - } - onKeyDownHandler(ev); }} className="mx_RoomList" diff --git a/src/components/views/spaces/SpacePanel.tsx b/src/components/views/spaces/SpacePanel.tsx index f6c7ce05a2b..fecaee93542 100644 --- a/src/components/views/spaces/SpacePanel.tsx +++ b/src/components/views/spaces/SpacePanel.tsx @@ -388,19 +388,16 @@ const SpacePanel: React.FC = () => { className={classNames("mx_SpacePanel", { collapsed: isPanelCollapsed })} onKeyDown={(ev) => { const navAction = getKeyBindingsManager().getNavigationAction(ev); - if (navAction === KeyBindingAction.PreviousLandmark) { - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); - ev.stopPropagation(); - ev.preventDefault(); - return; - } - if (navAction === KeyBindingAction.NextLandmark) { - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); + + if (navAction) { + LandmarkNavigation.findAndFocusNextLandmark( + Landmark.ACTIVE_SPACE_BUTTON, + navAction === KeyBindingAction.PreviousLandmark, + ); ev.stopPropagation(); ev.preventDefault(); return; } - onKeyDownHandler(ev); }} ref={ref} diff --git a/test/accessibility/LandmarkNavigation-test.tsx b/test/accessibility/LandmarkNavigation-test.tsx index 4b0375f6680..65d5315b3d3 100644 --- a/test/accessibility/LandmarkNavigation-test.tsx +++ b/test/accessibility/LandmarkNavigation-test.tsx @@ -40,35 +40,35 @@ describe("KeyboardLandmarkUtils", () => { ); // ACTIVE_SPACE_BUTTON <-> ROOM_SEARCH <-> ROOM_LIST <-> HOME <-> ACTIVE_SPACE_BUTTON // ACTIVE_SPACE_BUTTON -> ROOM_SEARCH - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ACTIVE_SPACE_BUTTON); expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); // ROOM_SEARCH -> ROOM_LIST - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_SEARCH); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ROOM_SEARCH); expect(screen.getByTestId("mx_RoomTile")).toHaveFocus(); // ROOM_LIST -> HOME_PAGE - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_LIST); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ROOM_LIST); expect(screen.getByTestId("mx_HomePage")).toHaveFocus(); // HOME_PAGE -> ACTIVE_SPACE_BUTTON - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.MESSAGE_COMPOSER_OR_HOME); expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); // HOME_PAGE <- ACTIVE_SPACE_BUTTON - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ACTIVE_SPACE_BUTTON, true); expect(screen.getByTestId("mx_HomePage")).toHaveFocus(); // ROOM_LIST <- HOME_PAGE - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.MESSAGE_COMPOSER_OR_HOME, true); expect(screen.getByTestId("mx_RoomTile")).toHaveFocus(); // ROOM_SEARCH <- ROOM_LIST - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_LIST); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ROOM_LIST, true); expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); // ACTIVE_SPACE_BUTTON <- ROOM_SEARCH - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_SEARCH); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ROOM_SEARCH, true); expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); }); @@ -96,35 +96,35 @@ describe("KeyboardLandmarkUtils", () => { ); // ACTIVE_SPACE_BUTTON <-> ROOM_SEARCH <-> ROOM_LIST <-> MESSAGE_COMPOSER <-> ACTIVE_SPACE_BUTTON // ACTIVE_SPACE_BUTTON -> ROOM_SEARCH - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ACTIVE_SPACE_BUTTON); expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); // ROOM_SEARCH -> ROOM_LIST - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_SEARCH); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ROOM_SEARCH); expect(screen.getByTestId("mx_RoomTile_selected")).toHaveFocus(); // ROOM_LIST -> MESSAGE_COMPOSER - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.ROOM_LIST); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ROOM_LIST); await waitFor(() => expect(callback).toHaveBeenCalledTimes(1)); // MESSAGE_COMPOSER -> ACTIVE_SPACE_BUTTON - LandmarkNavigation.navigateToNextLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.MESSAGE_COMPOSER_OR_HOME); expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); // MESSAGE_COMPOSER <- ACTIVE_SPACE_BUTTON - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ACTIVE_SPACE_BUTTON); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ACTIVE_SPACE_BUTTON, true); await waitFor(() => expect(callback).toHaveBeenCalledTimes(2)); // ROOM_LIST <- MESSAGE_COMPOSER - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.MESSAGE_COMPOSER_OR_HOME); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.MESSAGE_COMPOSER_OR_HOME, true); expect(screen.getByTestId("mx_RoomTile_selected")).toHaveFocus(); // ROOM_SEARCH <- ROOM_LIST - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_LIST); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ROOM_LIST, true); expect(screen.getByTestId("mx_RoomSearch")).toHaveFocus(); // ACTIVE_SPACE_BUTTON <- ROOM_SEARCH - LandmarkNavigation.navigateToPreviousLandmarkFrom(Landmark.ROOM_SEARCH); + LandmarkNavigation.findAndFocusNextLandmark(Landmark.ROOM_SEARCH, true); expect(screen.getByTestId("mx_SpaceButton_active")).toHaveFocus(); }); }); From 0961caea83d25b7d70febb49871e63f0ec6b10de Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 16 Jul 2024 19:26:24 +0530 Subject: [PATCH 32/36] Fix case; landmarkToDOMElementMap -> landmarkToDomElementMap --- src/accessibility/LandmarkNavigation.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/accessibility/LandmarkNavigation.ts b/src/accessibility/LandmarkNavigation.ts index 0c35cade80f..1dce5481220 100644 --- a/src/accessibility/LandmarkNavigation.ts +++ b/src/accessibility/LandmarkNavigation.ts @@ -65,7 +65,7 @@ export class LandmarkNavigation { let element: HTMLElement | null | undefined = null; while (element === null) { landmark = LandmarkNavigation.getLandmark(landmark, backwards); - element = landmarkToDOMElementMap[landmark](); + element = landmarkToDomElementMap[landmark](); } element?.focus(); } @@ -77,7 +77,7 @@ export class LandmarkNavigation { * - undefined if the DOM element exists but focus is given through an action * - null if the landmark does not exist */ -const landmarkToDOMElementMap = { +const landmarkToDomElementMap = { [Landmark.ACTIVE_SPACE_BUTTON]: () => document.querySelector(".mx_SpaceButton_active"), [Landmark.ROOM_SEARCH]: () => document.querySelector(".mx_RoomSearch"), From 114cc77acdcc8b9527e139710e162a951896d28b Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 16 Jul 2024 19:37:57 +0530 Subject: [PATCH 33/36] Add stricter check --- src/components/structures/LeftPanel.tsx | 2 +- src/components/views/rooms/BasicMessageComposer.tsx | 2 +- src/components/views/rooms/RoomList.tsx | 5 ++++- src/components/views/spaces/SpacePanel.tsx | 6 ++++-- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/components/structures/LeftPanel.tsx b/src/components/structures/LeftPanel.tsx index fe941a1a367..7ef3bc4ba31 100644 --- a/src/components/structures/LeftPanel.tsx +++ b/src/components/structures/LeftPanel.tsx @@ -311,7 +311,7 @@ export default class LeftPanel extends React.Component { } const navAction = getKeyBindingsManager().getNavigationAction(ev); - if (navAction) { + if (navAction === KeyBindingAction.PreviousLandmark || navAction === KeyBindingAction.NextLandmark) { ev.stopPropagation(); ev.preventDefault(); LandmarkNavigation.findAndFocusNextLandmark( diff --git a/src/components/views/rooms/BasicMessageComposer.tsx b/src/components/views/rooms/BasicMessageComposer.tsx index ef6a3b73812..38576dc2550 100644 --- a/src/components/views/rooms/BasicMessageComposer.tsx +++ b/src/components/views/rooms/BasicMessageComposer.tsx @@ -539,7 +539,7 @@ export default class BasicMessageEditor extends React.Component const navAction = getKeyBindingsManager().getNavigationAction(event); - if (navAction) { + if (navAction === KeyBindingAction.NextLandmark || navAction === KeyBindingAction.PreviousLandmark) { LandmarkNavigation.findAndFocusNextLandmark( Landmark.MESSAGE_COMPOSER_OR_HOME, navAction === KeyBindingAction.PreviousLandmark, diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 376b545b5a1..d088fbc927a 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -657,7 +657,10 @@ export default class RoomList extends React.PureComponent { onBlur={this.props.onBlur} onKeyDown={(ev) => { const navAction = getKeyBindingsManager().getNavigationAction(ev); - if (navAction) { + if ( + navAction === KeyBindingAction.NextLandmark || + navAction === KeyBindingAction.PreviousLandmark + ) { LandmarkNavigation.findAndFocusNextLandmark( Landmark.ROOM_LIST, navAction === KeyBindingAction.PreviousLandmark, diff --git a/src/components/views/spaces/SpacePanel.tsx b/src/components/views/spaces/SpacePanel.tsx index fecaee93542..5bd9a90850b 100644 --- a/src/components/views/spaces/SpacePanel.tsx +++ b/src/components/views/spaces/SpacePanel.tsx @@ -388,8 +388,10 @@ const SpacePanel: React.FC = () => { className={classNames("mx_SpacePanel", { collapsed: isPanelCollapsed })} onKeyDown={(ev) => { const navAction = getKeyBindingsManager().getNavigationAction(ev); - - if (navAction) { + if ( + navAction === KeyBindingAction.NextLandmark || + navAction === KeyBindingAction.PreviousLandmark + ) { LandmarkNavigation.findAndFocusNextLandmark( Landmark.ACTIVE_SPACE_BUTTON, navAction === KeyBindingAction.PreviousLandmark, From 1d8ab2a1bc886d44e8f0ddc0d955da7281a43193 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 16 Jul 2024 19:51:56 +0530 Subject: [PATCH 34/36] Add type to map --- src/accessibility/LandmarkNavigation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/accessibility/LandmarkNavigation.ts b/src/accessibility/LandmarkNavigation.ts index 1dce5481220..f8cb34c646b 100644 --- a/src/accessibility/LandmarkNavigation.ts +++ b/src/accessibility/LandmarkNavigation.ts @@ -77,7 +77,7 @@ export class LandmarkNavigation { * - undefined if the DOM element exists but focus is given through an action * - null if the landmark does not exist */ -const landmarkToDomElementMap = { +const landmarkToDomElementMap: Record HTMLElement | null | undefined> = { [Landmark.ACTIVE_SPACE_BUTTON]: () => document.querySelector(".mx_SpaceButton_active"), [Landmark.ROOM_SEARCH]: () => document.querySelector(".mx_RoomSearch"), From 9f29735f47a7ed7057d44273ba46f95485ea634e Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Wed, 17 Jul 2024 18:37:12 +0530 Subject: [PATCH 35/36] Pass focusVisible option to focus call --- src/accessibility/LandmarkNavigation.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/accessibility/LandmarkNavigation.ts b/src/accessibility/LandmarkNavigation.ts index f8cb34c646b..9ff05ef2e71 100644 --- a/src/accessibility/LandmarkNavigation.ts +++ b/src/accessibility/LandmarkNavigation.ts @@ -18,6 +18,12 @@ import { TimelineRenderingType } from "../contexts/RoomContext"; import { Action } from "../dispatcher/actions"; import defaultDispatcher from "../dispatcher/dispatcher"; +/** + * In future, browsers will support focusVisible option. + * See https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus#focusvisible + */ +type ExperimentalFocusOptions = FocusOptions & { focusVisible: true }; + export const enum Landmark { // This is the space/home button in the left panel. ACTIVE_SPACE_BUTTON, @@ -67,7 +73,7 @@ export class LandmarkNavigation { landmark = LandmarkNavigation.getLandmark(landmark, backwards); element = landmarkToDomElementMap[landmark](); } - element?.focus(); + element?.focus({ focusVisible: true } as ExperimentalFocusOptions); } } From d1085b7b8ef877d1481788b849c22ce805461b33 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Wed, 17 Jul 2024 18:49:25 +0530 Subject: [PATCH 36/36] Move type to global.d.ts --- src/@types/global.d.ts | 8 ++++++++ src/accessibility/LandmarkNavigation.ts | 8 +------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index c62733c0f07..e42e83d58db 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -224,6 +224,14 @@ declare global { readonly port: MessagePort; } + /** + * In future, browsers will support focusVisible option. + * See https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus#focusvisible + */ + interface FocusOptions { + focusVisible: boolean; + } + // https://github.com/microsoft/TypeScript/issues/28308#issuecomment-650802278 function registerProcessor( name: string, diff --git a/src/accessibility/LandmarkNavigation.ts b/src/accessibility/LandmarkNavigation.ts index 9ff05ef2e71..50ec478d2a0 100644 --- a/src/accessibility/LandmarkNavigation.ts +++ b/src/accessibility/LandmarkNavigation.ts @@ -18,12 +18,6 @@ import { TimelineRenderingType } from "../contexts/RoomContext"; import { Action } from "../dispatcher/actions"; import defaultDispatcher from "../dispatcher/dispatcher"; -/** - * In future, browsers will support focusVisible option. - * See https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus#focusvisible - */ -type ExperimentalFocusOptions = FocusOptions & { focusVisible: true }; - export const enum Landmark { // This is the space/home button in the left panel. ACTIVE_SPACE_BUTTON, @@ -73,7 +67,7 @@ export class LandmarkNavigation { landmark = LandmarkNavigation.getLandmark(landmark, backwards); element = landmarkToDomElementMap[landmark](); } - element?.focus({ focusVisible: true } as ExperimentalFocusOptions); + element?.focus({ focusVisible: true }); } }