Merge pull request #4911 from matrix-org/t3chguy/room-list/113

Show more/Show less keep focus in a relevant place

src/accessibility/RovingTabIndex.tsx

@@ -22,9 +22,13 @@ import React, {
     useMemo,
     useRef,
     useReducer,
+    Reducer,
+    RefObject,
+    Dispatch,
 } from "react";
-import PropTypes from "prop-types";
+
 import {Key} from "../Keyboard";
+import AccessibleButton from "../components/views/elements/AccessibleButton";
 
 /**
  * Module to simplify implementing the Roving TabIndex accessibility technique
@@ -41,7 +45,19 @@ import {Key} from "../Keyboard";
 
 const DOCUMENT_POSITION_PRECEDING = 2;
 
-const RovingTabIndexContext = createContext({
+type Ref = RefObject<HTMLElement>;
+
+interface IState {
+    activeRef: Ref;
+    refs: Ref[];
+}
+
+interface IContext {
+    state: IState;
+    dispatch: Dispatch<IAction>;
+}
+
+const RovingTabIndexContext = createContext<IContext>({
     state: {
         activeRef: null,
         refs: [], // list of refs in DOM order
@@ -50,16 +66,22 @@ const RovingTabIndexContext = createContext({
 });
 RovingTabIndexContext.displayName = "RovingTabIndexContext";
 
-// TODO use a TypeScript type here
+enum Type {
-const types = {
+    Register = "REGISTER",
-    REGISTER: "REGISTER",
+    Unregister = "UNREGISTER",
-    UNREGISTER: "UNREGISTER",
+    SetFocus = "SET_FOCUS",
-    SET_FOCUS: "SET_FOCUS",
+}
-};
 
-const reducer = (state, action) => {
+interface IAction {
+    type: Type;
+    payload: {
+        ref: Ref;
+    };
+}
+
+const reducer = (state: IState, action: IAction) => {
     switch (action.type) {
-        case types.REGISTER: {
+        case Type.Register: {
             if (state.refs.length === 0) {
                 // Our list of refs was empty, set activeRef to this first item
                 return {
@@ -92,7 +114,7 @@ const reducer = (state, action) => {
                 ],
             };
         }
-        case types.UNREGISTER: {
+        case Type.Unregister: {
             // filter out the ref which we are removing
             const refs = state.refs.filter(r => r !== action.payload.ref);
 
@@ -117,7 +139,7 @@ const reducer = (state, action) => {
                 refs,
             };
         }
-        case types.SET_FOCUS: {
+        case Type.SetFocus: {
             // update active ref
             return {
                 ...state,
@@ -129,13 +151,21 @@ const reducer = (state, action) => {
     }
 };
 
-export const RovingTabIndexProvider = ({children, handleHomeEnd, onKeyDown}) => {
+interface IProps {
-    const [state, dispatch] = useReducer(reducer, {
+    handleHomeEnd?: boolean;
+    children(renderProps: {
+        onKeyDownHandler(ev: React.KeyboardEvent);
+    });
+    onKeyDown?(ev: React.KeyboardEvent);
+}
+
+export const RovingTabIndexProvider: React.FC<IProps> = ({children, handleHomeEnd, onKeyDown}) => {
+    const [state, dispatch] = useReducer<Reducer<IState, IAction>>(reducer, {
         activeRef: null,
         refs: [],
     });
 
-    const context = useMemo(() => ({state, dispatch}), [state]);
+    const context = useMemo<IContext>(() => ({state, dispatch}), [state]);
 
     const onKeyDownHandler = useCallback((ev) => {
         let handled = false;
@@ -171,19 +201,17 @@ export const RovingTabIndexProvider = ({children, handleHomeEnd, onKeyDown}) =>
         { children({onKeyDownHandler}) }
     </RovingTabIndexContext.Provider>;
 };
-RovingTabIndexProvider.propTypes = {
+
-    handleHomeEnd: PropTypes.bool,
+type FocusHandler = () => void;
-    onKeyDown: PropTypes.func,
-};
 
 // Hook to register a roving tab index
 // inputRef parameter specifies the ref to use
 // onFocus should be called when the index gained focus in any manner
 // isActive should be used to set tabIndex in a manner such as `tabIndex={isActive ? 0 : -1}`
 // ref should be passed to a DOM node which will be used for DOM compareDocumentPosition
-export const useRovingTabIndex = (inputRef) => {
+export const useRovingTabIndex = (inputRef: Ref): [FocusHandler, boolean, Ref] => {
     const context = useContext(RovingTabIndexContext);
-    let ref = useRef(null);
+    let ref = useRef<HTMLElement>(null);
 
     if (inputRef) {
         // if we are given a ref, use it instead of ours
@@ -193,13 +221,13 @@ export const useRovingTabIndex = (inputRef) => {
     // setup (after refs)
     useLayoutEffect(() => {
         context.dispatch({
-            type: types.REGISTER,
+            type: Type.Register,
             payload: {ref},
         });
         // teardown
         return () => {
             context.dispatch({
-                type: types.UNREGISTER,
+                type: Type.Unregister,
                 payload: {ref},
             });
         };
@@ -207,7 +235,7 @@ export const useRovingTabIndex = (inputRef) => {
 
     const onFocus = useCallback(() => {
         context.dispatch({
-            type: types.SET_FOCUS,
+            type: Type.SetFocus,
             payload: {ref},
         });
     }, [ref, context]);
@@ -216,9 +244,28 @@ export const useRovingTabIndex = (inputRef) => {
     return [onFocus, isActive, ref];
 };
 
+interface IRovingTabIndexWrapperProps {
+    inputRef?: Ref;
+    children(renderProps: {
+        onFocus: FocusHandler;
+        isActive: boolean;
+        ref: Ref;
+    });
+}
+
 // Wrapper to allow use of useRovingTabIndex outside of React Functional Components.
-export const RovingTabIndexWrapper = ({children, inputRef}) => {
+export const RovingTabIndexWrapper: React.FC<IRovingTabIndexWrapperProps> = ({children, inputRef}) => {
     const [onFocus, isActive, ref] = useRovingTabIndex(inputRef);
     return children({onFocus, isActive, ref});
 };
 
+interface IRovingAccessibleButtonProps extends React.ComponentProps<typeof AccessibleButton> {
+    inputRef?: Ref;
+}
+
+// Wrapper to allow use of useRovingTabIndex for simple AccessibleButtons outside of React Functional Components.
+export const RovingAccessibleButton: React.FC<IRovingAccessibleButtonProps> = ({inputRef, ...props}) => {
+    const [onFocus, isActive, ref] = useRovingTabIndex(inputRef);
+    return <AccessibleButton {...props} onFocus={onFocus} inputRef={ref} tabIndex={isActive ? 0 : -1} />;
+};
+

src/components/structures/LeftPanel2.tsx

@@ -70,8 +70,6 @@ export default class LeftPanel2 extends React.Component<IProps, IState> {
     private tagPanelWatcherRef: string;
     private focusedElement = null;
 
-    // TODO: a11y: https://github.com/vector-im/riot-web/issues/14180
-
     constructor(props: IProps) {
         super(props);
 
@@ -264,7 +262,6 @@ export default class LeftPanel2 extends React.Component<IProps, IState> {
                     onVerticalArrow={this.onKeyDown}
                 />
                 <AccessibleButton
-                    // TODO fix the accessibility of this: https://github.com/vector-im/riot-web/issues/14180
                     className="mx_LeftPanel2_exploreButton"
                     onClick={this.onExplore}
                     title={_t("Explore rooms")}

src/components/views/rooms/RoomSublist2.tsx

@@ -20,7 +20,7 @@ import * as React from "react";
 import { createRef } from "react";
 import { Room } from "matrix-js-sdk/src/models/room";
 import classNames from 'classnames';
-import { RovingTabIndexWrapper } from "../../../accessibility/RovingTabIndex";
+import {RovingAccessibleButton, RovingTabIndexWrapper} from "../../../accessibility/RovingTabIndex";
 import { _t } from "../../../languageHandler";
 import AccessibleButton from "../../views/elements/AccessibleButton";
 import RoomTile2 from "./RoomTile2";
@@ -140,15 +140,25 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
     };
 
     private onShowAllClick = () => {
-        // TODO a11y keep focus somewhere useful: https://github.com/vector-im/riot-web/issues/14180
+        const numVisibleTiles = this.numVisibleTiles;
         this.props.layout.visibleTiles = this.props.layout.tilesWithPadding(this.numTiles, MAX_PADDING_HEIGHT);
         this.forceUpdate(); // because the layout doesn't trigger a re-render
+        setImmediate(this.focusRoomTile, numVisibleTiles); // focus the tile after the current bottom one
     };
 
     private onShowLessClick = () => {
-        // TODO a11y keep focus somewhere useful: https://github.com/vector-im/riot-web/issues/14180
         this.props.layout.visibleTiles = this.props.layout.defaultVisibleTiles;
         this.forceUpdate(); // because the layout doesn't trigger a re-render
+        // focus will flow to the show more button here
+    };
+
+    private focusRoomTile = (index: number) => {
+        if (!this.sublistRef.current) return;
+        const elements = this.sublistRef.current.querySelectorAll<HTMLDivElement>(".mx_RoomTile2");
+        const element = elements && elements[index];
+        if (element) {
+            element.focus();
+        }
     };
 
     private onOpenMenuClick = (ev: InputEvent) => {
@@ -450,7 +460,6 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
                         </div>
                     );
 
-                    // TODO: a11y (see old component): https://github.com/vector-im/riot-web/issues/14180
                     // Note: the addRoomButton conditionally gets moved around
                     // the DOM depending on whether or not the list is minimized.
                     // If we're minimized, we want it below the header so it
@@ -521,12 +530,12 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
                 );
                 if (this.props.isMinimized) showMoreText = null;
                 showNButton = (
-                    <AccessibleButton onClick={this.onShowAllClick} className={showMoreBtnClasses} tabIndex={-1}>
+                    <RovingAccessibleButton onClick={this.onShowAllClick} className={showMoreBtnClasses}>
                         <span className='mx_RoomSublist2_showMoreButtonChevron mx_RoomSublist2_showNButtonChevron'>
                             {/* set by CSS masking */}
                         </span>
                         {showMoreText}
-                    </AccessibleButton>
+                    </RovingAccessibleButton>
                 );
             } else if (this.numTiles <= visibleTiles.length && this.numTiles > this.props.layout.defaultVisibleTiles) {
                 // we have all tiles visible - add a button to show less
@@ -536,14 +545,13 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
                     </span>
                 );
                 if (this.props.isMinimized) showLessText = null;
-                // TODO Roving tab index / treeitem?: https://github.com/vector-im/riot-web/issues/14180
                 showNButton = (
-                    <AccessibleButton onClick={this.onShowLessClick} className={showMoreBtnClasses} tabIndex={-1}>
+                    <RovingAccessibleButton onClick={this.onShowLessClick} className={showMoreBtnClasses}>
                         <span className='mx_RoomSublist2_showLessButtonChevron mx_RoomSublist2_showNButtonChevron'>
                             {/* set by CSS masking */}
                         </span>
                         {showLessText}
-                    </AccessibleButton>
+                    </RovingAccessibleButton>
                 );
             }
 

src/components/views/rooms/RoomTile2.tsx

@@ -118,8 +118,6 @@ const NotifOption: React.FC<INotifOptionProps> = ({active, onClick, iconClassNam
 };
 
 export default class RoomTile2 extends React.Component<IProps, IState> {
-    // TODO: a11y: https://github.com/vector-im/riot-web/issues/14180
-
     constructor(props: IProps) {
         super(props);
 
@@ -390,7 +388,6 @@ export default class RoomTile2 extends React.Component<IProps, IState> {
 
     public render(): React.ReactElement {
         // TODO: Invites: https://github.com/vector-im/riot-web/issues/14198
-        // TODO: a11y proper: https://github.com/vector-im/riot-web/issues/14180
 
         const classes = classNames({
             'mx_RoomTile2': true,