diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 0e76152f86..d08bc09031 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/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; diff --git a/src/accessibility/RovingTabIndex.js b/src/accessibility/RovingTabIndex.tsx similarity index 75% rename from src/accessibility/RovingTabIndex.js rename to src/accessibility/RovingTabIndex.tsx index b481f08fe2..388d67d9f3 100644 --- a/src/accessibility/RovingTabIndex.js +++ b/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; + +interface IState { + activeRef: Ref; + refs: Ref[]; +} + +interface IContext { + state: IState; + dispatch: Dispatch; +} + +const RovingTabIndexContext = createContext({ 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 -const types = { - REGISTER: "REGISTER", - UNREGISTER: "UNREGISTER", - SET_FOCUS: "SET_FOCUS", -}; +enum Type { + Register = "REGISTER", + Unregister = "UNREGISTER", + SetFocus = "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}) => { - const [state, dispatch] = useReducer(reducer, { +interface IProps { + handleHomeEnd?: boolean; + children(renderProps: { + onKeyDownHandler(ev: React.KeyboardEvent); + }); + onKeyDown?(ev: React.KeyboardEvent); +} + +export const RovingTabIndexProvider: React.FC = ({children, handleHomeEnd, onKeyDown}) => { + const [state, dispatch] = useReducer>(reducer, { activeRef: null, refs: [], }); - const context = useMemo(() => ({state, dispatch}), [state]); + const context = useMemo(() => ({state, dispatch}), [state]); const onKeyDownHandler = useCallback((ev) => { let handled = false; @@ -171,19 +201,17 @@ export const RovingTabIndexProvider = ({children, handleHomeEnd, onKeyDown}) => { children({onKeyDownHandler}) } ; }; -RovingTabIndexProvider.propTypes = { - handleHomeEnd: PropTypes.bool, - onKeyDown: PropTypes.func, -}; + +type FocusHandler = () => void; // 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(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 = ({children, inputRef}) => { const [onFocus, isActive, ref] = useRovingTabIndex(inputRef); return children({onFocus, isActive, ref}); }; +interface IRovingAccessibleButtonProps extends React.ComponentProps { + inputRef?: Ref; +} + +// Wrapper to allow use of useRovingTabIndex for simple AccessibleButtons outside of React Functional Components. +export const RovingAccessibleButton: React.FC = ({inputRef, ...props}) => { + const [onFocus, isActive, ref] = useRovingTabIndex(inputRef); + return ; +}; + diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 4b954d7843..7fac6cbff1 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -69,8 +69,7 @@ export default class LeftPanel2 extends React.Component { private listContainerRef: React.RefObject = createRef(); private tagPanelWatcherRef: string; private focusedElement = null; - - // TODO: a11y: https://github.com/vector-im/riot-web/issues/14180 + private isDoingStickyHeaders = false; constructor(props: IProps) { super(props); @@ -115,6 +114,23 @@ export default class LeftPanel2 extends React.Component { }; 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 { onVerticalArrow={this.onKeyDown} /> { }; 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(".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 { ); - // 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 { ); if (this.props.isMinimized) showMoreText = null; showNButton = ( - + {/* set by CSS masking */} {showMoreText} - + ); } 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 { ); if (this.props.isMinimized) showLessText = null; - // TODO Roving tab index / treeitem?: https://github.com/vector-im/riot-web/issues/14180 showNButton = ( - + {/* set by CSS masking */} {showLessText} - + ); } diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index ea5b888e2c..90edbfb895 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -419,7 +419,6 @@ export default class RoomTile2 extends React.Component { 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, diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 58d9ed4f31..0011ecfccd 100644 --- a/src/settings/Settings.js +++ b/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"), diff --git a/src/settings/handlers/RoomSettingsHandler.js b/src/settings/handlers/RoomSettingsHandler.js index d8e775742c..00dd5b8bec 100644 --- a/src/settings/handlers/RoomSettingsHandler.js +++ b/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 - // we fail softly here. We shouldn't assume that the state being fired is current - // state, but we also don't need to explode just because we didn't find a room. - if (!room) console.warn(`Unknown room caused setting update: ${roomId}`); - if (room && state !== room.currentState) return; // ignore state updates which are not current + // Note: in tests and during the encryption setup on initial load we might not have + // rooms in the store, so we just quietly ignore the problem. If we log it then we'll + // just end up spamming the logs a few thousand times. It is perfectly fine for us + // to ignore the problem as the app will not have loaded enough to care yet. + 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']; diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index e5205f6051..82c419d79d 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/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 { } // 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 = (payload); // TODO: Type out the dispatcher types @@ -321,6 +319,28 @@ export class RoomListStore2 extends AsyncStore { return 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 { return localStorage.getItem(`mx_listOrder_${tagId}`); } - private async updateAlgorithmInstances() { - const defaultSort = SortAlgorithm.Alphabetic; + // logic must match calculateTagSorting + 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 storedOrder = this.getStoredListOrder(tag); - - const tagSort = storedSort ? storedSort : (definedSort ? definedSort : defaultSort); - const listOrder = storedOrder ? storedOrder : (definedOrder ? definedOrder : defaultOrder); + const tagSort = this.calculateTagSorting(tag); + const listOrder = this.calculateListOrder(tag); if (tagSort !== definedSort) { await this.setTagSorting(tag, tagSort); @@ -378,8 +414,8 @@ export class RoomListStore2 extends AsyncStore { const sorts: ITagSortingMap = {}; const orders: IListOrderingMap = {}; for (const tagId of OrderedDefaultTagIDs) { - sorts[tagId] = this.getStoredTagSorting(tagId) || SortAlgorithm.Alphabetic; - orders[tagId] = this.getStoredListOrder(tagId) || ListAlgorithm.Natural; + sorts[tagId] = this.calculateTagSorting(tagId); + orders[tagId] = this.calculateListOrder(tagId); } if (this.state.tagsEnabled) { diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index d5f2ed0053..8c7bbc8615 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/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 - newTags[DefaultTagID.Untagged].push(room); - - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[DEBUG] "${room.name}" (${room.roomId}) is Untagged`); + if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { + newTags[DefaultTagID.DM].push(room); + } else { + newTags[DefaultTagID.Untagged].push(room); + } } }