Merge pull request #3895 from matrix-org/t3chguy/roving

Fix roving room list for resizer and ff tabstop a11y

src/accessibility/RovingTabIndex.js

@@ -129,7 +129,7 @@ const reducer = (state, action) => {
     }
 };
 
-export const RovingTabIndexProvider = ({children, handleHomeEnd}) => {
+export const RovingTabIndexProvider = ({children, handleHomeEnd, onKeyDown}) => {
     const [state, dispatch] = useReducer(reducer, {
         activeRef: null,
         refs: [],
@@ -137,53 +137,43 @@ export const RovingTabIndexProvider = ({children, handleHomeEnd}) => {
 
     const context = useMemo(() => ({state, dispatch}), [state]);
 
-    if (handleHomeEnd) {
+    const onKeyDownHandler = useCallback((ev) => {
-        return <RovingTabIndexContext.Provider value={context}>
+        let handled = false;
-            <HomeEndHelper>
+        if (handleHomeEnd) {
-                { children }
+            // check if we actually have any items
-            </HomeEndHelper>
+            switch (ev.key) {
-        </RovingTabIndexContext.Provider>;
+                case Key.HOME:
-    }
+                    handled = true;
-
+                    // move focus to first item
-    return <RovingTabIndexContext.Provider value={context}>
+                    if (context.state.refs.length > 0) {
-        { children }
+                        context.state.refs[0].current.focus();
-    </RovingTabIndexContext.Provider>;
+                    }
-};
+                    break;
-RovingTabIndexProvider.propTypes = {
+                case Key.END:
-    handleHomeEnd: PropTypes.bool,
+                    handled = true;
-};
+                    // move focus to last item
-
+                    if (context.state.refs.length > 0) {
-// Helper to handle Home/End to jump to first/last roving-tab-index for widgets such as treeview
+                        context.state.refs[context.state.refs.length - 1].current.focus();
-export const HomeEndHelper = ({children}) => {
+                    }
-    const context = useContext(RovingTabIndexContext);
+                    break;
-
+            }
-    const onKeyDown = useCallback((ev) => {
-        // check if we actually have any items
-        if (context.state.refs.length <= 0) return;
-
-        let handled = true;
-        switch (ev.key) {
-            case Key.HOME:
-                // move focus to first item
-                context.state.refs[0].current.focus();
-                break;
-            case Key.END:
-                // move focus to last item
-                context.state.refs[context.state.refs.length - 1].current.focus();
-                break;
-            default:
-                handled = false;
         }
 
         if (handled) {
             ev.preventDefault();
             ev.stopPropagation();
+        } else if (onKeyDown) {
+            return onKeyDown(ev);
         }
-    }, [context.state]);
+    }, [context.state, onKeyDown, handleHomeEnd]);
 
-    return <div onKeyDown={onKeyDown}>
+    return <RovingTabIndexContext.Provider value={context}>
-        { children }
+        { children({onKeyDownHandler}) }
-    </div>;
+    </RovingTabIndexContext.Provider>;
+};
+RovingTabIndexProvider.propTypes = {
+    handleHomeEnd: PropTypes.bool,
+    onKeyDown: PropTypes.func,
 };
 
 // Hook to register a roving tab index

src/components/structures/RoomSubList.js

@@ -142,10 +142,6 @@ export default class RoomSubList extends React.PureComponent {
 
     onHeaderKeyDown = (ev) => {
         switch (ev.key) {
-            case Key.TAB:
-                // Prevent LeftPanel handling Tab if focus is on the sublist header itself
-                ev.stopPropagation();
-                break;
             case Key.ARROW_LEFT:
                 // On ARROW_LEFT collapse the room sublist
                 if (!this.state.hidden && !this.props.forceExpand) {

src/components/views/groups/GroupInviteTile.js

@@ -128,7 +128,8 @@ export default createReactClass({
             'mx_RoomTile_badgeShown': this.state.badgeHover || isMenuDisplayed,
         });
 
-        const label = <div title={this.props.group.groupId} className={nameClasses} dir="auto">
+        // XXX: this is a workaround for Firefox giving this div a tabstop :( [tabIndex]
+        const label = <div title={this.props.group.groupId} className={nameClasses} tabIndex={-1} dir="auto">
             { groupName }
         </div>;
 

src/components/views/rooms/RoomList.js

@@ -777,21 +777,22 @@ export default createReactClass({
 
         const subListComponents = this._mapSubListProps(subLists);
 
-        const {resizeNotifier, collapsed, searchFilter, ConferenceHandler, ...props} = this.props; // eslint-disable-line
+        const {resizeNotifier, collapsed, searchFilter, ConferenceHandler, onKeyDown, ...props} = this.props; // eslint-disable-line
         return (
-            <div
+            <RovingTabIndexProvider handleHomeEnd={true} onKeyDown={onKeyDown}>
-                {...props}
+                {({onKeyDownHandler}) => <div
-                ref={this._collectResizeContainer}
+                    {...props}
-                className="mx_RoomList"
+                    onKeyDown={onKeyDownHandler}
-                role="tree"
+                    ref={this._collectResizeContainer}
-                aria-label={_t("Rooms")}
+                    className="mx_RoomList"
-                onMouseMove={this.onMouseMove}
+                    role="tree"
-                onMouseLeave={this.onMouseLeave}
+                    aria-label={_t("Rooms")}
-            >
+                    onMouseMove={this.onMouseMove}
-                <RovingTabIndexProvider handleHomeEnd={true}>
+                    onMouseLeave={this.onMouseLeave}
+                >
                     { subListComponents }
-                </RovingTabIndexProvider>
+                </div> }
-            </div>
+            </RovingTabIndexProvider>
         );
     },
 });

src/components/views/rooms/RoomTile.js

@@ -353,7 +353,8 @@ export default createReactClass({
             });
 
             subtextLabel = subtext ? <span className="mx_RoomTile_subtext">{ subtext }</span> : null;
-            label = <div title={name} className={nameClasses} dir="auto">{ name }</div>;
+            // XXX: this is a workaround for Firefox giving this div a tabstop :( [tabIndex]
+            label = <div title={name} className={nameClasses} tabIndex={-1} dir="auto">{ name }</div>;
         } else if (this.state.hover) {
             const Tooltip = sdk.getComponent("elements.Tooltip");
             tooltip = <Tooltip className="mx_RoomTile_tooltip" label={this.props.room.name} dir="auto" />;

test/accessibility/RovingTabIndex-test.js

@@ -47,7 +47,7 @@ const button4 = <Button key={4}>d</Button>;
 describe("RovingTabIndex", () => {
     it("RovingTabIndexProvider renders children as expected", () => {
         const wrapper = mount(<RovingTabIndexProvider>
-            <div><span>Test</span></div>
+            {() => <div><span>Test</span></div>}
         </RovingTabIndexProvider>);
         expect(wrapper.text()).toBe("Test");
         expect(wrapper.html()).toBe('<div><span>Test</span></div>');
@@ -55,9 +55,11 @@ describe("RovingTabIndex", () => {
 
     it("RovingTabIndexProvider works as expected with useRovingTabIndex", () => {
         const wrapper = mount(<RovingTabIndexProvider>
-            { button1 }
+            {() => <React.Fragment>
-            { button2 }
+                { button1 }
-            { button3 }
+                { button2 }
+                { button3 }
+            </React.Fragment>}
         </RovingTabIndexProvider>);
 
         // should begin with 0th being active
@@ -80,14 +82,14 @@ describe("RovingTabIndex", () => {
 
         // update the children, it should remain on the same button
         wrapper.setProps({
-            children: [button1, button4, button2, button3],
+            children: () => [button1, button4, button2, button3],
         });
         wrapper.update();
         checkTabIndexes(wrapper.find("button"), [-1, -1, 0, -1]);
 
         // update the children, remove the active button, it should move to the next one
         wrapper.setProps({
-            children: [button1, button4, button3],
+            children: () => [button1, button4, button3],
         });
         wrapper.update();
         checkTabIndexes(wrapper.find("button"), [-1, -1, 0]);
@@ -95,13 +97,15 @@ describe("RovingTabIndex", () => {
 
     it("RovingTabIndexProvider works as expected with RovingTabIndexWrapper", () => {
         const wrapper = mount(<RovingTabIndexProvider>
-            { button1 }
+            {() => <React.Fragment>
-            { button2 }
+                { button1 }
-            <RovingTabIndexWrapper>
+                { button2 }
-                {({onFocus, isActive, ref}) =>
+                <RovingTabIndexWrapper>
-                    <button onFocus={onFocus} tabIndex={isActive ? 0 : -1} ref={ref}>.</button>
+                    {({onFocus, isActive, ref}) =>
-                }
+                        <button onFocus={onFocus} tabIndex={isActive ? 0 : -1} ref={ref}>.</button>
-            </RovingTabIndexWrapper>
+                    }
+                </RovingTabIndexWrapper>
+            </React.Fragment>}
         </RovingTabIndexProvider>);
 
         // should begin with 0th being active