Merge branches 'develop' and 't3chguy/room-list/12345' of github.com:matrix-org/matrix-react-sdk into t3chguy/room-list/12345

 Conflicts:

	src/components/views/rooms/RoomTile2.tsx

res/css/views/rooms/_RoomSublist2.scss

@@ -203,15 +203,16 @@ limitations under the License.
             // Update the render() function for RoomSublist2 if these change
             // Update the ListLayout class for minVisibleTiles if these change.
             //
-            // At 24px high and 8px padding on the top this equates to 0.65 of
+            // At 24px high, 8px padding on the top and 4px padding on the bottom  this equates to 0.73 of
             // a tile due to how the padding calculations work.
             height: 24px;
             padding-top: 8px;
+            padding-bottom: 4px;
 
             // We force this to the bottom so it will overlap rooms as needed.
             // We account for the space it takes up (24px) in the code through padding.
             position: absolute;
-            bottom: 4px; // the height of the resize handle
+            bottom: 0; // the height of the resize handle
             left: 0;
             right: 0;
 

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

@@ -69,8 +69,7 @@ export default class LeftPanel2 extends React.Component<IProps, IState> {
     private listContainerRef: React.RefObject<HTMLDivElement> = createRef();
     private tagPanelWatcherRef: string;
     private focusedElement = null;
-
+    private isDoingStickyHeaders = false;
-    // TODO: a11y: https://github.com/vector-im/riot-web/issues/14180
 
     constructor(props: IProps) {
         super(props);
@@ -115,6 +114,23 @@ export default class LeftPanel2 extends React.Component<IProps, IState> {
     };
 
     private handleStickyHeaders(list: HTMLDivElement) {
+        // TODO: Evaluate if this has any performance benefit or detriment.
+        // See https://github.com/vector-im/riot-web/issues/14035
+
+        if (this.isDoingStickyHeaders) return;
+        this.isDoingStickyHeaders = true;
+        if (window.requestAnimationFrame) {
+            window.requestAnimationFrame(() => {
+                this.doStickyHeaders(list);
+                this.isDoingStickyHeaders = false;
+            });
+        } else {
+            this.doStickyHeaders(list);
+            this.isDoingStickyHeaders = false;
+        }
+    }
+
+    private doStickyHeaders(list: HTMLDivElement) {
         const rlRect = list.getBoundingClientRect();
         const bottom = rlRect.bottom;
         const top = rlRect.top;
@@ -264,7 +280,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";
@@ -165,15 +165,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) => {
@@ -475,7 +485,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
@@ -546,12 +555,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
@@ -561,14 +570,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

@@ -419,7 +419,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,

src/settings/Settings.js

@@ -478,13 +478,13 @@ export const SETTINGS = {
             deny: [],
         },
     },
-    // TODO: Remove setting: https://github.com/vector-im/riot-web/issues/14231
+    // TODO: Remove setting: https://github.com/vector-im/riot-web/issues/14373
     "RoomList.orderAlphabetically": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
         displayName: _td("Order rooms by name"),
         default: false,
     },
-    // TODO: Remove setting: https://github.com/vector-im/riot-web/issues/14231
+    // TODO: Remove setting: https://github.com/vector-im/riot-web/issues/14373
     "RoomList.orderByImportance": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
         displayName: _td("Show rooms with unread notifications first"),

src/settings/handlers/RoomSettingsHandler.js

@@ -43,11 +43,14 @@ export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandl
         const roomId = event.getRoomId();
         const room = this.client.getRoom(roomId);
 
-        // Note: the tests often fire setting updates that don't have rooms in the store, so
+        // Note: in tests and during the encryption setup on initial load we might not have
-        // we fail softly here. We shouldn't assume that the state being fired is current
+        // rooms in the store, so we just quietly ignore the problem. If we log it then we'll
-        // state, but we also don't need to explode just because we didn't find a room.
+        // just end up spamming the logs a few thousand times. It is perfectly fine for us
-        if (!room) console.warn(`Unknown room caused setting update: ${roomId}`);
+        // to ignore the problem as the app will not have loaded enough to care yet.
-        if (room && state !== room.currentState) return; // ignore state updates which are not current
+        if (!room) return;
+
+        // ignore state updates which are not current
+        if (room && state !== room.currentState) return;
 
         if (event.getType() === "org.matrix.room.preview_urls") {
             let val = event.getContent()['disable'];

src/stores/room-list/RoomListStore2.ts

@@ -31,6 +31,7 @@ import RoomViewStore from "../RoomViewStore";
 import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/Algorithm";
 import { EffectiveMembership, getEffectiveMembership } from "./membership";
 import { ListLayout } from "./ListLayout";
+import { isNullOrUndefined } from "matrix-js-sdk/src/utils";
 
 interface IState {
     tagsEnabled?: boolean;
@@ -221,9 +222,6 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
             }
             // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
             console.log(`[RoomListDebug] Decrypted timeline event ${eventPayload.event.getId()} in ${roomId}`);
-            // TODO: Verify that e2e rooms are handled on init: https://github.com/vector-im/riot-web/issues/14238
-            // It seems like when viewing the room the timeline is decrypted, rather than at startup. This could
-            // cause inaccuracies with the list ordering. We may have to decrypt the last N messages of every room :(
             await this.handleRoomUpdate(room, RoomUpdateCause.Timeline);
         } else if (payload.action === 'MatrixActions.accountData' && payload.event_type === 'm.direct') {
             const eventPayload = (<any>payload); // TODO: Type out the dispatcher types
@@ -321,6 +319,28 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
         return <SortAlgorithm>localStorage.getItem(`mx_tagSort_${tagId}`);
     }
 
+    // logic must match calculateListOrder
+    private calculateTagSorting(tagId: TagID): SortAlgorithm {
+        const defaultSort = SortAlgorithm.Alphabetic;
+        const settingAlphabetical = SettingsStore.getValue("RoomList.orderAlphabetically", null, true);
+        const definedSort = this.getTagSorting(tagId);
+        const storedSort = this.getStoredTagSorting(tagId);
+
+        // We use the following order to determine which of the 4 flags to use:
+        // Stored > Settings > Defined > Default
+
+        let tagSort = defaultSort;
+        if (storedSort) {
+            tagSort = storedSort;
+        } else if (!isNullOrUndefined(settingAlphabetical)) {
+            tagSort = settingAlphabetical ? SortAlgorithm.Alphabetic : SortAlgorithm.Recent;
+        } else if (definedSort) {
+            tagSort = definedSort;
+        } // else default (already set)
+
+        return tagSort;
+    }
+
     public async setListOrder(tagId: TagID, order: ListAlgorithm) {
         await this.algorithm.setListOrdering(tagId, order);
         // TODO: Per-account? https://github.com/vector-im/riot-web/issues/14114
@@ -337,19 +357,35 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
         return <ListAlgorithm>localStorage.getItem(`mx_listOrder_${tagId}`);
     }
 
-    private async updateAlgorithmInstances() {
+    // logic must match calculateTagSorting
-        const defaultSort = SortAlgorithm.Alphabetic;
+    private calculateListOrder(tagId: TagID): ListAlgorithm {
         const defaultOrder = ListAlgorithm.Natural;
+        const settingImportance = SettingsStore.getValue("RoomList.orderByImportance", null, true);
+        const definedOrder = this.getListOrder(tagId);
+        const storedOrder = this.getStoredListOrder(tagId);
 
+        // We use the following order to determine which of the 4 flags to use:
+        // Stored > Settings > Defined > Default
+
+        let listOrder = defaultOrder;
+        if (storedOrder) {
+            listOrder = storedOrder;
+        } else if (!isNullOrUndefined(settingImportance)) {
+            listOrder = settingImportance ? ListAlgorithm.Importance : ListAlgorithm.Natural;
+        } else if (definedOrder) {
+            listOrder = definedOrder;
+        } // else default (already set)
+
+        return listOrder;
+    }
+
+    private async updateAlgorithmInstances() {
         for (const tag of Object.keys(this.orderedLists)) {
             const definedSort = this.getTagSorting(tag);
             const definedOrder = this.getListOrder(tag);
 
-            const storedSort = this.getStoredTagSorting(tag);
+            const tagSort = this.calculateTagSorting(tag);
-            const storedOrder = this.getStoredListOrder(tag);
+            const listOrder = this.calculateListOrder(tag);
-
-            const tagSort = storedSort ? storedSort : (definedSort ? definedSort : defaultSort);
-            const listOrder = storedOrder ? storedOrder : (definedOrder ? definedOrder : defaultOrder);
 
             if (tagSort !== definedSort) {
                 await this.setTagSorting(tag, tagSort);
@@ -378,8 +414,8 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
         const sorts: ITagSortingMap = {};
         const orders: IListOrderingMap = {};
         for (const tagId of OrderedDefaultTagIDs) {
-            sorts[tagId] = this.getStoredTagSorting(tagId) || SortAlgorithm.Alphabetic;
+            sorts[tagId] = this.calculateTagSorting(tagId);
-            orders[tagId] = this.getStoredListOrder(tagId) || ListAlgorithm.Natural;
+            orders[tagId] = this.calculateListOrder(tagId);
         }
 
         if (this.state.tagsEnabled) {

src/stores/room-list/algorithms/Algorithm.ts

@@ -109,6 +109,7 @@ export class Algorithm extends EventEmitter {
     }
 
     public getTagSorting(tagId: TagID): SortAlgorithm {
+        if (!this.sortAlgorithms) return null;
         return this.sortAlgorithms[tagId];
     }
 
@@ -125,6 +126,7 @@ export class Algorithm extends EventEmitter {
     }
 
     public getListOrdering(tagId: TagID): ListAlgorithm {
+        if (!this.listAlgorithms) return null;
         return this.listAlgorithms[tagId];
     }
 
@@ -501,13 +503,9 @@ export class Algorithm extends EventEmitter {
         // Split out the easy rooms first (leave and invite)
         const memberships = splitRoomsByMembership(rooms);
         for (const room of memberships[EffectiveMembership.Invite]) {
-            // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
-            console.log(`[DEBUG] "${room.name}" (${room.roomId}) is an Invite`);
             newTags[DefaultTagID.Invite].push(room);
         }
         for (const room of memberships[EffectiveMembership.Leave]) {
-            // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
-            console.log(`[DEBUG] "${room.name}" (${room.roomId}) is Historical`);
             newTags[DefaultTagID.Archived].push(room);
         }
 
@@ -518,11 +516,7 @@ export class Algorithm extends EventEmitter {
             let inTag = false;
             if (tags.length > 0) {
                 for (const tag of tags) {
-                    // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
-                    console.log(`[DEBUG] "${room.name}" (${room.roomId}) is tagged as ${tag}`);
                     if (!isNullOrUndefined(newTags[tag])) {
-                        // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
-                        console.log(`[DEBUG] "${room.name}" (${room.roomId}) is tagged with VALID tag ${tag}`);
                         newTags[tag].push(room);
                         inTag = true;
                     }
@@ -530,11 +524,11 @@ export class Algorithm extends EventEmitter {
             }
 
             if (!inTag) {
-                // TODO: Determine if DM and push there instead: https://github.com/vector-im/riot-web/issues/14236
+                if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) {
-                newTags[DefaultTagID.Untagged].push(room);
+                    newTags[DefaultTagID.DM].push(room);
-
+                } else {
-                // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
+                    newTags[DefaultTagID.Untagged].push(room);
-                console.log(`[DEBUG] "${room.name}" (${room.roomId}) is Untagged`);
+                }
             }
         }