From 58718dab37d145566c70b8f2731acd35ed798cd6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 1 Jul 2020 23:51:12 +0100 Subject: [PATCH 01/72] Convert ContextMenu to TypeScript Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/@types/common.ts | 1 + .../{ContextMenu.js => ContextMenu.tsx} | 259 ++++++++++-------- .../views/elements/AccessibleButton.tsx | 2 +- 3 files changed, 148 insertions(+), 114 deletions(-) rename src/components/structures/{ContextMenu.js => ContextMenu.tsx} (71%) diff --git a/src/@types/common.ts b/src/@types/common.ts index 9109993541..a24d47ac9e 100644 --- a/src/@types/common.ts +++ b/src/@types/common.ts @@ -17,3 +17,4 @@ limitations under the License. // Based on https://stackoverflow.com/a/53229857/3532235 export type Without<T, U> = {[P in Exclude<keyof T, keyof U>] ? : never}; export type XOR<T, U> = (T | U) extends object ? (Without<T, U> & U) | (Without<U, T> & T) : T | U; +export type Writeable<T> = { -readonly [P in keyof T]: T[P] }; diff --git a/src/components/structures/ContextMenu.js b/src/components/structures/ContextMenu.tsx similarity index 71% rename from src/components/structures/ContextMenu.js rename to src/components/structures/ContextMenu.tsx index 5ba2662796..f07c12f0b3 100644 --- a/src/components/structures/ContextMenu.js +++ b/src/components/structures/ContextMenu.tsx @@ -1,28 +1,28 @@ /* -Copyright 2015, 2016 OpenMarket Ltd -Copyright 2018 New Vector Ltd -Copyright 2019 The Matrix.org Foundation C.I.C. + * + * Copyright 2019 Michael Telatynski <7t3chguy@gmail.com> + * + * 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. + * / + */ -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 +import React, {CSSProperties, useRef, useState} from "react"; +import ReactDOM from "react-dom"; +import classNames from "classnames"; - 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 React, {useRef, useState} from 'react'; -import ReactDOM from 'react-dom'; -import PropTypes from 'prop-types'; -import classNames from 'classnames'; import {Key} from "../../Keyboard"; -import * as sdk from "../../index"; -import AccessibleButton from "../views/elements/AccessibleButton"; +import AccessibleButton, { IAccessibleButtonProps } from "../views/elements/AccessibleButton"; +import {Writeable} from "../../@types/common"; // Shamelessly ripped off Modal.js. There's probably a better way // of doing reusable widgets like dialog boxes & menus where we go and @@ -30,8 +30,8 @@ import AccessibleButton from "../views/elements/AccessibleButton"; const ContextualMenuContainerId = "mx_ContextualMenu_Container"; -function getOrCreateContainer() { - let container = document.getElementById(ContextualMenuContainerId); +function getOrCreateContainer(): HTMLDivElement { + let container = document.getElementById(ContextualMenuContainerId) as HTMLDivElement; if (!container) { container = document.createElement("div"); @@ -43,50 +43,70 @@ function getOrCreateContainer() { } const ARIA_MENU_ITEM_ROLES = new Set(["menuitem", "menuitemcheckbox", "menuitemradio"]); + +interface IPosition { + top?: number; + bottom?: number; + left?: number; + right?: number; +} + +export enum ChevronFace { + Top = "top", + Bottom = "bottom", + Left = "left", + Right = "right", + None = "none", +} + +interface IProps extends IPosition { + menuWidth?: number; + menuHeight?: number; + + chevronOffset?: number; + chevronFace?: ChevronFace; + + menuPaddingTop?: number; + menuPaddingBottom?: number; + menuPaddingLeft?: number; + menuPaddingRight?: number; + + zIndex?: number; + + // If true, insert an invisible screen-sized element behind the menu that when clicked will close it. + hasBackground?: boolean; + // whether this context menu should be focus managed. If false it must handle itself + managed?: boolean; + + // Function to be called on menu close + onFinished(); + // on resize callback + windowResize(); +} + +interface IState { + contextMenuElem: HTMLDivElement; +} + // Generic ContextMenu Portal wrapper // all options inside the menu should be of role=menuitem/menuitemcheckbox/menuitemradiobutton and have tabIndex={-1} // this will allow the ContextMenu to manage its own focus using arrow keys as per the ARIA guidelines. -export class ContextMenu extends React.Component { - static propTypes = { - top: PropTypes.number, - bottom: PropTypes.number, - left: PropTypes.number, - right: PropTypes.number, - menuWidth: PropTypes.number, - menuHeight: PropTypes.number, - chevronOffset: PropTypes.number, - chevronFace: PropTypes.string, // top, bottom, left, right or none - // Function to be called on menu close - onFinished: PropTypes.func.isRequired, - menuPaddingTop: PropTypes.number, - menuPaddingRight: PropTypes.number, - menuPaddingBottom: PropTypes.number, - menuPaddingLeft: PropTypes.number, - zIndex: PropTypes.number, - - // If true, insert an invisible screen-sized element behind the - // menu that when clicked will close it. - hasBackground: PropTypes.bool, - - // on resize callback - windowResize: PropTypes.func, - - managed: PropTypes.bool, // whether this context menu should be focus managed. If false it must handle itself - }; +export class ContextMenu extends React.PureComponent<IProps, IState> { + private initialFocus: HTMLElement; static defaultProps = { hasBackground: true, managed: true, }; - constructor() { - super(); + constructor(props, context) { + super(props, context); this.state = { contextMenuElem: null, }; // persist what had focus when we got initialized so we can return it after - this.initialFocus = document.activeElement; + this.initialFocus = document.activeElement as HTMLElement; } componentWillUnmount() { @@ -232,9 +252,8 @@ export class ContextMenu extends React.Component { } }; - renderMenu(hasBackground=this.props.hasBackground) { - const position = {}; - let chevronFace = null; + renderMenu(hasBackground = this.props.hasBackground) { + const position: Partial<Writeable<DOMRect>> = {}; const props = this.props; if (props.top) { @@ -243,23 +262,24 @@ export class ContextMenu extends React.Component { position.bottom = props.bottom; } + let chevronFace: IProps["chevronFace"]; if (props.left) { position.left = props.left; - chevronFace = 'left'; + chevronFace = ChevronFace.Left; } else { position.right = props.right; - chevronFace = 'right'; + chevronFace = ChevronFace.Right; } const contextMenuRect = this.state.contextMenuElem ? this.state.contextMenuElem.getBoundingClientRect() : null; - const chevronOffset = {}; + const chevronOffset: CSSProperties = {}; if (props.chevronFace) { chevronFace = props.chevronFace; } - const hasChevron = chevronFace && chevronFace !== "none"; + const hasChevron = chevronFace && chevronFace !== ChevronFace.None; - if (chevronFace === 'top' || chevronFace === 'bottom') { + if (chevronFace === ChevronFace.Top || chevronFace === ChevronFace.Bottom) { chevronOffset.left = props.chevronOffset; } else if (position.top !== undefined) { const target = position.top; @@ -289,13 +309,13 @@ export class ContextMenu extends React.Component { 'mx_ContextualMenu_right': !hasChevron && position.right, 'mx_ContextualMenu_top': !hasChevron && position.top, 'mx_ContextualMenu_bottom': !hasChevron && position.bottom, - 'mx_ContextualMenu_withChevron_left': chevronFace === 'left', - 'mx_ContextualMenu_withChevron_right': chevronFace === 'right', - 'mx_ContextualMenu_withChevron_top': chevronFace === 'top', - 'mx_ContextualMenu_withChevron_bottom': chevronFace === 'bottom', + 'mx_ContextualMenu_withChevron_left': chevronFace === ChevronFace.Left, + 'mx_ContextualMenu_withChevron_right': chevronFace === ChevronFace.Right, + 'mx_ContextualMenu_withChevron_top': chevronFace === ChevronFace.Top, + 'mx_ContextualMenu_withChevron_bottom': chevronFace === ChevronFace.Bottom, }); - const menuStyle = {}; + const menuStyle: CSSProperties = {}; if (props.menuWidth) { menuStyle.width = props.menuWidth; } @@ -326,13 +346,28 @@ export class ContextMenu extends React.Component { let background; if (hasBackground) { background = ( - <div className="mx_ContextualMenu_background" style={wrapperStyle} onClick={props.onFinished} onContextMenu={this.onContextMenu} /> + <div + className="mx_ContextualMenu_background" + style={wrapperStyle} + onClick={props.onFinished} + onContextMenu={this.onContextMenu} + /> ); } return ( - <div className="mx_ContextualMenu_wrapper" style={{...position, ...wrapperStyle}} onKeyDown={this._onKeyDown} onContextMenu={this.onContextMenuPreventBubbling}> - <div className={menuClasses} style={menuStyle} ref={this.collectContextMenuRect} role={this.props.managed ? "menu" : undefined}> + <div + className="mx_ContextualMenu_wrapper" + style={{...position, ...wrapperStyle}} + onKeyDown={this._onKeyDown} + onContextMenu={this.onContextMenuPreventBubbling} + > + <div + className={menuClasses} + style={menuStyle} + ref={this.collectContextMenuRect} + role={this.props.managed ? "menu" : undefined} + > { chevron } { props.children } </div> @@ -341,14 +376,19 @@ export class ContextMenu extends React.Component { ); } - render() { + render(): React.ReactChild { return ReactDOM.createPortal(this.renderMenu(), getOrCreateContainer()); } } +interface IContextMenuButtonProps extends IAccessibleButtonProps { + label?: string; + // whether or not the context menu is currently open + isExpanded: boolean; +} + // Semantic component for representing the AccessibleButton which launches a <ContextMenu /> -export const ContextMenuButton = ({ label, isExpanded, children, onClick, onContextMenu, ...props }) => { - const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); +export const ContextMenuButton: React.FC<IContextMenuButtonProps> = ({ label, isExpanded, children, onClick, onContextMenu, ...props }) => { return ( <AccessibleButton {...props} @@ -363,77 +403,70 @@ export const ContextMenuButton = ({ label, isExpanded, children, onClick, onCont </AccessibleButton> ); }; -ContextMenuButton.propTypes = { - ...AccessibleButton.propTypes, - label: PropTypes.string, - isExpanded: PropTypes.bool.isRequired, // whether or not the context menu is currently open -}; + +interface IMenuItemProps extends IAccessibleButtonProps { + label?: string; + className?: string; + onClick(); +} // Semantic component for representing a role=menuitem -export const MenuItem = ({children, label, ...props}) => { - const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); +export const MenuItem: React.FC<IMenuItemProps> = ({children, label, ...props}) => { return ( <AccessibleButton {...props} role="menuitem" tabIndex={-1} aria-label={label}> { children } </AccessibleButton> ); }; -MenuItem.propTypes = { - ...AccessibleButton.propTypes, - label: PropTypes.string, // optional - className: PropTypes.string, // optional - onClick: PropTypes.func.isRequired, -}; + +interface IMenuGroupProps extends React.HTMLAttributes<HTMLDivElement> { + label: string; + className?: string; +} // Semantic component for representing a role=group for grouping menu radios/checkboxes -export const MenuGroup = ({children, label, ...props}) => { +export const MenuGroup: React.FC<IMenuGroupProps> = ({children, label, ...props}) => { return <div {...props} role="group" aria-label={label}> { children } </div>; }; -MenuGroup.propTypes = { - label: PropTypes.string.isRequired, - className: PropTypes.string, // optional -}; + +interface IMenuItemCheckboxProps extends IAccessibleButtonProps { + label?: string; + active: boolean; + disabled?: boolean; + className?: string; + onClick(); +} // Semantic component for representing a role=menuitemcheckbox -export const MenuItemCheckbox = ({children, label, active=false, disabled=false, ...props}) => { - const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); +export const MenuItemCheckbox: React.FC<IMenuItemCheckboxProps> = ({children, label, active = false, disabled = false, ...props}) => { return ( <AccessibleButton {...props} role="menuitemcheckbox" aria-checked={active} aria-disabled={disabled} tabIndex={-1} aria-label={label}> { children } </AccessibleButton> ); }; -MenuItemCheckbox.propTypes = { - ...AccessibleButton.propTypes, - label: PropTypes.string, // optional - active: PropTypes.bool.isRequired, - disabled: PropTypes.bool, // optional - className: PropTypes.string, // optional - onClick: PropTypes.func.isRequired, -}; + +interface IMenuItemRadioProps extends IAccessibleButtonProps { + label?: string; + active: boolean; + disabled?: boolean; + className?: string; + onClick(); +} // Semantic component for representing a role=menuitemradio -export const MenuItemRadio = ({children, label, active=false, disabled=false, ...props}) => { - const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); +export const MenuItemRadio: React.FC<IMenuItemRadioProps> = ({children, label, active = false, disabled = false, ...props}) => { return ( <AccessibleButton {...props} role="menuitemradio" aria-checked={active} aria-disabled={disabled} tabIndex={-1} aria-label={label}> { children } </AccessibleButton> ); }; -MenuItemRadio.propTypes = { - ...AccessibleButton.propTypes, - label: PropTypes.string, // optional - active: PropTypes.bool.isRequired, - disabled: PropTypes.bool, // optional - className: PropTypes.string, // optional - onClick: PropTypes.func.isRequired, -}; // Placement method for <ContextMenu /> to position context menu to right of elementRect with chevronOffset -export const toRightOf = (elementRect, chevronOffset=12) => { +export const toRightOf = (elementRect: DOMRect, chevronOffset = 12) => { const left = elementRect.right + window.pageXOffset + 3; let top = elementRect.top + (elementRect.height / 2) + window.pageYOffset; top -= chevronOffset + 8; // where 8 is half the height of the chevron @@ -441,8 +474,8 @@ export const toRightOf = (elementRect, chevronOffset=12) => { }; // Placement method for <ContextMenu /> to position context menu right-aligned and flowing to the left of elementRect -export const aboveLeftOf = (elementRect, chevronFace="none") => { - const menuOptions = { chevronFace }; +export const aboveLeftOf = (elementRect: DOMRect, chevronFace = ChevronFace.None) => { + const menuOptions: IPosition & { chevronFace: ChevronFace } = { chevronFace }; const buttonRight = elementRect.right + window.pageXOffset; const buttonBottom = elementRect.bottom + window.pageYOffset; diff --git a/src/components/views/elements/AccessibleButton.tsx b/src/components/views/elements/AccessibleButton.tsx index 01a27d9522..489f11699a 100644 --- a/src/components/views/elements/AccessibleButton.tsx +++ b/src/components/views/elements/AccessibleButton.tsx @@ -42,7 +42,7 @@ interface IProps extends React.InputHTMLAttributes<Element> { onClick?(e?: ButtonEvent): void; } -interface IAccessibleButtonProps extends React.InputHTMLAttributes<Element> { +export interface IAccessibleButtonProps extends React.InputHTMLAttributes<Element> { ref?: React.Ref<Element>; } From 6802f9b4dfc237a61e465e5b445fe28604331a45 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 1 Jul 2020 23:52:49 +0100 Subject: [PATCH 02/72] unbreak copyright Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.tsx | 32 +++++++++++------------ 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index f07c12f0b3..3e38a18d98 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -1,20 +1,20 @@ /* - * - * Copyright 2019 Michael Telatynski <7t3chguy@gmail.com> - * - * 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. - * / - */ +Copyright 2015, 2016 OpenMarket Ltd +Copyright 2018 New Vector Ltd +Copyright 2019 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 React, {CSSProperties, useRef, useState} from "react"; import ReactDOM from "react-dom"; From 07e0a017e7885692e308b02a63c947d147c68cc2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 1 Jul 2020 23:56:57 +0100 Subject: [PATCH 03/72] fix types Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.tsx | 10 ++++----- src/components/structures/UserMenu.tsx | 22 ++++++++++---------- src/components/views/rooms/RoomSublist2.tsx | 23 ++++++++++----------- src/components/views/rooms/RoomTile2.tsx | 8 +++---- 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 3e38a18d98..be41ec1569 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -21,7 +21,7 @@ import ReactDOM from "react-dom"; import classNames from "classnames"; import {Key} from "../../Keyboard"; -import AccessibleButton, { IAccessibleButtonProps } from "../views/elements/AccessibleButton"; +import AccessibleButton, { IAccessibleButtonProps, ButtonEvent } from "../views/elements/AccessibleButton"; import {Writeable} from "../../@types/common"; // Shamelessly ripped off Modal.js. There's probably a better way @@ -81,7 +81,7 @@ interface IProps extends IPosition { // Function to be called on menu close onFinished(); // on resize callback - windowResize(); + windowResize?(); } interface IState { @@ -407,7 +407,7 @@ export const ContextMenuButton: React.FC<IContextMenuButtonProps> = ({ label, is interface IMenuItemProps extends IAccessibleButtonProps { label?: string; className?: string; - onClick(); + onClick(ev: ButtonEvent); } // Semantic component for representing a role=menuitem @@ -436,7 +436,7 @@ interface IMenuItemCheckboxProps extends IAccessibleButtonProps { active: boolean; disabled?: boolean; className?: string; - onClick(); + onClick(ev: ButtonEvent); } // Semantic component for representing a role=menuitemcheckbox @@ -453,7 +453,7 @@ interface IMenuItemRadioProps extends IAccessibleButtonProps { active: boolean; disabled?: boolean; className?: string; - onClick(); + onClick(ev: ButtonEvent); } // Semantic component for representing a role=menuitemradio diff --git a/src/components/structures/UserMenu.tsx b/src/components/structures/UserMenu.tsx index 1cfe244845..aba0a3f053 100644 --- a/src/components/structures/UserMenu.tsx +++ b/src/components/structures/UserMenu.tsx @@ -15,15 +15,15 @@ limitations under the License. */ import * as React from "react"; -import { MatrixClientPeg } from "../../MatrixClientPeg"; +import {createRef} from "react"; +import {MatrixClientPeg} from "../../MatrixClientPeg"; import defaultDispatcher from "../../dispatcher/dispatcher"; -import { ActionPayload } from "../../dispatcher/payloads"; -import { Action } from "../../dispatcher/actions"; -import { createRef } from "react"; -import { _t } from "../../languageHandler"; -import {ContextMenu, ContextMenuButton} from "./ContextMenu"; +import {ActionPayload} from "../../dispatcher/payloads"; +import {Action} from "../../dispatcher/actions"; +import {_t} from "../../languageHandler"; +import {ChevronFace, ContextMenu, ContextMenuButton} from "./ContextMenu"; import {USER_NOTIFICATIONS_TAB, USER_SECURITY_TAB} from "../views/dialogs/UserSettingsDialog"; -import { OpenToTabPayload } from "../../dispatcher/payloads/OpenToTabPayload"; +import {OpenToTabPayload} from "../../dispatcher/payloads/OpenToTabPayload"; import RedesignFeedbackDialog from "../views/dialogs/RedesignFeedbackDialog"; import Modal from "../../Modal"; import LogoutDialog from "../views/dialogs/LogoutDialog"; @@ -33,8 +33,8 @@ import {getHostingLink} from "../../utils/HostingLink"; import AccessibleButton, {ButtonEvent} from "../views/elements/AccessibleButton"; import SdkConfig from "../../SdkConfig"; import {getHomePageUrl} from "../../utils/pages"; -import { OwnProfileStore } from "../../stores/OwnProfileStore"; -import { UPDATE_EVENT } from "../../stores/AsyncStore"; +import {OwnProfileStore} from "../../stores/OwnProfileStore"; +import {UPDATE_EVENT} from "../../stores/AsyncStore"; import BaseAvatar from '../views/avatars/BaseAvatar'; import classNames from "classnames"; @@ -105,7 +105,7 @@ export default class UserMenu extends React.Component<IProps, IState> { if (this.buttonRef.current) this.buttonRef.current.click(); }; - private onOpenMenuClick = (ev: InputEvent) => { + private onOpenMenuClick = (ev: React.MouseEvent) => { ev.preventDefault(); ev.stopPropagation(); const target = ev.target as HTMLButtonElement; @@ -214,7 +214,7 @@ export default class UserMenu extends React.Component<IProps, IState> { return ( <ContextMenu - chevronFace="none" + chevronFace={ChevronFace.None} // -20 to overlap the context menu by just over the width of the `...` icon and make it look connected left={this.state.contextMenuPosition.width + this.state.contextMenuPosition.left - 20} top={this.state.contextMenuPosition.top + this.state.contextMenuPosition.height} diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 36d38c7087..fb3ebf4728 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -17,22 +17,21 @@ limitations under the License. */ import * as React from "react"; -import { createRef } from "react"; -import { Room } from "matrix-js-sdk/src/models/room"; +import {Room} from "matrix-js-sdk/src/models/room"; import classNames from 'classnames'; -import { RovingTabIndexWrapper } from "../../../accessibility/RovingTabIndex"; -import { _t } from "../../../languageHandler"; +import {RovingTabIndexWrapper} from "../../../accessibility/RovingTabIndex"; +import {_t} from "../../../languageHandler"; import AccessibleButton from "../../views/elements/AccessibleButton"; import RoomTile2 from "./RoomTile2"; -import { ResizableBox, ResizeCallbackData } from "react-resizable"; -import { ListLayout } from "../../../stores/room-list/ListLayout"; -import NotificationBadge, { ListNotificationState } from "./NotificationBadge"; -import { ContextMenu, ContextMenuButton } from "../../structures/ContextMenu"; +import {ResizableBox, ResizeCallbackData} from "react-resizable"; +import {ListLayout} from "../../../stores/room-list/ListLayout"; +import NotificationBadge, {ListNotificationState} from "./NotificationBadge"; +import {ChevronFace, ContextMenu, ContextMenuButton} from "../../structures/ContextMenu"; import StyledCheckbox from "../elements/StyledCheckbox"; import StyledRadioButton from "../elements/StyledRadioButton"; import RoomListStore from "../../../stores/room-list/RoomListStore2"; -import { ListAlgorithm, SortAlgorithm } from "../../../stores/room-list/algorithms/models"; -import { DefaultTagID, TagID } from "../../../stores/room-list/models"; +import {ListAlgorithm, SortAlgorithm} from "../../../stores/room-list/algorithms/models"; +import {DefaultTagID, TagID} from "../../../stores/room-list/models"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -128,7 +127,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { this.forceUpdate(); // because the layout doesn't trigger a re-render }; - private onOpenMenuClick = (ev: InputEvent) => { + private onOpenMenuClick = (ev: React.MouseEvent) => { ev.preventDefault(); ev.stopPropagation(); const target = ev.target as HTMLButtonElement; @@ -219,7 +218,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance; contextMenu = ( <ContextMenu - chevronFace="none" + chevronFace={ChevronFace.None} left={this.state.contextMenuPosition.left} top={this.state.contextMenuPosition.top + this.state.contextMenuPosition.height} onFinished={this.onCloseMenu} diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 7a07027913..5f535392c0 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -32,7 +32,7 @@ import NotificationBadge, { TagSpecificNotificationState } from "./NotificationBadge"; import { _t } from "../../../languageHandler"; -import { ContextMenu, ContextMenuButton, MenuItemRadio } from "../../structures/ContextMenu"; +import {ChevronFace, ContextMenu, ContextMenuButton, MenuItemRadio} from "../../structures/ContextMenu"; import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import { MessagePreviewStore } from "../../../stores/room-list/MessagePreviewStore"; import RoomTileIcon from "./RoomTileIcon"; @@ -74,7 +74,7 @@ const contextMenuBelow = (elementRect: PartialDOMRect) => { // align the context menu's icons with the icon which opened the context menu const left = elementRect.left + window.pageXOffset - 9; const top = elementRect.bottom + window.pageYOffset + 17; - const chevronFace = "none"; + const chevronFace = ChevronFace.None; return {left, top, chevronFace}; }; @@ -151,7 +151,7 @@ export default class RoomTile2 extends React.Component<IProps, IState> { this.setState({selected: isActive}); }; - private onNotificationsMenuOpenClick = (ev: InputEvent) => { + private onNotificationsMenuOpenClick = (ev: React.MouseEvent) => { ev.preventDefault(); ev.stopPropagation(); const target = ev.target as HTMLButtonElement; @@ -162,7 +162,7 @@ export default class RoomTile2 extends React.Component<IProps, IState> { this.setState({notificationsMenuPosition: null}); }; - private onGeneralMenuOpenClick = (ev: InputEvent) => { + private onGeneralMenuOpenClick = (ev: React.MouseEvent) => { ev.preventDefault(); ev.stopPropagation(); const target = ev.target as HTMLButtonElement; From 9fcc2ced3d3ed7af84169d2a80c61733033aac22 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 1 Jul 2020 23:59:06 +0100 Subject: [PATCH 04/72] fix types some more Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.tsx | 2 +- src/components/views/elements/AccessibleButton.tsx | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index be41ec1569..cec924c022 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -21,7 +21,7 @@ import ReactDOM from "react-dom"; import classNames from "classnames"; import {Key} from "../../Keyboard"; -import AccessibleButton, { IAccessibleButtonProps, ButtonEvent } from "../views/elements/AccessibleButton"; +import AccessibleButton, { IProps as IAccessibleButtonProps, ButtonEvent } from "../views/elements/AccessibleButton"; import {Writeable} from "../../@types/common"; // Shamelessly ripped off Modal.js. There's probably a better way diff --git a/src/components/views/elements/AccessibleButton.tsx b/src/components/views/elements/AccessibleButton.tsx index 489f11699a..34481601f7 100644 --- a/src/components/views/elements/AccessibleButton.tsx +++ b/src/components/views/elements/AccessibleButton.tsx @@ -27,7 +27,7 @@ export type ButtonEvent = React.MouseEvent<Element> | React.KeyboardEvent<Elemen * onClick: (required) Event handler for button activation. Should be * implemented exactly like a normal onClick handler. */ -interface IProps extends React.InputHTMLAttributes<Element> { +export interface IProps extends React.InputHTMLAttributes<Element> { inputRef?: React.Ref<Element>; element?: string; // The kind of button, similar to how Bootstrap works. @@ -42,7 +42,7 @@ interface IProps extends React.InputHTMLAttributes<Element> { onClick?(e?: ButtonEvent): void; } -export interface IAccessibleButtonProps extends React.InputHTMLAttributes<Element> { +interface IAccessibleButtonProps extends React.InputHTMLAttributes<Element> { ref?: React.Ref<Element>; } @@ -64,7 +64,6 @@ export default function AccessibleButton({ className, ...restProps }: IProps) { - const newProps: IAccessibleButtonProps = restProps; if (!disabled) { newProps.onClick = onClick; From 0854924b8dd3b84850649cf5c257293e4330ef60 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jul 2020 23:51:02 +0100 Subject: [PATCH 05/72] iterate some more Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.tsx | 34 +++++++++++------------ 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index c76bbe6133..e45d846bd0 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -114,7 +114,7 @@ export class ContextMenu extends React.PureComponent<IProps, IState> { this.initialFocus.focus(); } - collectContextMenuRect = (element) => { + private collectContextMenuRect = (element) => { // We don't need to clean up when unmounting, so ignore if (!element) return; @@ -131,7 +131,7 @@ export class ContextMenu extends React.PureComponent<IProps, IState> { }); }; - onContextMenu = (e) => { + private onContextMenu = (e) => { if (this.props.onFinished) { this.props.onFinished(); @@ -154,20 +154,20 @@ export class ContextMenu extends React.PureComponent<IProps, IState> { } }; - onContextMenuPreventBubbling = (e) => { + private onContextMenuPreventBubbling = (e) => { // stop propagation so that any context menu handlers don't leak out of this context menu // but do not inhibit the default browser menu e.stopPropagation(); }; // Prevent clicks on the background from going through to the component which opened the menu. - _onFinished = (ev: InputEvent) => { + private onFinished = (ev: React.MouseEvent) => { ev.stopPropagation(); ev.preventDefault(); if (this.props.onFinished) this.props.onFinished(); }; - _onMoveFocus = (element, up) => { + private onMoveFocus = (element: Element, up: boolean) => { let descending = false; // are we currently descending or ascending through the DOM tree? do { @@ -201,25 +201,25 @@ export class ContextMenu extends React.PureComponent<IProps, IState> { } while (element && !ARIA_MENU_ITEM_ROLES.has(element.getAttribute("role"))); if (element) { - element.focus(); + (element as HTMLElement).focus(); } }; - _onMoveFocusHomeEnd = (element, up) => { + private onMoveFocusHomeEnd = (element: Element, up: boolean) => { let results = element.querySelectorAll('[role^="menuitem"]'); if (!results) { results = element.querySelectorAll('[tab-index]'); } if (results && results.length) { if (up) { - results[0].focus(); + (results[0] as HTMLElement).focus(); } else { - results[results.length - 1].focus(); + (results[results.length - 1] as HTMLElement).focus(); } } }; - _onKeyDown = (ev) => { + private onKeyDown = (ev: React.KeyboardEvent) => { if (!this.props.managed) { if (ev.key === Key.ESCAPE) { this.props.onFinished(); @@ -237,16 +237,16 @@ export class ContextMenu extends React.PureComponent<IProps, IState> { this.props.onFinished(); break; case Key.ARROW_UP: - this._onMoveFocus(ev.target, true); + this.onMoveFocus(ev.target as Element, true); break; case Key.ARROW_DOWN: - this._onMoveFocus(ev.target, false); + this.onMoveFocus(ev.target as Element, false); break; case Key.HOME: - this._onMoveFocusHomeEnd(this.state.contextMenuElem, true); + this.onMoveFocusHomeEnd(this.state.contextMenuElem, true); break; case Key.END: - this._onMoveFocusHomeEnd(this.state.contextMenuElem, false); + this.onMoveFocusHomeEnd(this.state.contextMenuElem, false); break; default: handled = false; @@ -259,7 +259,7 @@ export class ContextMenu extends React.PureComponent<IProps, IState> { } }; - renderMenu(hasBackground = this.props.hasBackground) { + protected renderMenu(hasBackground = this.props.hasBackground) { const position: Partial<Writeable<DOMRect>> = {}; const props = this.props; @@ -356,7 +356,7 @@ export class ContextMenu extends React.PureComponent<IProps, IState> { <div className="mx_ContextualMenu_background" style={wrapperStyle} - onClick={this._onFinished} + onClick={this.onFinished} onContextMenu={this.onContextMenu} /> ); @@ -366,7 +366,7 @@ export class ContextMenu extends React.PureComponent<IProps, IState> { <div className="mx_ContextualMenu_wrapper" style={{...position, ...wrapperStyle}} - onKeyDown={this._onKeyDown} + onKeyDown={this.onKeyDown} onContextMenu={this.onContextMenuPreventBubbling} > <div From 4c7014167d19a55b858f4d7d61a959968347b4f1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jul 2020 01:06:36 +0100 Subject: [PATCH 06/72] Improve a11y of default BaseAvatar Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/avatars/BaseAvatar.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/avatars/BaseAvatar.js b/src/components/views/avatars/BaseAvatar.js index 508691e5fd..53e8d0072b 100644 --- a/src/components/views/avatars/BaseAvatar.js +++ b/src/components/views/avatars/BaseAvatar.js @@ -132,7 +132,7 @@ const BaseAvatar = (props) => { ); } else { return ( - <span className="mx_BaseAvatar" ref={inputRef} {...otherProps}> + <span className="mx_BaseAvatar" ref={inputRef} {...otherProps} role="presentation"> { textNode } { imgNode } </span> From 1620feb55eb4419dd660439f2d91d4434a468c09 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jul 2020 01:07:46 +0100 Subject: [PATCH 07/72] Sprinkle in some better ARIA props Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/LeftPanel2.tsx | 9 ++++-- src/components/structures/RoomSearch.tsx | 7 +++-- src/components/views/rooms/RoomList2.tsx | 3 -- src/components/views/rooms/RoomSublist2.tsx | 22 +++++++++----- src/components/views/rooms/RoomTile2.tsx | 33 ++++++++++++++++++--- 5 files changed, 55 insertions(+), 19 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 23a9e74646..0b3fa2f7f6 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -235,7 +235,12 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { private renderSearchExplore(): React.ReactNode { return ( - <div className="mx_LeftPanel2_filterContainer" onFocus={this.onFocus} onBlur={this.onBlur}> + <div + className="mx_LeftPanel2_filterContainer" + onFocus={this.onFocus} + onBlur={this.onBlur} + onKeyDown={this.onKeyDown} + > <RoomSearch onQueryUpdate={this.onSearch} isMinimized={this.props.isMinimized} @@ -245,7 +250,7 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { // TODO fix the accessibility of this: https://github.com/vector-im/riot-web/issues/14180 className="mx_LeftPanel2_exploreButton" onClick={this.onExplore} - alt={_t("Explore rooms")} + title={_t("Explore rooms")} /> </div> ); diff --git a/src/components/structures/RoomSearch.tsx b/src/components/structures/RoomSearch.tsx index 7ed2acf276..15f3bd5b54 100644 --- a/src/components/structures/RoomSearch.tsx +++ b/src/components/structures/RoomSearch.tsx @@ -149,7 +149,8 @@ export default class RoomSearch extends React.PureComponent<IProps, IState> { let clearButton = ( <AccessibleButton tabIndex={-1} - className='mx_RoomSearch_clearButton' + title={_t("Clear filter")} + className="mx_RoomSearch_clearButton" onClick={this.clearInput} /> ); @@ -157,8 +158,8 @@ export default class RoomSearch extends React.PureComponent<IProps, IState> { if (this.props.isMinimized) { icon = ( <AccessibleButton - tabIndex={-1} - className='mx_RoomSearch_icon' + title={_t("Search rooms")} + className="mx_RoomSearch_icon" onClick={this.openSearch} /> ); diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index b0bb70c9a0..06708931a2 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -276,9 +276,6 @@ export default class RoomList2 extends React.Component<IProps, IState> { className="mx_RoomList2" role="tree" aria-label={_t("Rooms")} - // Firefox sometimes makes this element focusable due to - // overflow:scroll;, so force it out of tab order. - tabIndex={-1} >{sublists}</div> )} </RovingTabIndexProvider> diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 21e7c581f0..69125ca88f 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -63,7 +63,7 @@ interface IProps { onAddRoom?: () => void; addRoomLabel: string; isInvite: boolean; - layout: ListLayout; + layout?: ListLayout; isMinimized: boolean; tagId: TagID; @@ -203,6 +203,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { dis.dispatch({ action: 'view_room', room_id: room.roomId, + show_room_tile: true, // to make sure the room gets scrolled into view }); } }; @@ -383,16 +384,22 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { private renderHeader(): React.ReactElement { return ( - <RovingTabIndexWrapper> + <RovingTabIndexWrapper inputRef={this.headerButton}> {({onFocus, isActive, ref}) => { const tabIndex = isActive ? 0 : -1; + let ariaLabel = _t("Jump to first unread room."); + if (this.props.tagId === DefaultTagID.Invite) { + ariaLabel = _t("Jump to first invite."); + } + const badge = ( <NotificationBadge forceCount={true} notification={this.state.notificationState} onClick={this.onBadgeClick} tabIndex={tabIndex} + aria-label={ariaLabel} /> ); @@ -433,7 +440,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { // doesn't become sticky. // The same applies to the notification badge. return ( - <div className={classes} onKeyDown={this.onHeaderKeyDown} onFocus={onFocus}> + <div className={classes} onKeyDown={this.onHeaderKeyDown} onFocus={onFocus} aria-label={this.props.label}> <div className="mx_RoomSublist2_stickable"> <AccessibleButton onFocus={onFocus} @@ -441,6 +448,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { tabIndex={tabIndex} className="mx_RoomSublist2_headerText" role="treeitem" + aria-expanded={!this.props.layout || !this.props.layout.isCollapsed} aria-level={1} onClick={this.onHeaderClick} onContextMenu={this.onContextMenu} @@ -496,12 +504,12 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { ); if (this.props.isMinimized) showMoreText = null; showNButton = ( - <div onClick={this.onShowAllClick} className={showMoreBtnClasses}> + <AccessibleButton onClick={this.onShowAllClick} className={showMoreBtnClasses} tabIndex={-1}> <span className='mx_RoomSublist2_showMoreButtonChevron mx_RoomSublist2_showNButtonChevron'> {/* set by CSS masking */} </span> {showMoreText} - </div> + </AccessibleButton> ); } else if (this.numTiles <= visibleTiles.length && this.numTiles > this.props.layout.defaultVisibleTiles) { // we have all tiles visible - add a button to show less @@ -512,12 +520,12 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { ); if (this.props.isMinimized) showLessText = null; showNButton = ( - <div onClick={this.onShowLessClick} className={showMoreBtnClasses}> + <AccessibleButton onClick={this.onShowLessClick} className={showMoreBtnClasses} tabIndex={-1}> <span className='mx_RoomSublist2_showLessButtonChevron mx_RoomSublist2_showNButtonChevron'> {/* set by CSS masking */} </span> {showLessText} - </div> + </AccessibleButton> ); } diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 8a9712b5a4..46b6d57501 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -30,9 +30,15 @@ import { ContextMenu, ContextMenuButton, MenuItemRadio } from "../../structures/ import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import { MessagePreviewStore } from "../../../stores/room-list/MessagePreviewStore"; import DecoratedRoomAvatar from "../avatars/DecoratedRoomAvatar"; -import { getRoomNotifsState, ALL_MESSAGES, ALL_MESSAGES_LOUD, MENTIONS_ONLY, MUTE } from "../../../RoomNotifs"; +import { + getRoomNotifsState, + setRoomNotifsState, + ALL_MESSAGES, + ALL_MESSAGES_LOUD, + MENTIONS_ONLY, + MUTE, +} from "../../../RoomNotifs"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; -import { setRoomNotifsState } from "../../../RoomNotifs"; import { TagSpecificNotificationState } from "../../../stores/notifications/TagSpecificNotificationState"; import { INotificationState } from "../../../stores/notifications/INotificationState"; import NotificationBadge from "./NotificationBadge"; @@ -406,10 +412,11 @@ export default class RoomTile2 extends React.Component<IProps, IState> { } } + const notificationColor = this.state.notificationState.color; const nameClasses = classNames({ "mx_RoomTile2_name": true, "mx_RoomTile2_nameWithPreview": !!messagePreview, - "mx_RoomTile2_nameHasUnreadEvents": this.state.notificationState.color >= NotificationColor.Bold, + "mx_RoomTile2_nameHasUnreadEvents": notificationColor >= NotificationColor.Bold, }); let nameContainer = ( @@ -422,6 +429,22 @@ export default class RoomTile2 extends React.Component<IProps, IState> { ); if (this.props.isMinimized) nameContainer = null; + let ariaLabel = name; + // The following labels are written in such a fashion to increase screen reader efficiency (speed). + if (this.props.tag === DefaultTagID.Invite) { + // append nothing + } else if (notificationColor >= NotificationColor.Red) { + ariaLabel += " " + _t("%(count)s unread messages including mentions.", { + count: this.state.notificationState.count, + }); + } else if (notificationColor >= NotificationColor.Grey) { + ariaLabel += " " + _t("%(count)s unread messages.", { + count: this.state.notificationState.count, + }); + } else if (notificationColor >= NotificationColor.Bold) { + ariaLabel += " " + _t("Unread messages."); + } + return ( <React.Fragment> <RovingTabIndexWrapper> @@ -434,8 +457,10 @@ export default class RoomTile2 extends React.Component<IProps, IState> { onMouseEnter={this.onTileMouseEnter} onMouseLeave={this.onTileMouseLeave} onClick={this.onTileClick} - role="treeitem" onContextMenu={this.onContextMenu} + role="treeitem" + aria-label={ariaLabel} + aria-selected={this.state.selected} > {roomAvatar} {nameContainer} From 4f1cd82b665e2d481cae53d3ae0a9f1e337aff8a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jul 2020 01:24:22 +0100 Subject: [PATCH 08/72] i18n Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/i18n/strings/en_EN.json | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index b23264a297..3794816a27 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1208,6 +1208,8 @@ "Show": "Show", "Message preview": "Message preview", "List options": "List options", + "Jump to first unread room.": "Jump to first unread room.", + "Jump to first invite.": "Jump to first invite.", "Add room": "Add room", "Show %(count)s more|other": "Show %(count)s more", "Show %(count)s more|one": "Show %(count)s more", @@ -2089,6 +2091,8 @@ "Find a room…": "Find a room…", "Find a room… (e.g. %(exampleRoom)s)": "Find a room… (e.g. %(exampleRoom)s)", "If you can't find the room you're looking for, ask for an invite or <a>Create a new room</a>.": "If you can't find the room you're looking for, ask for an invite or <a>Create a new room</a>.", + "Clear filter": "Clear filter", + "Search rooms": "Search rooms", "You can't send any messages until you review and agree to <consentLink>our terms and conditions</consentLink>.": "You can't send any messages until you review and agree to <consentLink>our terms and conditions</consentLink>.", "Your message wasn't sent because this homeserver has hit its Monthly Active User Limit. Please <a>contact your service administrator</a> to continue using the service.": "Your message wasn't sent because this homeserver has hit its Monthly Active User Limit. Please <a>contact your service administrator</a> to continue using the service.", "Your message wasn't sent because this homeserver has exceeded a resource limit. Please <a>contact your service administrator</a> to continue using the service.": "Your message wasn't sent because this homeserver has exceeded a resource limit. Please <a>contact your service administrator</a> to continue using the service.", @@ -2100,8 +2104,6 @@ "Sent messages will be stored until your connection has returned.": "Sent messages will be stored until your connection has returned.", "Active call": "Active call", "There's no one else here! Would you like to <inviteText>invite others</inviteText> or <nowarnText>stop warning about the empty room</nowarnText>?": "There's no one else here! Would you like to <inviteText>invite others</inviteText> or <nowarnText>stop warning about the empty room</nowarnText>?", - "Jump to first unread room.": "Jump to first unread room.", - "Jump to first invite.": "Jump to first invite.", "You seem to be uploading files, are you sure you want to quit?": "You seem to be uploading files, are you sure you want to quit?", "You seem to be in a call, are you sure you want to quit?": "You seem to be in a call, are you sure you want to quit?", "Search failed": "Search failed", @@ -2116,7 +2118,6 @@ "Click to mute video": "Click to mute video", "Click to unmute audio": "Click to unmute audio", "Click to mute audio": "Click to mute audio", - "Clear filter": "Clear filter", "Tried to load a specific point in this room's timeline, but you do not have permission to view the message in question.": "Tried to load a specific point in this room's timeline, but you do not have permission to view the message in question.", "Tried to load a specific point in this room's timeline, but was unable to find it.": "Tried to load a specific point in this room's timeline, but was unable to find it.", "Failed to load timeline position": "Failed to load timeline position", From 83cfdd9c07e647ee7ed9c5f728e421771bf9c625 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jul 2020 01:30:22 +0100 Subject: [PATCH 09/72] Fix accessibility of the Explore button Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- res/css/structures/_LeftPanel2.scss | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/res/css/structures/_LeftPanel2.scss b/res/css/structures/_LeftPanel2.scss index bdaada0d15..a73658d916 100644 --- a/res/css/structures/_LeftPanel2.scss +++ b/res/css/structures/_LeftPanel2.scss @@ -86,11 +86,15 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations .mx_RoomSearch_expanded + .mx_LeftPanel2_exploreButton { // Cheaty way to return the occupied space to the filter input + flex-basis: 0; margin: 0; width: 0; - // Don't forget to hide the masked ::before icon - visibility: hidden; + // Don't forget to hide the masked ::before icon, + // using display:none or visibility:hidden would break accessibility + &::before { + content: none; + } } .mx_LeftPanel2_exploreButton { From 069cdf3ce01fa29181b53dc1d336bf699d7b709b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jul 2020 18:23:57 +0100 Subject: [PATCH 10/72] Fix room list v2 context menus to be aria menus Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.js | 36 +++++++++++++++++++++ src/components/views/rooms/RoomSublist2.tsx | 26 ++++++++------- src/components/views/rooms/RoomTile2.tsx | 24 ++++++++++---- 3 files changed, 67 insertions(+), 19 deletions(-) diff --git a/src/components/structures/ContextMenu.js b/src/components/structures/ContextMenu.js index e43b0d1431..038208e49f 100644 --- a/src/components/structures/ContextMenu.js +++ b/src/components/structures/ContextMenu.js @@ -23,6 +23,8 @@ import classNames from 'classnames'; import {Key} from "../../Keyboard"; import * as sdk from "../../index"; import AccessibleButton from "../views/elements/AccessibleButton"; +import StyledCheckbox from "../views/elements/StyledCheckbox"; +import StyledRadioButton from "../views/elements/StyledRadioButton"; // Shamelessly ripped off Modal.js. There's probably a better way // of doing reusable widgets like dialog boxes & menus where we go and @@ -421,6 +423,23 @@ MenuItemCheckbox.propTypes = { onClick: PropTypes.func.isRequired, }; +// Semantic component for representing a styled role=menuitemcheckbox +export const StyledMenuItemCheckbox = ({children, label, active=false, disabled=false, ...props}) => { + return ( + <StyledCheckbox {...props} role="menuitemcheckbox" aria-checked={active} aria-disabled={disabled} tabIndex={-1} aria-label={label}> + { children } + </StyledCheckbox> + ); +}; +StyledMenuItemCheckbox.propTypes = { + ...AccessibleButton.propTypes, + label: PropTypes.string, // optional + active: PropTypes.bool.isRequired, + disabled: PropTypes.bool, // optional + className: PropTypes.string, // optional + onClick: PropTypes.func.isRequired, +}; + // Semantic component for representing a role=menuitemradio export const MenuItemRadio = ({children, label, active=false, disabled=false, ...props}) => { const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); @@ -439,6 +458,23 @@ MenuItemRadio.propTypes = { onClick: PropTypes.func.isRequired, }; +// Semantic component for representing a styled role=menuitemradio +export const StyledMenuItemRadio = ({children, label, active=false, disabled=false, ...props}) => { + return ( + <StyledRadioButton {...props} role="menuitemradio" aria-checked={active} aria-disabled={disabled} tabIndex={-1} aria-label={label}> + { children } + </StyledRadioButton> + ); +}; +StyledMenuItemRadio.propTypes = { + ...StyledMenuItemRadio.propTypes, + label: PropTypes.string, // optional + active: PropTypes.bool.isRequired, + disabled: PropTypes.bool, // optional + className: PropTypes.string, // optional + onClick: PropTypes.func.isRequired, +}; + // Placement method for <ContextMenu /> to position context menu to right of elementRect with chevronOffset export const toRightOf = (elementRect, chevronOffset=12) => { const left = elementRect.right + window.pageXOffset + 3; diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 69125ca88f..1b1e2ed66d 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -26,16 +26,18 @@ import AccessibleButton from "../../views/elements/AccessibleButton"; import RoomTile2 from "./RoomTile2"; import { ResizableBox, ResizeCallbackData } from "react-resizable"; import { ListLayout } from "../../../stores/room-list/ListLayout"; -import { ContextMenu, ContextMenuButton } from "../../structures/ContextMenu"; -import StyledCheckbox from "../elements/StyledCheckbox"; -import StyledRadioButton from "../elements/StyledRadioButton"; +import { + ContextMenu, + ContextMenuButton, + StyledMenuItemCheckbox, + StyledMenuItemRadio, +} from "../../structures/ContextMenu"; import RoomListStore from "../../../stores/room-list/RoomListStore2"; import { ListAlgorithm, SortAlgorithm } from "../../../stores/room-list/algorithms/models"; import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import dis from "../../../dispatcher/dispatcher"; import NotificationBadge from "./NotificationBadge"; import { ListNotificationState } from "../../../stores/notifications/ListNotificationState"; -import Tooltip from "../elements/Tooltip"; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; import { Key } from "../../../Keyboard"; @@ -329,40 +331,40 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { <div className="mx_RoomSublist2_contextMenu"> <div> <div className='mx_RoomSublist2_contextMenu_title'>{_t("Sort by")}</div> - <StyledRadioButton + <StyledMenuItemRadio onChange={() => this.onTagSortChanged(SortAlgorithm.Recent)} checked={!isAlphabetical} name={`mx_${this.props.tagId}_sortBy`} > {_t("Activity")} - </StyledRadioButton> - <StyledRadioButton + </StyledMenuItemRadio> + <StyledMenuItemRadio onChange={() => this.onTagSortChanged(SortAlgorithm.Alphabetic)} checked={isAlphabetical} name={`mx_${this.props.tagId}_sortBy`} > {_t("A-Z")} - </StyledRadioButton> + </StyledMenuItemRadio> </div> <hr /> <div> <div className='mx_RoomSublist2_contextMenu_title'>{_t("Unread rooms")}</div> - <StyledCheckbox + <StyledMenuItemCheckbox onChange={this.onUnreadFirstChanged} checked={isUnreadFirst} > {_t("Always show first")} - </StyledCheckbox> + </StyledMenuItemCheckbox> </div> <hr /> <div> <div className='mx_RoomSublist2_contextMenu_title'>{_t("Show")}</div> - <StyledCheckbox + <StyledMenuItemCheckbox onChange={this.onMessagePreviewChanged} checked={this.props.layout.showPreviews} > {_t("Message preview")} - </StyledCheckbox> + </StyledMenuItemCheckbox> </div> </div> </ContextMenu> diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 46b6d57501..dbaed0d819 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -26,7 +26,13 @@ import dis from '../../../dispatcher/dispatcher'; import { Key } from "../../../Keyboard"; import ActiveRoomObserver from "../../../ActiveRoomObserver"; import { _t } from "../../../languageHandler"; -import { ContextMenu, ContextMenuButton, MenuItemRadio } from "../../structures/ContextMenu"; +import { + ContextMenu, + ContextMenuButton, + MenuItemRadio, + MenuItemCheckbox, + MenuItem, +} from "../../structures/ContextMenu"; import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import { MessagePreviewStore } from "../../../stores/room-list/MessagePreviewStore"; import DecoratedRoomAvatar from "../avatars/DecoratedRoomAvatar"; @@ -328,20 +334,24 @@ export default class RoomTile2 extends React.Component<IProps, IState> { <ContextMenu {...contextMenuBelow(this.state.generalMenuPosition)} onFinished={this.onCloseGeneralMenu}> <div className="mx_IconizedContextMenu mx_IconizedContextMenu_compact mx_RoomTile2_contextMenu"> <div className="mx_IconizedContextMenu_optionList"> - <AccessibleButton onClick={(e) => this.onTagRoom(e, DefaultTagID.Favourite)}> + <MenuItemCheckbox + onClick={(e) => this.onTagRoom(e, DefaultTagID.Favourite)} + active={false} // TODO: https://github.com/vector-im/riot-web/issues/14283 + label={_t("Favourite")} + > <span className="mx_IconizedContextMenu_icon mx_RoomTile2_iconStar" /> <span className="mx_IconizedContextMenu_label">{_t("Favourite")}</span> - </AccessibleButton> - <AccessibleButton onClick={this.onOpenRoomSettings}> + </MenuItemCheckbox> + <MenuItem onClick={this.onOpenRoomSettings} label={_t("Settings")}> <span className="mx_IconizedContextMenu_icon mx_RoomTile2_iconSettings" /> <span className="mx_IconizedContextMenu_label">{_t("Settings")}</span> - </AccessibleButton> + </MenuItem> </div> <div className="mx_IconizedContextMenu_optionList mx_RoomTile2_contextMenu_redRow"> - <AccessibleButton onClick={this.onLeaveRoomClick}> + <MenuItem onClick={this.onLeaveRoomClick} label={_t("Leave Room")}> <span className="mx_IconizedContextMenu_icon mx_RoomTile2_iconSignOut" /> <span className="mx_IconizedContextMenu_label">{_t("Leave Room")}</span> - </AccessibleButton> + </MenuItem> </div> </div> </ContextMenu> From 3cebfc8072b5f84de38c205b5e9be52788d81673 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jul 2020 19:31:24 +0100 Subject: [PATCH 11/72] Fix StyledMenuItemCheckbox and StyledMenuItemRadio Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.js | 28 +++++++++++++++++++----- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/components/structures/ContextMenu.js b/src/components/structures/ContextMenu.js index 038208e49f..c4825ca1da 100644 --- a/src/components/structures/ContextMenu.js +++ b/src/components/structures/ContextMenu.js @@ -424,9 +424,17 @@ MenuItemCheckbox.propTypes = { }; // Semantic component for representing a styled role=menuitemcheckbox -export const StyledMenuItemCheckbox = ({children, label, active=false, disabled=false, ...props}) => { +export const StyledMenuItemCheckbox = ({children, label, checked=false, disabled=false, ...props}) => { return ( - <StyledCheckbox {...props} role="menuitemcheckbox" aria-checked={active} aria-disabled={disabled} tabIndex={-1} aria-label={label}> + <StyledCheckbox + {...props} + role="menuitemcheckbox" + aria-checked={checked} + checked={checked} + aria-disabled={disabled} + tabIndex={-1} + aria-label={label} + > { children } </StyledCheckbox> ); @@ -434,7 +442,7 @@ export const StyledMenuItemCheckbox = ({children, label, active=false, disabled= StyledMenuItemCheckbox.propTypes = { ...AccessibleButton.propTypes, label: PropTypes.string, // optional - active: PropTypes.bool.isRequired, + checked: PropTypes.bool.isRequired, disabled: PropTypes.bool, // optional className: PropTypes.string, // optional onClick: PropTypes.func.isRequired, @@ -459,9 +467,17 @@ MenuItemRadio.propTypes = { }; // Semantic component for representing a styled role=menuitemradio -export const StyledMenuItemRadio = ({children, label, active=false, disabled=false, ...props}) => { +export const StyledMenuItemRadio = ({children, label, checked=false, disabled=false, ...props}) => { return ( - <StyledRadioButton {...props} role="menuitemradio" aria-checked={active} aria-disabled={disabled} tabIndex={-1} aria-label={label}> + <StyledRadioButton + {...props} + role="menuitemradio" + aria-checked={checked} + checked={checked} + aria-disabled={disabled} + tabIndex={-1} + aria-label={label} + > { children } </StyledRadioButton> ); @@ -469,7 +485,7 @@ export const StyledMenuItemRadio = ({children, label, active=false, disabled=fal StyledMenuItemRadio.propTypes = { ...StyledMenuItemRadio.propTypes, label: PropTypes.string, // optional - active: PropTypes.bool.isRequired, + checked: PropTypes.bool.isRequired, disabled: PropTypes.bool, // optional className: PropTypes.string, // optional onClick: PropTypes.func.isRequired, From a68e23c9e089a66894f85b8cdd24ae0cc2be54b4 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jul 2020 19:38:45 +0100 Subject: [PATCH 12/72] Make message previews accessible via describedby Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomTile2.tsx | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index dbaed0d819..3d9a4b5aca 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -80,6 +80,8 @@ interface IState { generalMenuPosition: PartialDOMRect; } +const messagePreviewId = (roomId: string) => `mx_RoomTile2_messagePreview_${roomId}`; + const contextMenuBelow = (elementRect: PartialDOMRect) => { // align the context menu's icons with the icon which opened the context menu const left = elementRect.left + window.pageXOffset - 9; @@ -135,6 +137,10 @@ export default class RoomTile2 extends React.Component<IProps, IState> { return !this.props.isMinimized && this.props.tag !== DefaultTagID.Invite; } + private get showMessagePreview(): boolean { + return !this.props.isMinimized && this.props.showMessagePreview; + } + public componentWillUnmount() { if (this.props.room) { ActiveRoomObserver.removeListener(this.props.room.roomId, this.onActiveRoomUpdate); @@ -408,14 +414,14 @@ export default class RoomTile2 extends React.Component<IProps, IState> { name = name.replace(":", ":\u200b"); // add a zero-width space to allow linewrapping after the colon let messagePreview = null; - if (this.props.showMessagePreview && !this.props.isMinimized) { + if (this.showMessagePreview) { // The preview store heavily caches this info, so should be safe to hammer. const text = MessagePreviewStore.instance.getPreviewForRoom(this.props.room, this.props.tag); // Only show the preview if there is one to show. if (text) { messagePreview = ( - <div className="mx_RoomTile2_messagePreview"> + <div className="mx_RoomTile2_messagePreview" id={messagePreviewId(this.props.room.roomId)}> {text} </div> ); @@ -455,6 +461,11 @@ export default class RoomTile2 extends React.Component<IProps, IState> { ariaLabel += " " + _t("Unread messages."); } + let ariaDescribedBy: string; + if (this.showMessagePreview) { + ariaDescribedBy = messagePreviewId(this.props.room.roomId); + } + return ( <React.Fragment> <RovingTabIndexWrapper> @@ -471,6 +482,7 @@ export default class RoomTile2 extends React.Component<IProps, IState> { role="treeitem" aria-label={ariaLabel} aria-selected={this.state.selected} + aria-describedby={ariaDescribedBy} > {roomAvatar} {nameContainer} From 7c29a53ebdca7cc06dbd51e34565371c9b04561e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 5 Jul 2020 19:59:29 +0100 Subject: [PATCH 13/72] aria-hide the notifications badge on room tiles as we have manual labels here Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomTile2.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 3d9a4b5aca..f3b22c3a64 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -397,8 +397,9 @@ export default class RoomTile2 extends React.Component<IProps, IState> { let badge: React.ReactNode; if (!this.props.isMinimized) { + // aria-hidden because we summarise the unread count/highlight status in a manual aria-label below badge = ( - <div className="mx_RoomTile2_badgeContainer"> + <div className="mx_RoomTile2_badgeContainer" aria-hidden="true"> <NotificationBadge notification={this.state.notificationState} forceCount={false} From 2d09ac9f88655cbcecdeb9891ced9ad1d82a9e3c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 01:01:40 +0100 Subject: [PATCH 14/72] Improve UserMenu label Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/UserMenu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/UserMenu.tsx b/src/components/structures/UserMenu.tsx index 5955a046a4..221ec07439 100644 --- a/src/components/structures/UserMenu.tsx +++ b/src/components/structures/UserMenu.tsx @@ -329,7 +329,7 @@ export default class UserMenu extends React.Component<IProps, IState> { className={classes} onClick={this.onOpenMenuClick} inputRef={this.buttonRef} - label={_t("Account settings")} + label={_t("User menu")} isExpanded={!!this.state.contextMenuPosition} onContextMenu={this.onContextMenu} > From b7c23b690c41e8b3c407a0551eb40787f7d5be88 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 01:14:02 +0100 Subject: [PATCH 15/72] Include more classes for room list keyboard navigation Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/LeftPanel2.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 0b3fa2f7f6..70fcf327b3 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -55,6 +55,15 @@ interface IState { showTagPanel: boolean; } +// List of CSS classes which should be included in keyboard navigation within the room list +const cssClasses = [ + "mx_RoomSearch_input", + "mx_RoomSearch_icon", // minimized <RoomSearch /> + "mx_RoomSublist2_headerText", + "mx_RoomTile2", + "mx_RoomSublist2_showNButton", +]; + export default class LeftPanel2 extends React.Component<IProps, IState> { private listContainerRef: React.RefObject<HTMLDivElement> = createRef(); private tagPanelWatcherRef: string; @@ -204,10 +213,7 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { if (element) { classes = element.classList; } - } while (element && !( - classes.contains("mx_RoomTile2") || - classes.contains("mx_RoomSublist2_headerText") || - classes.contains("mx_RoomSearch_input"))); + } while (element && !cssClasses.some(c => classes.contains(c))); if (element) { element.focus(); From 87a7a8a02b97660f3495c4e47a158779b2462e79 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 01:15:53 +0100 Subject: [PATCH 16/72] Add TODOs Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomSublist2.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 1b1e2ed66d..bc42f2321c 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -138,11 +138,13 @@ 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 this.props.layout.visibleTiles = this.props.layout.tilesWithPadding(this.numTiles, MAX_PADDING_HEIGHT); this.forceUpdate(); // because the layout doesn't trigger a re-render }; 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 }; @@ -521,6 +523,7 @@ 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}> <span className='mx_RoomSublist2_showLessButtonChevron mx_RoomSublist2_showNButtonChevron'> From d366ca12a0c1737eede5219b9d04cf8a50a3c512 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 01:19:00 +0100 Subject: [PATCH 17/72] i18n Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/i18n/strings/en_EN.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 3794816a27..f9a2f3a5a0 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2132,7 +2132,7 @@ "All settings": "All settings", "Archived rooms": "Archived rooms", "Feedback": "Feedback", - "Account settings": "Account settings", + "User menu": "User menu", "Could not load user profile": "Could not load user profile", "Verify this login": "Verify this login", "Session verified": "Session verified", From 6cb0ac6a5010ae88b9008a0445e357a5456975b7 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 10:18:49 +0100 Subject: [PATCH 18/72] Fix checkboxes/radios in context menus should only close on ENTER, not SPACE Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.js | 38 ++++++++++++++++++--- src/components/views/rooms/RoomSublist2.tsx | 4 +++ src/components/views/rooms/RoomTile2.tsx | 16 +++++++-- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/components/structures/ContextMenu.js b/src/components/structures/ContextMenu.js index c4825ca1da..76f31a1017 100644 --- a/src/components/structures/ContextMenu.js +++ b/src/components/structures/ContextMenu.js @@ -424,7 +424,18 @@ MenuItemCheckbox.propTypes = { }; // Semantic component for representing a styled role=menuitemcheckbox -export const StyledMenuItemCheckbox = ({children, label, checked=false, disabled=false, ...props}) => { +export const StyledMenuItemCheckbox = ({children, label, onChange, onClose, checked=false, disabled=false, ...props}) => { + const onKeyDown = (ev) => { + // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 + if (ev.key === Key.ENTER || ev.key === Key.SPACE) { + ev.preventDefault(); + ev.stopPropagation(); + onChange(ev); + if (ev.key === Key.ENTER) { + onClose(); + } + } + }; return ( <StyledCheckbox {...props} @@ -434,18 +445,21 @@ export const StyledMenuItemCheckbox = ({children, label, checked=false, disabled aria-disabled={disabled} tabIndex={-1} aria-label={label} + onChange={onChange} + onKeyDown={onKeyDown} > { children } </StyledCheckbox> ); }; StyledMenuItemCheckbox.propTypes = { - ...AccessibleButton.propTypes, + ...StyledCheckbox.propTypes, label: PropTypes.string, // optional checked: PropTypes.bool.isRequired, disabled: PropTypes.bool, // optional className: PropTypes.string, // optional - onClick: PropTypes.func.isRequired, + onChange: PropTypes.func.isRequired, + onClose: PropTypes.func.isRequired, // gets called after onChange on Key.ENTER }; // Semantic component for representing a role=menuitemradio @@ -467,7 +481,18 @@ MenuItemRadio.propTypes = { }; // Semantic component for representing a styled role=menuitemradio -export const StyledMenuItemRadio = ({children, label, checked=false, disabled=false, ...props}) => { +export const StyledMenuItemRadio = ({children, label, onChange, onClose, checked=false, disabled=false, ...props}) => { + const onKeyDown = (ev) => { + // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 + if (ev.key === Key.ENTER || ev.key === Key.SPACE) { + ev.preventDefault(); + ev.stopPropagation(); + onChange(ev); + if (ev.key === Key.ENTER) { + onClose(); + } + } + }; return ( <StyledRadioButton {...props} @@ -477,6 +502,8 @@ export const StyledMenuItemRadio = ({children, label, checked=false, disabled=fa aria-disabled={disabled} tabIndex={-1} aria-label={label} + onChange={onChange} + onKeyDown={onKeyDown} > { children } </StyledRadioButton> @@ -488,7 +515,8 @@ StyledMenuItemRadio.propTypes = { checked: PropTypes.bool.isRequired, disabled: PropTypes.bool, // optional className: PropTypes.string, // optional - onClick: PropTypes.func.isRequired, + onChange: PropTypes.func.isRequired, + onClose: PropTypes.func.isRequired, // gets called after onChange on Key.ENTER }; // Placement method for <ContextMenu /> to position context menu to right of elementRect with chevronOffset diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index bc42f2321c..9817302445 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -334,6 +334,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { <div> <div className='mx_RoomSublist2_contextMenu_title'>{_t("Sort by")}</div> <StyledMenuItemRadio + onClose={this.onCloseMenu} onChange={() => this.onTagSortChanged(SortAlgorithm.Recent)} checked={!isAlphabetical} name={`mx_${this.props.tagId}_sortBy`} @@ -341,6 +342,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { {_t("Activity")} </StyledMenuItemRadio> <StyledMenuItemRadio + onClose={this.onCloseMenu} onChange={() => this.onTagSortChanged(SortAlgorithm.Alphabetic)} checked={isAlphabetical} name={`mx_${this.props.tagId}_sortBy`} @@ -352,6 +354,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { <div> <div className='mx_RoomSublist2_contextMenu_title'>{_t("Unread rooms")}</div> <StyledMenuItemCheckbox + onClose={this.onCloseMenu} onChange={this.onUnreadFirstChanged} checked={isUnreadFirst} > @@ -362,6 +365,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { <div> <div className='mx_RoomSublist2_contextMenu_title'>{_t("Show")}</div> <StyledMenuItemCheckbox + onClose={this.onCloseMenu} onChange={this.onMessagePreviewChanged} checked={this.props.layout.showPreviews} > diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index f3b22c3a64..8ee838fbba 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -80,6 +80,8 @@ interface IState { generalMenuPosition: PartialDOMRect; } +type Volume = ALL_MESSAGES_LOUD | ALL_MESSAGES | MENTIONS_ONLY | MUTE; + const messagePreviewId = (roomId: string) => `mx_RoomTile2_messagePreview_${roomId}`; const contextMenuBelow = (elementRect: PartialDOMRect) => { @@ -213,6 +215,11 @@ export default class RoomTile2 extends React.Component<IProps, IState> { // TODO: Support tagging: https://github.com/vector-im/riot-web/issues/14211 // TODO: XOR favourites and low priority: https://github.com/vector-im/riot-web/issues/14210 + + if ((ev as React.KeyboardEvent).key === Key.ENTER) { + // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 + this.setState({generalMenuPosition: null}); // hide the menu + } }; private onLeaveRoomClick = (ev: ButtonEvent) => { @@ -237,11 +244,13 @@ export default class RoomTile2 extends React.Component<IProps, IState> { this.setState({generalMenuPosition: null}); // hide the menu }; - private async saveNotifState(ev: ButtonEvent, newState: ALL_MESSAGES_LOUD | ALL_MESSAGES | MENTIONS_ONLY | MUTE) { + private async saveNotifState(ev: ButtonEvent, newState: Volume) { ev.preventDefault(); ev.stopPropagation(); if (MatrixClientPeg.get().isGuest()) return; + // get key before we go async and React discards the nativeEvent + const key = (ev as React.KeyboardEvent).key; try { // TODO add local echo - https://github.com/vector-im/riot-web/issues/14280 await setRoomNotifsState(this.props.room.roomId, newState); @@ -251,7 +260,10 @@ export default class RoomTile2 extends React.Component<IProps, IState> { console.error(error); } - this.setState({notificationsMenuPosition: null}); // Close the context menu + if (key === Key.ENTER) { + // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 + this.setState({notificationsMenuPosition: null}); // hide the menu + } } private onClickAllNotifs = ev => this.saveNotifState(ev, ALL_MESSAGES); From 823ada374d17bea81ef37b1090acb196e00aafd5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 10:30:06 +0100 Subject: [PATCH 19/72] delint Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/ContextMenu.js b/src/components/structures/ContextMenu.js index 76f31a1017..157b3e1be3 100644 --- a/src/components/structures/ContextMenu.js +++ b/src/components/structures/ContextMenu.js @@ -424,7 +424,7 @@ MenuItemCheckbox.propTypes = { }; // Semantic component for representing a styled role=menuitemcheckbox -export const StyledMenuItemCheckbox = ({children, label, onChange, onClose, checked=false, disabled=false, ...props}) => { +export const StyledMenuItemCheckbox = ({children, label, onChange, onClose, checked, disabled=false, ...props}) => { const onKeyDown = (ev) => { // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 if (ev.key === Key.ENTER || ev.key === Key.SPACE) { From 66ca095706b6a146a7f9f0a356c869e1870c9a4b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 10:41:35 +0100 Subject: [PATCH 20/72] Fix double handling of native inputs wrapped for aria menus Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/ContextMenu.js | 46 ++++++++++++++++-------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/src/components/structures/ContextMenu.js b/src/components/structures/ContextMenu.js index 157b3e1be3..bda194ddd0 100644 --- a/src/components/structures/ContextMenu.js +++ b/src/components/structures/ContextMenu.js @@ -425,17 +425,25 @@ MenuItemCheckbox.propTypes = { // Semantic component for representing a styled role=menuitemcheckbox export const StyledMenuItemCheckbox = ({children, label, onChange, onClose, checked, disabled=false, ...props}) => { - const onKeyDown = (ev) => { - // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 - if (ev.key === Key.ENTER || ev.key === Key.SPACE) { - ev.preventDefault(); - ev.stopPropagation(); - onChange(ev); - if (ev.key === Key.ENTER) { + const onKeyDown = (e) => { + if (e.key === Key.ENTER || e.key === Key.SPACE) { + e.stopPropagation(); + e.preventDefault(); + onChange(); + // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 + if (e.key === Key.ENTER) { onClose(); } } }; + const onKeyUp = (e) => { + // prevent the input default handler as we handle it on keydown to match + // https://www.w3.org/TR/wai-aria-practices/examples/menubar/menubar-2/menubar-2.html + if (e.key === Key.SPACE || e.key === Key.ENTER) { + e.stopPropagation(); + e.preventDefault(); + } + }; return ( <StyledCheckbox {...props} @@ -447,6 +455,7 @@ export const StyledMenuItemCheckbox = ({children, label, onChange, onClose, chec aria-label={label} onChange={onChange} onKeyDown={onKeyDown} + onKeyUp={onKeyUp} > { children } </StyledCheckbox> @@ -482,17 +491,25 @@ MenuItemRadio.propTypes = { // Semantic component for representing a styled role=menuitemradio export const StyledMenuItemRadio = ({children, label, onChange, onClose, checked=false, disabled=false, ...props}) => { - const onKeyDown = (ev) => { - // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 - if (ev.key === Key.ENTER || ev.key === Key.SPACE) { - ev.preventDefault(); - ev.stopPropagation(); - onChange(ev); - if (ev.key === Key.ENTER) { + const onKeyDown = (e) => { + if (e.key === Key.ENTER || e.key === Key.SPACE) { + e.stopPropagation(); + e.preventDefault(); + onChange(); + // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 + if (e.key === Key.ENTER) { onClose(); } } }; + const onKeyUp = (e) => { + // prevent the input default handler as we handle it on keydown to match + // https://www.w3.org/TR/wai-aria-practices/examples/menubar/menubar-2/menubar-2.html + if (e.key === Key.SPACE || e.key === Key.ENTER) { + e.stopPropagation(); + e.preventDefault(); + } + }; return ( <StyledRadioButton {...props} @@ -504,6 +521,7 @@ export const StyledMenuItemRadio = ({children, label, onChange, onClose, checked aria-label={label} onChange={onChange} onKeyDown={onKeyDown} + onKeyUp={onKeyUp} > { children } </StyledRadioButton> From 6042e015e08c247e90887edefd643366207ebf7a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 17:49:37 +0100 Subject: [PATCH 21/72] Remove unused dispatches view_indexed_room and view_prev_room Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/LoggedInView.tsx | 14 -------------- src/components/structures/MatrixChat.tsx | 19 ------------------- 2 files changed, 33 deletions(-) diff --git a/src/components/structures/LoggedInView.tsx b/src/components/structures/LoggedInView.tsx index 9fbc98dee3..4876d79131 100644 --- a/src/components/structures/LoggedInView.tsx +++ b/src/components/structures/LoggedInView.tsx @@ -409,20 +409,6 @@ class LoggedInView extends React.Component<IProps, IState> { }; _onKeyDown = (ev) => { - /* - // Remove this for now as ctrl+alt = alt-gr so this breaks keyboards which rely on alt-gr for numbers - // Will need to find a better meta key if anyone actually cares about using this. - if (ev.altKey && ev.ctrlKey && ev.keyCode > 48 && ev.keyCode < 58) { - dis.dispatch({ - action: 'view_indexed_room', - roomIndex: ev.keyCode - 49, - }); - ev.stopPropagation(); - ev.preventDefault(); - return; - } - */ - let handled = false; const ctrlCmdOnly = isOnlyCtrlOrCmdKeyEvent(ev); const hasModifier = ev.altKey || ev.ctrlKey || ev.metaKey || ev.shiftKey; diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 315c648e15..89ee1bc22d 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -596,15 +596,9 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> { } break; } - case 'view_prev_room': - this.viewNextRoom(-1); - break; case 'view_next_room': this.viewNextRoom(1); break; - case 'view_indexed_room': - this.viewIndexedRoom(payload.roomIndex); - break; case Action.ViewUserSettings: { const tabPayload = payload as OpenToTabPayload; const UserSettingsDialog = sdk.getComponent("dialogs.UserSettingsDialog"); @@ -812,19 +806,6 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> { }); } - // TODO: Move to RoomViewStore - private viewIndexedRoom(roomIndex: number) { - const allRooms = RoomListSorter.mostRecentActivityFirst( - MatrixClientPeg.get().getRooms(), - ); - if (allRooms[roomIndex]) { - dis.dispatch({ - action: 'view_room', - room_id: allRooms[roomIndex].roomId, - }); - } - } - // switch view to the given room // // @param {Object} roomInfo Object containing data about the room to be joined From 2da1320d99d2e811311b4bd8e4eee617a420128e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 17:57:40 +0100 Subject: [PATCH 22/72] Type view_room_delta as ViewRoomDelta Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/LoggedInView.tsx | 5 +-- src/dispatcher/actions.ts | 5 +++ .../payloads/ViewRoomDeltaPayload.ts | 32 +++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 src/dispatcher/payloads/ViewRoomDeltaPayload.ts diff --git a/src/components/structures/LoggedInView.tsx b/src/components/structures/LoggedInView.tsx index 4876d79131..a5a1c628a3 100644 --- a/src/components/structures/LoggedInView.tsx +++ b/src/components/structures/LoggedInView.tsx @@ -53,6 +53,7 @@ import { } from "../../toasts/ServerLimitToast"; import { Action } from "../../dispatcher/actions"; import LeftPanel2 from "./LeftPanel2"; +import { ViewRoomDeltaPayload } from "../../dispatcher/payloads/ViewRoomDeltaPayload"; // 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. @@ -460,8 +461,8 @@ class LoggedInView extends React.Component<IProps, IState> { case Key.ARROW_UP: case Key.ARROW_DOWN: if (ev.altKey && !ev.ctrlKey && !ev.metaKey) { - dis.dispatch({ - action: 'view_room_delta', + dis.dispatch<ViewRoomDeltaPayload>({ + action: Action.ViewRoomDelta, delta: ev.key === Key.ARROW_UP ? -1 : 1, unread: ev.shiftKey, }); diff --git a/src/dispatcher/actions.ts b/src/dispatcher/actions.ts index 379a0a4451..9be674b59e 100644 --- a/src/dispatcher/actions.ts +++ b/src/dispatcher/actions.ts @@ -79,4 +79,9 @@ export enum Action { * Sets a system font. Should be used with UpdateSystemFontPayload */ UpdateSystemFont = "update_system_font", + + /** + * Changes room based on room list order and payload parameters. Should be used with ViewRoomDeltaPayload. + */ + ViewRoomDelta = "view_room_delta", } diff --git a/src/dispatcher/payloads/ViewRoomDeltaPayload.ts b/src/dispatcher/payloads/ViewRoomDeltaPayload.ts new file mode 100644 index 0000000000..de33a88b2e --- /dev/null +++ b/src/dispatcher/payloads/ViewRoomDeltaPayload.ts @@ -0,0 +1,32 @@ +/* +Copyright 2020 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 { ActionPayload } from "../payloads"; +import { Action } from "../actions"; + +export interface ViewRoomDeltaPayload extends ActionPayload { + action: Action.ViewRoomDelta; + + /** + * The delta index of the room to view. + */ + delta: number; + + /** + * Optionally, whether or not to filter to unread (Bold/Grey/Red) rooms only. (Default: false) + */ + unread?: boolean; +} From 1849ed90d264e2d84081e53d2d3a295c7aef17fa Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 17:58:29 +0100 Subject: [PATCH 23/72] Implement ViewRoomDelta for the new Room List Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomList2.tsx | 53 ++++++++++++++++++- .../notifications/RoomNotificationState.ts | 2 +- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index 06708931a2..0eef2143dd 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -17,13 +17,16 @@ limitations under the License. */ import * as React from "react"; +import { Dispatcher } from "flux"; +import { Room } from "matrix-js-sdk/src/models/room"; + import { _t, _td } from "../../../languageHandler"; import { RovingTabIndexProvider } from "../../../accessibility/RovingTabIndex"; import { ResizeNotifier } from "../../../utils/ResizeNotifier"; -import RoomListStore, { LISTS_UPDATE_EVENT, RoomListStore2 } from "../../../stores/room-list/RoomListStore2"; +import RoomListStore, { LISTS_UPDATE_EVENT } from "../../../stores/room-list/RoomListStore2"; +import RoomViewStore from "../../../stores/RoomViewStore"; import { ITagMap } from "../../../stores/room-list/algorithms/models"; import { DefaultTagID, TagID } from "../../../stores/room-list/models"; -import { Dispatcher } from "flux"; import dis from "../../../dispatcher/dispatcher"; import defaultDispatcher from "../../../dispatcher/dispatcher"; import RoomSublist2 from "./RoomSublist2"; @@ -35,6 +38,9 @@ import GroupAvatar from "../avatars/GroupAvatar"; import TemporaryTile from "./TemporaryTile"; import { StaticNotificationState } from "../../../stores/notifications/StaticNotificationState"; import { NotificationColor } from "../../../stores/notifications/NotificationColor"; +import { TagSpecificNotificationState } from "../../../stores/notifications/TagSpecificNotificationState"; +import { Action } from "../../../dispatcher/actions"; +import { ViewRoomDeltaPayload } from "../../../dispatcher/payloads/ViewRoomDeltaPayload"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -140,6 +146,7 @@ const TAG_AESTHETICS: { export default class RoomList2 extends React.Component<IProps, IState> { private searchFilter: NameFilterCondition = new NameFilterCondition(); + private dispatcherRef; constructor(props: IProps) { super(props); @@ -148,6 +155,8 @@ export default class RoomList2 extends React.Component<IProps, IState> { sublists: {}, layouts: new Map<TagID, ListLayout>(), }; + + this.dispatcherRef = defaultDispatcher.register(this.onAction); } public componentDidUpdate(prevProps: Readonly<IProps>): void { @@ -172,8 +181,48 @@ export default class RoomList2 extends React.Component<IProps, IState> { public componentWillUnmount() { RoomListStore.instance.off(LISTS_UPDATE_EVENT, this.updateLists); + defaultDispatcher.unregister(this.dispatcherRef); } + private onAction = (payload: ActionPayload) => { + if (payload.action === Action.ViewRoomDelta) { + const viewRoomDeltaPayload = payload as ViewRoomDeltaPayload; + const currentRoomId = RoomViewStore.getRoomId(); + const room = this.getRoomDelta(currentRoomId, viewRoomDeltaPayload.delta, viewRoomDeltaPayload.unread); + if (room) { + dis.dispatch({ + action: 'view_room', + room_id: room.roomId, + show_room_tile: true, // to make sure the room gets scrolled into view + }); + } + } + }; + + private getRoomDelta = (roomId: string, delta: number, unread = false) => { + const lists = RoomListStore.instance.orderedLists; + let rooms: Room = []; + TAG_ORDER.forEach(t => { + let listRooms = lists[t]; + + if (unread) { + const notificationStates = rooms.map(r => new TagSpecificNotificationState(r, t)); + // filter to only notification rooms (and our current active room so we can index properly) + listRooms = notificationStates.filter(state => { + return state.room.roomId === roomId || state.color >= NotificationColor.Bold; + }); + notificationStates.forEach(state => state.destroy()); + } + + rooms.push(...listRooms); + }); + + const currentIndex = rooms.findIndex(r => r.roomId === roomId); + // use slice to account for looping around the start + const [room] = rooms.slice((currentIndex + delta) % rooms.length); + return room; + }; + private updateLists = () => { const newLists = RoomListStore.instance.orderedLists; console.log("new lists", newLists); diff --git a/src/stores/notifications/RoomNotificationState.ts b/src/stores/notifications/RoomNotificationState.ts index f9b19fcbcb..51355a2d4d 100644 --- a/src/stores/notifications/RoomNotificationState.ts +++ b/src/stores/notifications/RoomNotificationState.ts @@ -31,7 +31,7 @@ export class RoomNotificationState extends EventEmitter implements IDestroyable, private _count: number; private _color: NotificationColor; - constructor(private room: Room) { + constructor(public readonly room: Room) { super(); this.room.on("Room.receipt", this.handleReadReceipt); this.room.on("Room.timeline", this.handleRoomEventUpdate); From 8acec1f417e632e21b7ddf1fd9bd2ac05b9c5dfb Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 17:58:39 +0100 Subject: [PATCH 24/72] delint Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/LoggedInView.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/structures/LoggedInView.tsx b/src/components/structures/LoggedInView.tsx index a5a1c628a3..6354bb8739 100644 --- a/src/components/structures/LoggedInView.tsx +++ b/src/components/structures/LoggedInView.tsx @@ -19,7 +19,6 @@ limitations under the License. import * as React from 'react'; import * as PropTypes from 'prop-types'; import { MatrixClient } from 'matrix-js-sdk/src/client'; -import { MatrixEvent } from 'matrix-js-sdk/src/models/event'; import { DragDropContext } from 'react-beautiful-dnd'; import {Key, isOnlyCtrlOrCmdKeyEvent, isOnlyCtrlOrCmdIgnoreShiftKeyEvent} from '../../Keyboard'; From 18064c19a382985b8d05b346254f3a1b8570fd83 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 18:13:11 +0100 Subject: [PATCH 25/72] add TODO Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomList2.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index 0eef2143dd..66c1ea8798 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -206,6 +206,8 @@ export default class RoomList2 extends React.Component<IProps, IState> { let listRooms = lists[t]; if (unread) { + // TODO Be smarter and not spin up a bunch of wasted listeners just to kill them 4 lines later + // https://github.com/vector-im/riot-web/issues/14035 const notificationStates = rooms.map(r => new TagSpecificNotificationState(r, t)); // filter to only notification rooms (and our current active room so we can index properly) listRooms = notificationStates.filter(state => { From 64237c9f4ecd44e1696e4d3a5cc3db51f9408ec2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 00:11:32 +0100 Subject: [PATCH 26/72] Apply scroll margins to RoomTile so that they don't scroll under the "sticky" headers Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- res/css/views/rooms/_RoomTile2.scss | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/res/css/views/rooms/_RoomTile2.scss b/res/css/views/rooms/_RoomTile2.scss index 7b606ab947..23cecff477 100644 --- a/res/css/views/rooms/_RoomTile2.scss +++ b/res/css/views/rooms/_RoomTile2.scss @@ -21,6 +21,10 @@ limitations under the License. margin-bottom: 4px; padding: 4px; + // allow scrollIntoView to ignore the sticky headers, must match combined height of .mx_RoomSublist2_headerContainer + scroll-margin-top: 32px; + scroll-margin-bottom: 32px; + // The tile is also a flexbox row itself display: flex; @@ -164,6 +168,11 @@ limitations under the License. } } +// do not apply scroll-margin-bottom to the sublist which will not have a sticky header below it +.mx_RoomSublist2:last-child .mx_RoomTile2 { + scroll-margin-bottom: 0; +} + // We use these both in context menus and the room tiles .mx_RoomTile2_iconBell::before { mask-image: url('$(res)/img/feather-customised/bell.svg'); From 0b6f744a58b218d5b3ca1cae573d01ecf92428ca Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Fri, 3 Jul 2020 15:36:04 -0600 Subject: [PATCH 27/72] Wrap handleRoomUpdate in a lock Dev note: this is removed in a later commit --- src/stores/room-list/algorithms/Algorithm.ts | 118 ++++++++++--------- 1 file changed, 63 insertions(+), 55 deletions(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 36abf86975..b6e2a7c1f6 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -33,6 +33,7 @@ import { FILTER_CHANGED, FilterPriority, IFilterCondition } from "../filters/IFi import { EffectiveMembership, getEffectiveMembership, splitRoomsByMembership } from "../membership"; import { OrderingAlgorithm } from "./list-ordering/OrderingAlgorithm"; import { getListAlgorithmInstance } from "./list-ordering"; +import AwaitLock from "await-lock"; // TODO: Add locking support to avoid concurrent writes? https://github.com/vector-im/riot-web/issues/14235 @@ -66,6 +67,7 @@ export class Algorithm extends EventEmitter { } = {}; private allowedByFilter: Map<IFilterCondition, Room[]> = new Map<IFilterCondition, Room[]>(); private allowedRoomsByFilters: Set<Room> = new Set<Room>(); + private readonly lock = new AwaitLock(); public constructor() { super(); @@ -607,67 +609,73 @@ export class Algorithm extends EventEmitter { * processing. */ public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean> { - if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); + try { + await this.lock.acquireAsync(); - if (cause === RoomUpdateCause.NewRoom) { - const roomTags = this.roomIdsToTags[room.roomId]; - if (roomTags && roomTags.length > 0) { - console.warn(`${room.roomId} is reportedly new but is already known - assuming TagChange instead`); - cause = RoomUpdateCause.PossibleTagChange; + if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); + + if (cause === RoomUpdateCause.NewRoom) { + const roomTags = this.roomIdsToTags[room.roomId]; + if (roomTags && roomTags.length > 0) { + console.warn(`${room.roomId} is reportedly new but is already known - assuming TagChange instead`); + cause = RoomUpdateCause.PossibleTagChange; + } } - } - if (cause === RoomUpdateCause.PossibleTagChange) { - // TODO: Be smarter and splice rather than regen the planet. https://github.com/vector-im/riot-web/issues/14035 - // TODO: No-op if no change. https://github.com/vector-im/riot-web/issues/14035 - await this.setKnownRooms(this.rooms); - return true; - } + if (cause === RoomUpdateCause.PossibleTagChange) { + // TODO: Be smarter and splice rather than regen the planet. https://github.com/vector-im/riot-web/issues/14035 + // TODO: No-op if no change. https://github.com/vector-im/riot-web/issues/14035 + await this.setKnownRooms(this.rooms); + return true; + } - // If the update is for a room change which might be the sticky room, prevent it. We - // need to make sure that the causes (NewRoom and RoomRemoved) are still triggered though - // as the sticky room relies on this. - if (cause !== RoomUpdateCause.NewRoom && cause !== RoomUpdateCause.RoomRemoved) { - if (this.stickyRoom === room) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.warn(`[RoomListDebug] Received ${cause} update for sticky room ${room.roomId} - ignoring`); + // If the update is for a room change which might be the sticky room, prevent it. We + // need to make sure that the causes (NewRoom and RoomRemoved) are still triggered though + // as the sticky room relies on this. + if (cause !== RoomUpdateCause.NewRoom && cause !== RoomUpdateCause.RoomRemoved) { + if (this.stickyRoom === room) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.warn(`[RoomListDebug] Received ${cause} update for sticky room ${room.roomId} - ignoring`); + return false; + } + } + + if (cause === RoomUpdateCause.NewRoom && !this.roomIdsToTags[room.roomId]) { + console.log(`[RoomListDebug] Updating tags for new room ${room.roomId} (${room.name})`); + + // Get the tags for the room and populate the cache + const roomTags = this.getTagsForRoom(room).filter(t => !isNullOrUndefined(this.cachedRooms[t])); + + // "This should never happen" condition - we specify DefaultTagID.Untagged in getTagsForRoom(), + // which means we should *always* have a tag to go off of. + if (!roomTags.length) throw new Error(`Tags cannot be determined for ${room.roomId}`); + + this.roomIdsToTags[room.roomId] = roomTags; + } + + let tags = this.roomIdsToTags[room.roomId]; + if (!tags) { + console.warn(`No tags known for "${room.name}" (${room.roomId})`); return false; } + + let changed = false; + for (const tag of tags) { + const algorithm: OrderingAlgorithm = this.algorithms[tag]; + if (!algorithm) throw new Error(`No algorithm for ${tag}`); + + await algorithm.handleRoomUpdate(room, cause); + this.cachedRooms[tag] = algorithm.orderedRooms; + + // Flag that we've done something + this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list + this.recalculateStickyRoom(tag); // update sticky room to make sure it appears if needed + changed = true; + } + + return true; + } finally { + this.lock.release(); } - - if (cause === RoomUpdateCause.NewRoom && !this.roomIdsToTags[room.roomId]) { - console.log(`[RoomListDebug] Updating tags for new room ${room.roomId} (${room.name})`); - - // Get the tags for the room and populate the cache - const roomTags = this.getTagsForRoom(room).filter(t => !isNullOrUndefined(this.cachedRooms[t])); - - // "This should never happen" condition - we specify DefaultTagID.Untagged in getTagsForRoom(), - // which means we should *always* have a tag to go off of. - if (!roomTags.length) throw new Error(`Tags cannot be determined for ${room.roomId}`); - - this.roomIdsToTags[room.roomId] = roomTags; - } - - let tags = this.roomIdsToTags[room.roomId]; - if (!tags) { - console.warn(`No tags known for "${room.name}" (${room.roomId})`); - return false; - } - - let changed = false; - for (const tag of tags) { - const algorithm: OrderingAlgorithm = this.algorithms[tag]; - if (!algorithm) throw new Error(`No algorithm for ${tag}`); - - await algorithm.handleRoomUpdate(room, cause); - this.cachedRooms[tag] = algorithm.orderedRooms; - - // Flag that we've done something - this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list - this.recalculateStickyRoom(tag); // update sticky room to make sure it appears if needed - changed = true; - } - - return true; } } From 18df29b62717620025bd1175206b9208228a1cf8 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Fri, 3 Jul 2020 15:36:17 -0600 Subject: [PATCH 28/72] Flag & add some debugging --- src/stores/room-list/algorithms/Algorithm.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index b6e2a7c1f6..254c75f055 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -641,11 +641,15 @@ export class Algorithm extends EventEmitter { } if (cause === RoomUpdateCause.NewRoom && !this.roomIdsToTags[room.roomId]) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[RoomListDebug] Updating tags for new room ${room.roomId} (${room.name})`); // Get the tags for the room and populate the cache const roomTags = this.getTagsForRoom(room).filter(t => !isNullOrUndefined(this.cachedRooms[t])); + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Updated tags for ${room.roomId}:`, roomTags); + // "This should never happen" condition - we specify DefaultTagID.Untagged in getTagsForRoom(), // which means we should *always* have a tag to go off of. if (!roomTags.length) throw new Error(`Tags cannot be determined for ${room.roomId}`); From 4345f972e0bbe19ccd23e9b6c2f753838b3869d5 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 17:49:18 -0600 Subject: [PATCH 29/72] Handle sticky room to avoid accidental removal Plus a bunch of logging. This fixes a case where switching rooms would cause the last room you were on to disappear due to an optimization where known NewRoom fires would be translated to tag change fires, which wouldn't re-add the room to the underlying tag algorithm. By tracking the last sticky room, we can identify when we're about to do this and avoid it. This commit also adds a check to ensure that we have the latest reference of a room stored as rooms changing from invite -> join change references. This commit additionally updates the PossibleTagChange handling to be faster and smarter, leading to a more stable generation of the room list. We convert the update cause to a Timeline update in order to indicate it is a change within the same tag rather than having to jump tags. This also means that PossibleTagChange should no longer make it as far as the underlying algorithm. New logging has also been added to aid debugging. --- src/stores/room-list/algorithms/Algorithm.ts | 87 ++++++++++++++++---- 1 file changed, 72 insertions(+), 15 deletions(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 254c75f055..51ab98fb0f 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -18,7 +18,7 @@ import { Room } from "matrix-js-sdk/src/models/room"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; import DMRoomMap from "../../../utils/DMRoomMap"; import { EventEmitter } from "events"; -import { arrayHasDiff, ArrayUtil } from "../../../utils/arrays"; +import { arrayDiff, arrayHasDiff, ArrayUtil } from "../../../utils/arrays"; import { getEnumValues } from "../../../utils/enums"; import { DefaultTagID, RoomUpdateCause, TagID } from "../models"; import { @@ -58,6 +58,7 @@ export class Algorithm extends EventEmitter { private _cachedStickyRooms: ITagMap = {}; // a clone of the _cachedRooms, with the sticky room private filteredRooms: ITagMap = {}; private _stickyRoom: IStickyRoom = null; + private _lastStickyRoom: IStickyRoom = null; private sortAlgorithms: ITagSortingMap; private listAlgorithms: IListOrderingMap; private algorithms: IOrderingAlgorithmMap; @@ -172,6 +173,7 @@ export class Algorithm extends EventEmitter { if (this._stickyRoom) { const stickyRoom = this._stickyRoom.room; this._stickyRoom = null; // clear before we go to update the algorithm + this._lastStickyRoom = stickyRoom; // Lie to the algorithm and re-add the room to the algorithm await this.handleRoomUpdate(stickyRoom, RoomUpdateCause.NewRoom); @@ -198,7 +200,8 @@ export class Algorithm extends EventEmitter { // the same thing it no-ops. After we're done calling the algorithm, we'll issue // a new update for ourselves. const lastStickyRoom = this._stickyRoom; - this._stickyRoom = null; + this._stickyRoom = null; // clear before we update the algorithm + this._lastStickyRoom = lastStickyRoom; this.recalculateStickyRoom(); // When we do have the room, re-add the old room (if needed) to the algorithm @@ -562,7 +565,7 @@ export class Algorithm extends EventEmitter { /** * Updates the roomsToTags map */ - protected updateTagsFromCache() { + private updateTagsFromCache() { const newMap = {}; const tags = Object.keys(this.cachedRooms); @@ -609,24 +612,73 @@ export class Algorithm extends EventEmitter { * processing. */ public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean> { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.trace(`Handle room update for ${room.roomId} called with cause ${cause}`); try { await this.lock.acquireAsync(); if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] @@ ${room.roomId} with ${cause} for tags ${this.roomIdsToTags[room.roomId]} and sticky is currently ${this._stickyRoom && this._stickyRoom.room ? this._stickyRoom.room.roomId : '<none>'} and last is ${this._lastStickyRoom && this._lastStickyRoom.room ? this._lastStickyRoom.room.roomId : '<none>'}`); + if (cause === RoomUpdateCause.NewRoom) { + const isForLastSticky = this._lastStickyRoom && this._lastStickyRoom.room === room; const roomTags = this.roomIdsToTags[room.roomId]; - if (roomTags && roomTags.length > 0) { + const hasTags = roomTags && roomTags.length > 0; + + // Don't change the cause if the last sticky room is being re-added. If we fail to + // pass the cause through as NewRoom, we'll fail to lie to the algorithm and thus + // lose the room. + if (hasTags && !isForLastSticky) { console.warn(`${room.roomId} is reportedly new but is already known - assuming TagChange instead`); cause = RoomUpdateCause.PossibleTagChange; } + + // If we have tags for a room and don't have the room referenced, the room reference + // probably changed. We need to swap out the problematic reference. + if (hasTags && !this.rooms.includes(room)) { + console.warn(`${room.roomId} is missing from room array but is known - trying to find duplicate`); + this.rooms = this.rooms.map(r => r.roomId === room.roomId ? room : r); + + // Sanity check + if (!this.rooms.includes(room)) { + throw new Error(`Failed to replace ${room.roomId} with an updated reference`); + } + } } if (cause === RoomUpdateCause.PossibleTagChange) { - // TODO: Be smarter and splice rather than regen the planet. https://github.com/vector-im/riot-web/issues/14035 - // TODO: No-op if no change. https://github.com/vector-im/riot-web/issues/14035 - await this.setKnownRooms(this.rooms); - return true; + const oldTags = this.roomIdsToTags[room.roomId] || []; + const newTags = this.getTagsForRoom(room); + const diff = arrayDiff(oldTags, newTags); + if (diff.removed.length > 0 || diff.added.length > 0) { + for (const rmTag of diff.removed) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Removing ${room.roomId} from ${rmTag}`); + const algorithm: OrderingAlgorithm = this.algorithms[rmTag]; + if (!algorithm) throw new Error(`No algorithm for ${rmTag}`); + await algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved); + } + for (const addTag of diff.added) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Adding ${room.roomId} to ${addTag}`); + const algorithm: OrderingAlgorithm = this.algorithms[addTag]; + if (!algorithm) throw new Error(`No algorithm for ${addTag}`); + await algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom); + } + + // Update the tag map so we don't regen it in a moment + this.roomIdsToTags[room.roomId] = newTags; + + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Changing update cause for ${room.roomId} to TIMELINE to sort rooms`); + cause = RoomUpdateCause.Timeline; + } else { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.warn(`Received no-op update for ${room.roomId} - changing to TIMELINE update`); + cause = RoomUpdateCause.Timeline; + } } // If the update is for a room change which might be the sticky room, prevent it. We @@ -640,24 +692,27 @@ export class Algorithm extends EventEmitter { } } - if (cause === RoomUpdateCause.NewRoom && !this.roomIdsToTags[room.roomId]) { + if (!this.roomIdsToTags[room.roomId]) { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Updating tags for new room ${room.roomId} (${room.name})`); + console.log(`[RoomListDebug] Updating tags for room ${room.roomId} (${room.name})`); // Get the tags for the room and populate the cache const roomTags = this.getTagsForRoom(room).filter(t => !isNullOrUndefined(this.cachedRooms[t])); - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Updated tags for ${room.roomId}:`, roomTags); - // "This should never happen" condition - we specify DefaultTagID.Untagged in getTagsForRoom(), // which means we should *always* have a tag to go off of. if (!roomTags.length) throw new Error(`Tags cannot be determined for ${room.roomId}`); this.roomIdsToTags[room.roomId] = roomTags; + + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Updated tags for ${room.roomId}:`, roomTags); } - let tags = this.roomIdsToTags[room.roomId]; + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Reached algorithmic handling for ${room.roomId} and cause ${cause}`); + + const tags = this.roomIdsToTags[room.roomId]; if (!tags) { console.warn(`No tags known for "${room.name}" (${room.roomId})`); return false; @@ -677,7 +732,9 @@ export class Algorithm extends EventEmitter { changed = true; } - return true; + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Finished handling ${room.roomId} with cause ${cause} (changed=${changed})`); + return changed; } finally { this.lock.release(); } From dd833f4f2ffd78bfe4199a80ac1294d86536f45d Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 19:32:31 -0600 Subject: [PATCH 30/72] Ensure the sticky room changes if the tag changes This fixes a case where a user accepts an invite, which causes a tag change, but the room stays stuck in the invites list. The tag change additionally gets swallowed when the user moves away, causing the room to get lost. By moving it when we see it, potentially during a sticky room change itself (though extremely rare), we avoid having the room get lost in the wrong lists. A side effect of this is that accepting an invite puts it at the top of the tag it's going to (usually untagged), however this feels like the best option for the user. A rare case of a tag change happening during a sticky room change is when a leave event comes in for the sticky room, but because it's come through as a tag change it can get swallowed. If it does get swallowed and the user clicks away, the tag change will happen when the room is re-introduced to the list (fake NewRoom event). --- src/stores/room-list/algorithms/Algorithm.ts | 65 ++++++++++++++++++-- 1 file changed, 60 insertions(+), 5 deletions(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 51ab98fb0f..4e06f93359 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -58,7 +58,7 @@ export class Algorithm extends EventEmitter { private _cachedStickyRooms: ITagMap = {}; // a clone of the _cachedRooms, with the sticky room private filteredRooms: ITagMap = {}; private _stickyRoom: IStickyRoom = null; - private _lastStickyRoom: IStickyRoom = null; + private _lastStickyRoom: IStickyRoom = null; // only not-null when changing the sticky room private sortAlgorithms: ITagSortingMap; private listAlgorithms: IListOrderingMap; private algorithms: IOrderingAlgorithmMap; @@ -165,15 +165,26 @@ export class Algorithm extends EventEmitter { } private async updateStickyRoom(val: Room) { + try { + return await this.doUpdateStickyRoom(val); + } finally { + this._lastStickyRoom = null; // clear to indicate we're done changing + } + } + + private async doUpdateStickyRoom(val: Room) { // Note throughout: We need async so we can wait for handleRoomUpdate() to do its thing, // otherwise we risk duplicating rooms. + // Set the last sticky room to indicate that we're in a change. The code throughout the + // class can safely handle a null room, so this should be safe to do as a backup. + this._lastStickyRoom = this._stickyRoom || <IStickyRoom>{}; + // It's possible to have no selected room. In that case, clear the sticky room if (!val) { if (this._stickyRoom) { const stickyRoom = this._stickyRoom.room; this._stickyRoom = null; // clear before we go to update the algorithm - this._lastStickyRoom = stickyRoom; // Lie to the algorithm and re-add the room to the algorithm await this.handleRoomUpdate(stickyRoom, RoomUpdateCause.NewRoom); @@ -183,7 +194,7 @@ export class Algorithm extends EventEmitter { } // When we do have a room though, we expect to be able to find it - const tag = this.roomIdsToTags[val.roomId][0]; + let tag = this.roomIdsToTags[val.roomId][0]; if (!tag) throw new Error(`${val.roomId} does not belong to a tag and cannot be sticky`); // We specifically do NOT use the ordered rooms set as it contains the sticky room, which @@ -201,7 +212,6 @@ export class Algorithm extends EventEmitter { // a new update for ourselves. const lastStickyRoom = this._stickyRoom; this._stickyRoom = null; // clear before we update the algorithm - this._lastStickyRoom = lastStickyRoom; this.recalculateStickyRoom(); // When we do have the room, re-add the old room (if needed) to the algorithm @@ -214,6 +224,22 @@ export class Algorithm extends EventEmitter { // Lie to the algorithm and remove the room from it's field of view await this.handleRoomUpdate(val, RoomUpdateCause.RoomRemoved); + // Check for tag & position changes while we're here. We also check the room to ensure + // it is still the same room. + if (this._stickyRoom) { + if (this._stickyRoom.room !== val) { + // Check the room IDs just in case + if (this._stickyRoom.room.roomId === val.roomId) { + console.warn("Sticky room changed references"); + } else { + throw new Error("Sticky room changed while the sticky room was changing"); + } + } + + tag = this._stickyRoom.tag; + position = this._stickyRoom.position; + } + // Now that we're done lying to the algorithm, we need to update our position // marker only if the user is moving further down the same list. If they're switching // lists, or moving upwards, the position marker will splice in just fine but if @@ -619,6 +645,8 @@ export class Algorithm extends EventEmitter { if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); + const isSticky = this._stickyRoom && this._stickyRoom.room === room; + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[RoomListDebug] @@ ${room.roomId} with ${cause} for tags ${this.roomIdsToTags[room.roomId]} and sticky is currently ${this._stickyRoom && this._stickyRoom.room ? this._stickyRoom.room.roomId : '<none>'} and last is ${this._lastStickyRoom && this._lastStickyRoom.room ? this._lastStickyRoom.room.roomId : '<none>'}`); @@ -637,7 +665,7 @@ export class Algorithm extends EventEmitter { // If we have tags for a room and don't have the room referenced, the room reference // probably changed. We need to swap out the problematic reference. - if (hasTags && !this.rooms.includes(room)) { + if (hasTags && !this.rooms.includes(room) && !isSticky) { console.warn(`${room.roomId} is missing from room array but is known - trying to find duplicate`); this.rooms = this.rooms.map(r => r.roomId === room.roomId ? room : r); @@ -646,9 +674,17 @@ export class Algorithm extends EventEmitter { throw new Error(`Failed to replace ${room.roomId} with an updated reference`); } } + + // Like above, update the reference to the sticky room if we need to + if (hasTags && isSticky) { + // Go directly in and set the sticky room's new reference, being careful not + // to trigger a sticky room update ourselves. + this._stickyRoom.room = room; + } } if (cause === RoomUpdateCause.PossibleTagChange) { + let didTagChange = false; const oldTags = this.roomIdsToTags[room.roomId] || []; const newTags = this.getTagsForRoom(room); const diff = arrayDiff(oldTags, newTags); @@ -674,11 +710,30 @@ export class Algorithm extends EventEmitter { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`Changing update cause for ${room.roomId} to TIMELINE to sort rooms`); cause = RoomUpdateCause.Timeline; + didTagChange = true; } else { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.warn(`Received no-op update for ${room.roomId} - changing to TIMELINE update`); cause = RoomUpdateCause.Timeline; } + + if (didTagChange && isSticky) { + // Manually update the tag for the sticky room without triggering a sticky room + // update. The update will be handled implicitly by the sticky room handling and + // requires no changes on our part, if we're in the middle of a sticky room change. + if (this._lastStickyRoom) { + this._stickyRoom = { + room, + tag: this.roomIdsToTags[room.roomId][0], + position: 0, // right at the top as it changed tags + }; + } else { + // We have to clear the lock as the sticky room change will trigger updates. + this.lock.release(); + await this.setStickyRoomAsync(room); + await this.lock.acquireAsync(); + } + } } // If the update is for a room change which might be the sticky room, prevent it. We From 70e5da677b70171a62052079ce9d5b0918ab38d3 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 19:35:58 -0600 Subject: [PATCH 31/72] Clean up debug logging --- src/stores/room-list/algorithms/Algorithm.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 4e06f93359..930759e68c 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -236,6 +236,9 @@ export class Algorithm extends EventEmitter { } } + console.warn(`Sticky room changed tag & position from ${tag} / ${position} ` + + `to ${this._stickyRoom.tag} / ${this._stickyRoom.position}`); + tag = this._stickyRoom.tag; position = this._stickyRoom.position; } @@ -639,17 +642,13 @@ export class Algorithm extends EventEmitter { */ public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean> { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.trace(`Handle room update for ${room.roomId} called with cause ${cause}`); + console.log(`Handle room update for ${room.roomId} called with cause ${cause}`); try { await this.lock.acquireAsync(); if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); const isSticky = this._stickyRoom && this._stickyRoom.room === room; - - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] @@ ${room.roomId} with ${cause} for tags ${this.roomIdsToTags[room.roomId]} and sticky is currently ${this._stickyRoom && this._stickyRoom.room ? this._stickyRoom.room.roomId : '<none>'} and last is ${this._lastStickyRoom && this._lastStickyRoom.room ? this._lastStickyRoom.room.roomId : '<none>'}`); - if (cause === RoomUpdateCause.NewRoom) { const isForLastSticky = this._lastStickyRoom && this._lastStickyRoom.room === room; const roomTags = this.roomIdsToTags[room.roomId]; @@ -708,12 +707,12 @@ export class Algorithm extends EventEmitter { this.roomIdsToTags[room.roomId] = newTags; // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`Changing update cause for ${room.roomId} to TIMELINE to sort rooms`); + console.log(`Changing update cause for ${room.roomId} to Timeline to sort rooms`); cause = RoomUpdateCause.Timeline; didTagChange = true; } else { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.warn(`Received no-op update for ${room.roomId} - changing to TIMELINE update`); + console.warn(`Received no-op update for ${room.roomId} - changing to Timeline update`); cause = RoomUpdateCause.Timeline; } From 34bd59c1519a57fa0921330db17d049a81c7ed46 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 19:37:57 -0600 Subject: [PATCH 32/72] Remove the lock around the algorithm This isn't needed --- src/stores/room-list/algorithms/Algorithm.ts | 248 +++++++++---------- 1 file changed, 119 insertions(+), 129 deletions(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 930759e68c..d85ff4e2e1 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -33,7 +33,6 @@ import { FILTER_CHANGED, FilterPriority, IFilterCondition } from "../filters/IFi import { EffectiveMembership, getEffectiveMembership, splitRoomsByMembership } from "../membership"; import { OrderingAlgorithm } from "./list-ordering/OrderingAlgorithm"; import { getListAlgorithmInstance } from "./list-ordering"; -import AwaitLock from "await-lock"; // TODO: Add locking support to avoid concurrent writes? https://github.com/vector-im/riot-web/issues/14235 @@ -68,7 +67,6 @@ export class Algorithm extends EventEmitter { } = {}; private allowedByFilter: Map<IFilterCondition, Room[]> = new Map<IFilterCondition, Room[]>(); private allowedRoomsByFilters: Set<Room> = new Set<Room>(); - private readonly lock = new AwaitLock(); public constructor() { super(); @@ -643,154 +641,146 @@ export class Algorithm extends EventEmitter { public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean> { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`Handle room update for ${room.roomId} called with cause ${cause}`); - try { - await this.lock.acquireAsync(); + if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); - if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); + const isSticky = this._stickyRoom && this._stickyRoom.room === room; + if (cause === RoomUpdateCause.NewRoom) { + const isForLastSticky = this._lastStickyRoom && this._lastStickyRoom.room === room; + const roomTags = this.roomIdsToTags[room.roomId]; + const hasTags = roomTags && roomTags.length > 0; - const isSticky = this._stickyRoom && this._stickyRoom.room === room; - if (cause === RoomUpdateCause.NewRoom) { - const isForLastSticky = this._lastStickyRoom && this._lastStickyRoom.room === room; - const roomTags = this.roomIdsToTags[room.roomId]; - const hasTags = roomTags && roomTags.length > 0; + // Don't change the cause if the last sticky room is being re-added. If we fail to + // pass the cause through as NewRoom, we'll fail to lie to the algorithm and thus + // lose the room. + if (hasTags && !isForLastSticky) { + console.warn(`${room.roomId} is reportedly new but is already known - assuming TagChange instead`); + cause = RoomUpdateCause.PossibleTagChange; + } - // Don't change the cause if the last sticky room is being re-added. If we fail to - // pass the cause through as NewRoom, we'll fail to lie to the algorithm and thus - // lose the room. - if (hasTags && !isForLastSticky) { - console.warn(`${room.roomId} is reportedly new but is already known - assuming TagChange instead`); - cause = RoomUpdateCause.PossibleTagChange; - } + // If we have tags for a room and don't have the room referenced, the room reference + // probably changed. We need to swap out the problematic reference. + if (hasTags && !this.rooms.includes(room) && !isSticky) { + console.warn(`${room.roomId} is missing from room array but is known - trying to find duplicate`); + this.rooms = this.rooms.map(r => r.roomId === room.roomId ? room : r); - // If we have tags for a room and don't have the room referenced, the room reference - // probably changed. We need to swap out the problematic reference. - if (hasTags && !this.rooms.includes(room) && !isSticky) { - console.warn(`${room.roomId} is missing from room array but is known - trying to find duplicate`); - this.rooms = this.rooms.map(r => r.roomId === room.roomId ? room : r); - - // Sanity check - if (!this.rooms.includes(room)) { - throw new Error(`Failed to replace ${room.roomId} with an updated reference`); - } - } - - // Like above, update the reference to the sticky room if we need to - if (hasTags && isSticky) { - // Go directly in and set the sticky room's new reference, being careful not - // to trigger a sticky room update ourselves. - this._stickyRoom.room = room; + // Sanity check + if (!this.rooms.includes(room)) { + throw new Error(`Failed to replace ${room.roomId} with an updated reference`); } } - if (cause === RoomUpdateCause.PossibleTagChange) { - let didTagChange = false; - const oldTags = this.roomIdsToTags[room.roomId] || []; - const newTags = this.getTagsForRoom(room); - const diff = arrayDiff(oldTags, newTags); - if (diff.removed.length > 0 || diff.added.length > 0) { - for (const rmTag of diff.removed) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`Removing ${room.roomId} from ${rmTag}`); - const algorithm: OrderingAlgorithm = this.algorithms[rmTag]; - if (!algorithm) throw new Error(`No algorithm for ${rmTag}`); - await algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved); - } - for (const addTag of diff.added) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`Adding ${room.roomId} to ${addTag}`); - const algorithm: OrderingAlgorithm = this.algorithms[addTag]; - if (!algorithm) throw new Error(`No algorithm for ${addTag}`); - await algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom); - } - - // Update the tag map so we don't regen it in a moment - this.roomIdsToTags[room.roomId] = newTags; + // Like above, update the reference to the sticky room if we need to + if (hasTags && isSticky) { + // Go directly in and set the sticky room's new reference, being careful not + // to trigger a sticky room update ourselves. + this._stickyRoom.room = room; + } + } + if (cause === RoomUpdateCause.PossibleTagChange) { + let didTagChange = false; + const oldTags = this.roomIdsToTags[room.roomId] || []; + const newTags = this.getTagsForRoom(room); + const diff = arrayDiff(oldTags, newTags); + if (diff.removed.length > 0 || diff.added.length > 0) { + for (const rmTag of diff.removed) { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`Changing update cause for ${room.roomId} to Timeline to sort rooms`); - cause = RoomUpdateCause.Timeline; - didTagChange = true; + console.log(`Removing ${room.roomId} from ${rmTag}`); + const algorithm: OrderingAlgorithm = this.algorithms[rmTag]; + if (!algorithm) throw new Error(`No algorithm for ${rmTag}`); + await algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved); + } + for (const addTag of diff.added) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Adding ${room.roomId} to ${addTag}`); + const algorithm: OrderingAlgorithm = this.algorithms[addTag]; + if (!algorithm) throw new Error(`No algorithm for ${addTag}`); + await algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom); + } + + // Update the tag map so we don't regen it in a moment + this.roomIdsToTags[room.roomId] = newTags; + + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Changing update cause for ${room.roomId} to Timeline to sort rooms`); + cause = RoomUpdateCause.Timeline; + didTagChange = true; + } else { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.warn(`Received no-op update for ${room.roomId} - changing to Timeline update`); + cause = RoomUpdateCause.Timeline; + } + + if (didTagChange && isSticky) { + // Manually update the tag for the sticky room without triggering a sticky room + // update. The update will be handled implicitly by the sticky room handling and + // requires no changes on our part, if we're in the middle of a sticky room change. + if (this._lastStickyRoom) { + this._stickyRoom = { + room, + tag: this.roomIdsToTags[room.roomId][0], + position: 0, // right at the top as it changed tags + }; } else { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.warn(`Received no-op update for ${room.roomId} - changing to Timeline update`); - cause = RoomUpdateCause.Timeline; - } - - if (didTagChange && isSticky) { - // Manually update the tag for the sticky room without triggering a sticky room - // update. The update will be handled implicitly by the sticky room handling and - // requires no changes on our part, if we're in the middle of a sticky room change. - if (this._lastStickyRoom) { - this._stickyRoom = { - room, - tag: this.roomIdsToTags[room.roomId][0], - position: 0, // right at the top as it changed tags - }; - } else { - // We have to clear the lock as the sticky room change will trigger updates. - this.lock.release(); - await this.setStickyRoomAsync(room); - await this.lock.acquireAsync(); - } + // We have to clear the lock as the sticky room change will trigger updates. + await this.setStickyRoomAsync(room); } } + } - // If the update is for a room change which might be the sticky room, prevent it. We - // need to make sure that the causes (NewRoom and RoomRemoved) are still triggered though - // as the sticky room relies on this. - if (cause !== RoomUpdateCause.NewRoom && cause !== RoomUpdateCause.RoomRemoved) { - if (this.stickyRoom === room) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.warn(`[RoomListDebug] Received ${cause} update for sticky room ${room.roomId} - ignoring`); - return false; - } - } - - if (!this.roomIdsToTags[room.roomId]) { + // If the update is for a room change which might be the sticky room, prevent it. We + // need to make sure that the causes (NewRoom and RoomRemoved) are still triggered though + // as the sticky room relies on this. + if (cause !== RoomUpdateCause.NewRoom && cause !== RoomUpdateCause.RoomRemoved) { + if (this.stickyRoom === room) { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Updating tags for room ${room.roomId} (${room.name})`); - - // Get the tags for the room and populate the cache - const roomTags = this.getTagsForRoom(room).filter(t => !isNullOrUndefined(this.cachedRooms[t])); - - // "This should never happen" condition - we specify DefaultTagID.Untagged in getTagsForRoom(), - // which means we should *always* have a tag to go off of. - if (!roomTags.length) throw new Error(`Tags cannot be determined for ${room.roomId}`); - - this.roomIdsToTags[room.roomId] = roomTags; - - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Updated tags for ${room.roomId}:`, roomTags); - } - - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Reached algorithmic handling for ${room.roomId} and cause ${cause}`); - - const tags = this.roomIdsToTags[room.roomId]; - if (!tags) { - console.warn(`No tags known for "${room.name}" (${room.roomId})`); + console.warn(`[RoomListDebug] Received ${cause} update for sticky room ${room.roomId} - ignoring`); return false; } + } - let changed = false; - for (const tag of tags) { - const algorithm: OrderingAlgorithm = this.algorithms[tag]; - if (!algorithm) throw new Error(`No algorithm for ${tag}`); + if (!this.roomIdsToTags[room.roomId]) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Updating tags for room ${room.roomId} (${room.name})`); - await algorithm.handleRoomUpdate(room, cause); - this.cachedRooms[tag] = algorithm.orderedRooms; + // Get the tags for the room and populate the cache + const roomTags = this.getTagsForRoom(room).filter(t => !isNullOrUndefined(this.cachedRooms[t])); - // Flag that we've done something - this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list - this.recalculateStickyRoom(tag); // update sticky room to make sure it appears if needed - changed = true; - } + // "This should never happen" condition - we specify DefaultTagID.Untagged in getTagsForRoom(), + // which means we should *always* have a tag to go off of. + if (!roomTags.length) throw new Error(`Tags cannot be determined for ${room.roomId}`); + + this.roomIdsToTags[room.roomId] = roomTags; // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Finished handling ${room.roomId} with cause ${cause} (changed=${changed})`); - return changed; - } finally { - this.lock.release(); + console.log(`[RoomListDebug] Updated tags for ${room.roomId}:`, roomTags); } + + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Reached algorithmic handling for ${room.roomId} and cause ${cause}`); + + const tags = this.roomIdsToTags[room.roomId]; + if (!tags) { + console.warn(`No tags known for "${room.name}" (${room.roomId})`); + return false; + } + + let changed = false; + for (const tag of tags) { + const algorithm: OrderingAlgorithm = this.algorithms[tag]; + if (!algorithm) throw new Error(`No algorithm for ${tag}`); + + await algorithm.handleRoomUpdate(room, cause); + this.cachedRooms[tag] = algorithm.orderedRooms; + + // Flag that we've done something + this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list + this.recalculateStickyRoom(tag); // update sticky room to make sure it appears if needed + changed = true; + } + + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Finished handling ${room.roomId} with cause ${cause} (changed=${changed})`); + return changed; } } From 8739e2f78120a83674ec9129b3003d9b3d2997d1 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 20:12:25 -0600 Subject: [PATCH 33/72] Fix room duplication when the sticky room reference changes --- src/stores/room-list/algorithms/Algorithm.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index d85ff4e2e1..d5f2ed0053 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -215,7 +215,10 @@ export class Algorithm extends EventEmitter { // When we do have the room, re-add the old room (if needed) to the algorithm // and remove the sticky room from the algorithm. This is so the underlying // algorithm doesn't try and confuse itself with the sticky room concept. - if (lastStickyRoom) { + // We don't add the new room if the sticky room isn't changing because that's + // an easy way to cause duplication. We have to do room ID checks instead of + // referential checks as the references can differ through the lifecycle. + if (lastStickyRoom && lastStickyRoom.room && lastStickyRoom.room.roomId !== val.roomId) { // Lie to the algorithm and re-add the room to the algorithm await this.handleRoomUpdate(lastStickyRoom.room, RoomUpdateCause.NewRoom); } @@ -643,7 +646,8 @@ export class Algorithm extends EventEmitter { console.log(`Handle room update for ${room.roomId} called with cause ${cause}`); if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); - const isSticky = this._stickyRoom && this._stickyRoom.room === room; + // Note: check the isSticky against the room ID just in case the reference is wrong + const isSticky = this._stickyRoom && this._stickyRoom.room && this._stickyRoom.room.roomId === room.roomId; if (cause === RoomUpdateCause.NewRoom) { const isForLastSticky = this._lastStickyRoom && this._lastStickyRoom.room === room; const roomTags = this.roomIdsToTags[room.roomId]; From b28a267669f1aeeab4a3c99d370bf832bfdeb307 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 20:32:09 -0600 Subject: [PATCH 34/72] Remove old community invite placeholder handling We ended up shoving it into the invite list, so don't render it here. --- src/components/views/rooms/RoomList2.tsx | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index 3933b66c92..b0284bac66 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -64,8 +64,6 @@ interface IState { } const TAG_ORDER: TagID[] = [ - // -- Community Invites Placeholder -- - DefaultTagID.Invite, DefaultTagID.Favourite, DefaultTagID.DM, @@ -77,7 +75,6 @@ const TAG_ORDER: TagID[] = [ DefaultTagID.ServerNotice, DefaultTagID.Archived, ]; -const COMMUNITY_TAGS_BEFORE_TAG = DefaultTagID.Invite; const CUSTOM_TAGS_BEFORE_TAG = DefaultTagID.LowPriority; const ALWAYS_VISIBLE_TAGS: TagID[] = [ DefaultTagID.DM, @@ -227,10 +224,6 @@ export default class RoomList2 extends React.Component<IProps, IState> { const components: React.ReactElement[] = []; for (const orderedTagId of TAG_ORDER) { - if (COMMUNITY_TAGS_BEFORE_TAG === orderedTagId) { - // Populate community invites if we have the chance - // TODO: Community invites: https://github.com/vector-im/riot-web/issues/14179 - } if (CUSTOM_TAGS_BEFORE_TAG === orderedTagId) { // Populate custom tags if needed // TODO: Custom tags: https://github.com/vector-im/riot-web/issues/14091 From f103fd1ccfd4ff1f041eac7a07d2207356e9dff6 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 20:32:37 -0600 Subject: [PATCH 35/72] Make community invites appear even if there's no room invites Fixes https://github.com/vector-im/riot-web/issues/14358 --- src/components/views/rooms/RoomList2.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index b0284bac66..91bef0fc3d 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -230,7 +230,9 @@ export default class RoomList2 extends React.Component<IProps, IState> { } const orderedRooms = this.state.sublists[orderedTagId] || []; - if (orderedRooms.length === 0 && !ALWAYS_VISIBLE_TAGS.includes(orderedTagId)) { + const extraTiles = orderedTagId === DefaultTagID.Invite ? this.renderCommunityInvites() : null; + const totalTiles = orderedRooms.length + (extraTiles ? extraTiles.length : 0); + if (totalTiles === 0 && !ALWAYS_VISIBLE_TAGS.includes(orderedTagId)) { continue; // skip tag - not needed } @@ -238,7 +240,6 @@ export default class RoomList2 extends React.Component<IProps, IState> { if (!aesthetics) throw new Error(`Tag ${orderedTagId} does not have aesthetics`); const onAddRoomFn = aesthetics.onAddRoom ? () => aesthetics.onAddRoom(dis) : null; - const extraTiles = orderedTagId === DefaultTagID.Invite ? this.renderCommunityInvites() : null; components.push( <RoomSublist2 key={`sublist-${orderedTagId}`} From 3284cc730eea51253c0f32c23ee6ea632328e74a Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 20:37:04 -0600 Subject: [PATCH 36/72] Show ordering options on invites Fixes https://github.com/vector-im/riot-web/issues/14309 --- src/components/views/rooms/RoomSublist2.tsx | 61 ++++++++++++--------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 2ca9ec5dd1..f5a1b67571 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -27,7 +27,6 @@ import RoomTile2 from "./RoomTile2"; import { ResizableBox, ResizeCallbackData } from "react-resizable"; import { ListLayout } from "../../../stores/room-list/ListLayout"; import { ContextMenu, ContextMenuButton } from "../../structures/ContextMenu"; -import StyledCheckbox from "../elements/StyledCheckbox"; import StyledRadioButton from "../elements/StyledRadioButton"; import RoomListStore from "../../../stores/room-list/RoomListStore2"; import { ListAlgorithm, SortAlgorithm } from "../../../stores/room-list/algorithms/models"; @@ -35,9 +34,9 @@ import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import dis from "../../../dispatcher/dispatcher"; import NotificationBadge from "./NotificationBadge"; import { ListNotificationState } from "../../../stores/notifications/ListNotificationState"; -import Tooltip from "../elements/Tooltip"; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; import { Key } from "../../../Keyboard"; +import StyledCheckbox from "../elements/StyledCheckbox"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -311,15 +310,42 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { } private renderMenu(): React.ReactElement { - // TODO: Get a proper invite context menu, or take invites out of the room list. - if (this.props.tagId === DefaultTagID.Invite) { - return null; - } - let contextMenu = null; if (this.state.contextMenuPosition) { const isAlphabetical = RoomListStore.instance.getTagSorting(this.props.tagId) === SortAlgorithm.Alphabetic; const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance; + + // Invites don't get some nonsense options, so only add them if we have to. We do + // this with an array instead of a containing div to ensure that the DOM structure + // is relatively sane. + let otherSections = []; + if (this.props.tagId !== DefaultTagID.Invite) { + otherSections.push(<hr key={otherSections.length} />); + otherSections.push( + <div key={otherSections.length}> + <div className='mx_RoomSublist2_contextMenu_title'>{_t("Unread rooms")}</div> + <StyledCheckbox + onChange={this.onUnreadFirstChanged} + checked={isUnreadFirst} + > + {_t("Always show first")} + </StyledCheckbox> + </div>, + ); + otherSections.push(<hr key={otherSections.length} />); + otherSections.push( + <div key={otherSections.length}> + <div className='mx_RoomSublist2_contextMenu_title'>{_t("Show")}</div> + <StyledCheckbox + onChange={this.onMessagePreviewChanged} + checked={this.props.layout.showPreviews} + > + {_t("Message preview")} + </StyledCheckbox> + </div>, + ); + } + contextMenu = ( <ContextMenu chevronFace="none" @@ -345,26 +371,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { {_t("A-Z")} </StyledRadioButton> </div> - <hr /> - <div> - <div className='mx_RoomSublist2_contextMenu_title'>{_t("Unread rooms")}</div> - <StyledCheckbox - onChange={this.onUnreadFirstChanged} - checked={isUnreadFirst} - > - {_t("Always show first")} - </StyledCheckbox> - </div> - <hr /> - <div> - <div className='mx_RoomSublist2_contextMenu_title'>{_t("Show")}</div> - <StyledCheckbox - onChange={this.onMessagePreviewChanged} - checked={this.props.layout.showPreviews} - > - {_t("Message preview")} - </StyledCheckbox> - </div> + {otherSections} </div> </ContextMenu> ); From 56333ae017583d149fc00fb60d4b85b62740339d Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 20:42:25 -0600 Subject: [PATCH 37/72] Ensure the recents algorithm is aware of invites --- .../algorithms/tag-sorting/RecentAlgorithm.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts index a122ee3ae6..e7ca94ed95 100644 --- a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts +++ b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts @@ -19,6 +19,7 @@ import { TagID } from "../../models"; import { IAlgorithm } from "./IAlgorithm"; import { MatrixClientPeg } from "../../../../MatrixClientPeg"; import * as Unread from "../../../../Unread"; +import { EffectiveMembership, getEffectiveMembership } from "../../membership"; /** * Sorts rooms according to the last event's timestamp in each room that seems @@ -37,6 +38,8 @@ export class RecentAlgorithm implements IAlgorithm { // actually changed (probably needs to be done higher up?) then we could do an // insertion sort or similar on the limited set of changes. + const myUserId = MatrixClientPeg.get().getUserId(); + const tsCache: { [roomId: string]: number } = {}; const getLastTs = (r: Room) => { if (tsCache[r.roomId]) { @@ -50,13 +53,23 @@ export class RecentAlgorithm implements IAlgorithm { return Number.MAX_SAFE_INTEGER; } + // If the room hasn't been joined yet, it probably won't have a timeline to + // parse. We'll still fall back to the timeline if this fails, but chances + // are we'll at least have our own membership event to go off of. + const effectiveMembership = getEffectiveMembership(r.getMyMembership()); + if (effectiveMembership !== EffectiveMembership.Join) { + const membershipEvent = r.currentState.getStateEvents("m.room.member", myUserId); + if (membershipEvent && !Array.isArray(membershipEvent)) { + return membershipEvent.getTs(); + } + } + for (let i = r.timeline.length - 1; i >= 0; --i) { const ev = r.timeline[i]; if (!ev.getTs()) continue; // skip events that don't have timestamps (tests only?) // TODO: Don't assume we're using the same client as the peg - if (ev.getSender() === MatrixClientPeg.get().getUserId() - || Unread.eventTriggersUnreadCount(ev)) { + if (ev.getSender() === myUserId || Unread.eventTriggersUnreadCount(ev)) { return ev.getTs(); } } From 29aeea2974d46940cda34e597b1824837deb3201 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 20:45:36 -0600 Subject: [PATCH 38/72] Fix i18n --- src/i18n/strings/en_EN.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index b23264a297..b551805184 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1200,13 +1200,13 @@ "Securely back up your keys to avoid losing them. <a>Learn more.</a>": "Securely back up your keys to avoid losing them. <a>Learn more.</a>", "Not now": "Not now", "Don't ask me again": "Don't ask me again", - "Sort by": "Sort by", - "Activity": "Activity", - "A-Z": "A-Z", "Unread rooms": "Unread rooms", "Always show first": "Always show first", "Show": "Show", "Message preview": "Message preview", + "Sort by": "Sort by", + "Activity": "Activity", + "A-Z": "A-Z", "List options": "List options", "Add room": "Add room", "Show %(count)s more|other": "Show %(count)s more", From 2c502ed2fe1af1b91887dd843da51055b43944f9 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Mon, 6 Jul 2020 20:48:49 -0600 Subject: [PATCH 39/72] Decrease default visible rooms down to 5 --- src/stores/room-list/ListLayout.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stores/room-list/ListLayout.ts b/src/stores/room-list/ListLayout.ts index efb0c4bdfb..f31e92b8ae 100644 --- a/src/stores/room-list/ListLayout.ts +++ b/src/stores/room-list/ListLayout.ts @@ -85,8 +85,8 @@ export class ListLayout { } public get defaultVisibleTiles(): number { - // 10 is what "feels right", and mostly subject to design's opinion. - return 10 + RESIZER_BOX_FACTOR; + // This number is what "feels right", and mostly subject to design's opinion. + return 5 + RESIZER_BOX_FACTOR; } public setVisibleTilesWithin(diff: number, maxPossible: number) { From 63ec793fadda6bfa09d5fa019850d35102d79b79 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 10:34:42 +0100 Subject: [PATCH 40/72] Support view_room's show_room_tile in the new room list Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomSublist2.tsx | 25 +++++++++++++++ src/components/views/rooms/RoomTile2.tsx | 35 +++++++++++++++++++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 9a36ea00e9..5dce3f769f 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -40,6 +40,8 @@ import NotificationBadge from "./NotificationBadge"; import { ListNotificationState } from "../../../stores/notifications/ListNotificationState"; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; import { Key } from "../../../Keyboard"; +import defaultDispatcher from "../../../dispatcher/dispatcher"; +import {ActionPayload} from "../../../dispatcher/payloads"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -88,6 +90,7 @@ interface IState { export default class RoomSublist2 extends React.Component<IProps, IState> { private headerButton = createRef<HTMLDivElement>(); private sublistRef = createRef<HTMLDivElement>(); + private dispatcherRef: string; constructor(props: IProps) { super(props); @@ -98,6 +101,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { isResizing: false, }; this.state.notificationState.setRooms(this.props.rooms); + this.dispatcherRef = defaultDispatcher.register(this.onAction); } private get numTiles(): number { @@ -116,8 +120,29 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { public componentWillUnmount() { this.state.notificationState.destroy(); + defaultDispatcher.unregister(this.dispatcherRef); } + private onAction = (payload: ActionPayload) => { + if (payload.action === "view_room" && payload.show_room_tile && this.props.rooms) { + // XXX: we have to do this a tick later because we have incorrect intermediate props during a room change + // where we lose the room we are changing from temporarily and then it comes back in an update right after. + setImmediate(() => { + const isCollapsed = this.props.layout.isCollapsed; + const roomIndex = this.props.rooms.findIndex((r) => r.roomId === payload.room_id); + + if (isCollapsed && roomIndex > -1) { + this.toggleCollapsed(); + } + // extend the visible section to include the room + if (roomIndex >= this.numVisibleTiles) { + this.props.layout.visibleTiles = this.props.layout.tilesWithPadding(roomIndex + 1, MAX_PADDING_HEIGHT); + this.forceUpdate(); // because the layout doesn't trigger a re-render + } + }); + } + }; + private onAddRoom = (e) => { e.stopPropagation(); if (this.props.onAddRoom) this.props.onAddRoom(); diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 8ee838fbba..21901026a5 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -17,7 +17,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React from "react"; +import React, {createRef} from "react"; import { Room } from "matrix-js-sdk/src/models/room"; import classNames from "classnames"; import { RovingTabIndexWrapper } from "../../../accessibility/RovingTabIndex"; @@ -49,6 +49,8 @@ import { TagSpecificNotificationState } from "../../../stores/notifications/TagS import { INotificationState } from "../../../stores/notifications/INotificationState"; import NotificationBadge from "./NotificationBadge"; import { NotificationColor } from "../../../stores/notifications/NotificationColor"; +import defaultDispatcher from "../../../dispatcher/dispatcher"; +import {ActionPayload} from "../../../dispatcher/payloads"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -119,6 +121,8 @@ const NotifOption: React.FC<INotifOptionProps> = ({active, onClick, iconClassNam }; export default class RoomTile2 extends React.Component<IProps, IState> { + private dispatcherRef: string; + private roomTileRef = createRef<HTMLDivElement>(); // TODO: a11y: https://github.com/vector-im/riot-web/issues/14180 constructor(props: IProps) { @@ -133,6 +137,7 @@ export default class RoomTile2 extends React.Component<IProps, IState> { }; ActiveRoomObserver.addListener(this.props.room.roomId, this.onActiveRoomUpdate); + this.dispatcherRef = defaultDispatcher.register(this.onAction); } private get showContextMenu(): boolean { @@ -143,12 +148,37 @@ export default class RoomTile2 extends React.Component<IProps, IState> { return !this.props.isMinimized && this.props.showMessagePreview; } + public componentDidMount() { + // when we're first rendered (or our sublist is expanded) make sure we are visible if we're active + if (this.state.selected) { + this.scrollIntoView(); + } + } + public componentWillUnmount() { if (this.props.room) { ActiveRoomObserver.removeListener(this.props.room.roomId, this.onActiveRoomUpdate); } + defaultDispatcher.unregister(this.dispatcherRef); } + private onAction = (payload: ActionPayload) => { + if (payload.action === "view_room" && payload.room_id === this.props.room.roomId && payload.show_room_tile) { + setImmediate(() => { + this.scrollIntoView(); + }); + } + }; + + private scrollIntoView = () => { + console.log("DEBUG scrollIntoView", this.roomTileRef.current); + if (!this.roomTileRef.current) return; + this.roomTileRef.current.scrollIntoView({ + block: "nearest", + behavior: "auto", + }); + }; + private onTileMouseEnter = () => { this.setState({hover: true}); }; @@ -162,7 +192,6 @@ export default class RoomTile2 extends React.Component<IProps, IState> { ev.stopPropagation(); dis.dispatch({ action: 'view_room', - // TODO: Support show_room_tile in new room list: https://github.com/vector-im/riot-web/issues/14233 show_room_tile: true, // make sure the room is visible in the list room_id: this.props.room.roomId, clear_search: (ev && (ev.key === Key.ENTER || ev.key === Key.SPACE)), @@ -481,7 +510,7 @@ export default class RoomTile2 extends React.Component<IProps, IState> { return ( <React.Fragment> - <RovingTabIndexWrapper> + <RovingTabIndexWrapper inputRef={this.roomTileRef}> {({onFocus, isActive, ref}) => <AccessibleButton onFocus={onFocus} From 19e1c79796723600a21e2ea11cb25f701040bef3 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 10:35:16 +0100 Subject: [PATCH 41/72] update comment Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomSublist2.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 5dce3f769f..4dc2097421 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -134,7 +134,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { if (isCollapsed && roomIndex > -1) { this.toggleCollapsed(); } - // extend the visible section to include the room + // extend the visible section to include the room if it is entirely invisible if (roomIndex >= this.numVisibleTiles) { this.props.layout.visibleTiles = this.props.layout.tilesWithPadding(roomIndex + 1, MAX_PADDING_HEIGHT); this.forceUpdate(); // because the layout doesn't trigger a re-render From 994d8708f2e62f944aec46d2c7890cb23072170a Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Tue, 7 Jul 2020 06:52:44 -0600 Subject: [PATCH 42/72] Move to a fragment --- src/components/views/rooms/RoomSublist2.tsx | 52 ++++++++++----------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index f5a1b67571..ca798d12f1 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -315,34 +315,32 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { const isAlphabetical = RoomListStore.instance.getTagSorting(this.props.tagId) === SortAlgorithm.Alphabetic; const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance; - // Invites don't get some nonsense options, so only add them if we have to. We do - // this with an array instead of a containing div to ensure that the DOM structure - // is relatively sane. - let otherSections = []; + // Invites don't get some nonsense options, so only add them if we have to. + let otherSections = null; if (this.props.tagId !== DefaultTagID.Invite) { - otherSections.push(<hr key={otherSections.length} />); - otherSections.push( - <div key={otherSections.length}> - <div className='mx_RoomSublist2_contextMenu_title'>{_t("Unread rooms")}</div> - <StyledCheckbox - onChange={this.onUnreadFirstChanged} - checked={isUnreadFirst} - > - {_t("Always show first")} - </StyledCheckbox> - </div>, - ); - otherSections.push(<hr key={otherSections.length} />); - otherSections.push( - <div key={otherSections.length}> - <div className='mx_RoomSublist2_contextMenu_title'>{_t("Show")}</div> - <StyledCheckbox - onChange={this.onMessagePreviewChanged} - checked={this.props.layout.showPreviews} - > - {_t("Message preview")} - </StyledCheckbox> - </div>, + otherSections = ( + <React.Fragment> + <hr /> + <div> + <div className='mx_RoomSublist2_contextMenu_title'>{_t("Unread rooms")}</div> + <StyledCheckbox + onChange={this.onUnreadFirstChanged} + checked={isUnreadFirst} + > + {_t("Always show first")} + </StyledCheckbox> + </div> + <hr /> + <div> + <div className='mx_RoomSublist2_contextMenu_title'>{_t("Show")}</div> + <StyledCheckbox + onChange={this.onMessagePreviewChanged} + checked={this.props.layout.showPreviews} + > + {_t("Message preview")} + </StyledCheckbox> + </div> + </React.Fragment> ); } From 1b48b99f99b712ecbef7cc5fb5643000b5115a1a Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Tue, 7 Jul 2020 06:53:17 -0600 Subject: [PATCH 43/72] Append community invites to bottom instead --- src/components/views/rooms/RoomSublist2.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index ca798d12f1..90ca8b2d4b 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -279,10 +279,6 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { const tiles: React.ReactElement[] = []; - if (this.props.extraBadTilesThatShouldntExist) { - tiles.push(...this.props.extraBadTilesThatShouldntExist); - } - if (this.props.rooms) { const visibleRooms = this.props.rooms.slice(0, this.numVisibleTiles); for (const room of visibleRooms) { @@ -298,6 +294,10 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { } } + if (this.props.extraBadTilesThatShouldntExist) { + tiles.push(...this.props.extraBadTilesThatShouldntExist); + } + // We only have to do this because of the extra tiles. We do it conditionally // to avoid spending cycles on slicing. It's generally fine to do this though // as users are unlikely to have more than a handful of tiles when the extra From 44ae83f228fca22fad11e377df774ca15e8c0930 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 14:07:35 +0100 Subject: [PATCH 44/72] Move the Volume union type out to a throwaway Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/RoomNotifsTypes.ts | 24 ++++++++++++++++++++++++ src/components/views/rooms/RoomTile2.tsx | 3 +-- 2 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 src/RoomNotifsTypes.ts diff --git a/src/RoomNotifsTypes.ts b/src/RoomNotifsTypes.ts new file mode 100644 index 0000000000..0e7093e434 --- /dev/null +++ b/src/RoomNotifsTypes.ts @@ -0,0 +1,24 @@ +/* +Copyright 2020 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 { + ALL_MESSAGES, + ALL_MESSAGES_LOUD, + MENTIONS_ONLY, + MUTE, +} from "./RoomNotifs"; + +export type Volume = ALL_MESSAGES_LOUD | ALL_MESSAGES | MENTIONS_ONLY | MUTE; diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 8ee838fbba..7192bb6611 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -49,6 +49,7 @@ import { TagSpecificNotificationState } from "../../../stores/notifications/TagS import { INotificationState } from "../../../stores/notifications/INotificationState"; import NotificationBadge from "./NotificationBadge"; import { NotificationColor } from "../../../stores/notifications/NotificationColor"; +import { Volume } from "../../../RoomNotifsTypes"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -80,8 +81,6 @@ interface IState { generalMenuPosition: PartialDOMRect; } -type Volume = ALL_MESSAGES_LOUD | ALL_MESSAGES | MENTIONS_ONLY | MUTE; - const messagePreviewId = (roomId: string) => `mx_RoomTile2_messagePreview_${roomId}`; const contextMenuBelow = (elementRect: PartialDOMRect) => { From 8c2286a0447de504d6cb717271df18de1c2e39a6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 15:24:46 +0100 Subject: [PATCH 45/72] Move all the ContextMenu semantic helper (ARIA) components out to separate modules Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../context_menu/ContextMenuButton.tsx | 51 +++++ src/accessibility/context_menu/MenuGroup.tsx | 31 +++ src/accessibility/context_menu/MenuItem.tsx | 36 ++++ .../context_menu/MenuItemCheckbox.tsx | 45 ++++ .../context_menu/MenuItemRadio.tsx | 45 ++++ .../context_menu/StyledMenuItemCheckbox.tsx | 64 ++++++ .../context_menu/StyledMenuItemRadio.tsx | 65 ++++++ src/components/structures/ContextMenu.tsx | 193 +----------------- 8 files changed, 346 insertions(+), 184 deletions(-) create mode 100644 src/accessibility/context_menu/ContextMenuButton.tsx create mode 100644 src/accessibility/context_menu/MenuGroup.tsx create mode 100644 src/accessibility/context_menu/MenuItem.tsx create mode 100644 src/accessibility/context_menu/MenuItemCheckbox.tsx create mode 100644 src/accessibility/context_menu/MenuItemRadio.tsx create mode 100644 src/accessibility/context_menu/StyledMenuItemCheckbox.tsx create mode 100644 src/accessibility/context_menu/StyledMenuItemRadio.tsx diff --git a/src/accessibility/context_menu/ContextMenuButton.tsx b/src/accessibility/context_menu/ContextMenuButton.tsx new file mode 100644 index 0000000000..c358155e10 --- /dev/null +++ b/src/accessibility/context_menu/ContextMenuButton.tsx @@ -0,0 +1,51 @@ +/* +Copyright 2015, 2016 OpenMarket Ltd +Copyright 2018 New Vector Ltd +Copyright 2019 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 React from "react"; + +import AccessibleButton, {IProps as IAccessibleButtonProps} from "../../components/views/elements/AccessibleButton"; + +interface IProps extends IAccessibleButtonProps { + label?: string; + // whether or not the context menu is currently open + isExpanded: boolean; +} + +// Semantic component for representing the AccessibleButton which launches a <ContextMenu /> +export const ContextMenuButton: React.FC<IProps> = ({ + label, + isExpanded, + children, + onClick, + onContextMenu, + ...props +}) => { + return ( + <AccessibleButton + {...props} + onClick={onClick} + onContextMenu={onContextMenu || onClick} + title={label} + aria-label={label} + aria-haspopup={true} + aria-expanded={isExpanded} + > + { children } + </AccessibleButton> + ); +}; diff --git a/src/accessibility/context_menu/MenuGroup.tsx b/src/accessibility/context_menu/MenuGroup.tsx new file mode 100644 index 0000000000..f4b7b6bc56 --- /dev/null +++ b/src/accessibility/context_menu/MenuGroup.tsx @@ -0,0 +1,31 @@ +/* +Copyright 2015, 2016 OpenMarket Ltd +Copyright 2018 New Vector Ltd +Copyright 2019 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 React from "react"; + +interface IProps extends React.HTMLAttributes<HTMLDivElement> { + label: string; + className?: string; +} + +// Semantic component for representing a role=group for grouping menu radios/checkboxes +export const MenuGroup: React.FC<IProps> = ({children, label, ...props}) => { + return <div {...props} role="group" aria-label={label}> + { children } + </div>; +}; diff --git a/src/accessibility/context_menu/MenuItem.tsx b/src/accessibility/context_menu/MenuItem.tsx new file mode 100644 index 0000000000..8e33d55de4 --- /dev/null +++ b/src/accessibility/context_menu/MenuItem.tsx @@ -0,0 +1,36 @@ +/* +Copyright 2015, 2016 OpenMarket Ltd +Copyright 2018 New Vector Ltd +Copyright 2019 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 React from "react"; + +import AccessibleButton, {ButtonEvent, IProps as IAccessibleButtonProps} from "../../components/views/elements/AccessibleButton"; + +interface IProps extends IAccessibleButtonProps { + label?: string; + className?: string; + onClick(ev: ButtonEvent); +} + +// Semantic component for representing a role=menuitem +export const MenuItem: React.FC<IProps> = ({children, label, ...props}) => { + return ( + <AccessibleButton {...props} role="menuitem" tabIndex={-1} aria-label={label}> + { children } + </AccessibleButton> + ); +}; diff --git a/src/accessibility/context_menu/MenuItemCheckbox.tsx b/src/accessibility/context_menu/MenuItemCheckbox.tsx new file mode 100644 index 0000000000..e2cc04b5a6 --- /dev/null +++ b/src/accessibility/context_menu/MenuItemCheckbox.tsx @@ -0,0 +1,45 @@ +/* +Copyright 2015, 2016 OpenMarket Ltd +Copyright 2018 New Vector Ltd +Copyright 2019 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 React from "react"; + +import AccessibleButton, {ButtonEvent, IProps as IAccessibleButtonProps} from "../../components/views/elements/AccessibleButton"; + +interface IProps extends IAccessibleButtonProps { + label?: string; + active: boolean; + disabled?: boolean; + className?: string; + onClick(ev: ButtonEvent); +} + +// Semantic component for representing a role=menuitemcheckbox +export const MenuItemCheckbox: React.FC<IProps> = ({children, label, active, disabled, ...props}) => { + return ( + <AccessibleButton + {...props} + role="menuitemcheckbox" + aria-checked={active} + aria-disabled={disabled} + tabIndex={-1} + aria-label={label} + > + { children } + </AccessibleButton> + ); +}; diff --git a/src/accessibility/context_menu/MenuItemRadio.tsx b/src/accessibility/context_menu/MenuItemRadio.tsx new file mode 100644 index 0000000000..21732220df --- /dev/null +++ b/src/accessibility/context_menu/MenuItemRadio.tsx @@ -0,0 +1,45 @@ +/* +Copyright 2015, 2016 OpenMarket Ltd +Copyright 2018 New Vector Ltd +Copyright 2019 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 React from "react"; + +import AccessibleButton, {ButtonEvent, IProps as IAccessibleButtonProps} from "../../components/views/elements/AccessibleButton"; + +interface IProps extends IAccessibleButtonProps { + label?: string; + active: boolean; + disabled?: boolean; + className?: string; + onClick(ev: ButtonEvent); +} + +// Semantic component for representing a role=menuitemradio +export const MenuItemRadio: React.FC<IProps> = ({children, label, active, disabled, ...props}) => { + return ( + <AccessibleButton + {...props} + role="menuitemradio" + aria-checked={active} + aria-disabled={disabled} + tabIndex={-1} + aria-label={label} + > + { children } + </AccessibleButton> + ); +}; diff --git a/src/accessibility/context_menu/StyledMenuItemCheckbox.tsx b/src/accessibility/context_menu/StyledMenuItemCheckbox.tsx new file mode 100644 index 0000000000..f5a510f517 --- /dev/null +++ b/src/accessibility/context_menu/StyledMenuItemCheckbox.tsx @@ -0,0 +1,64 @@ +/* +Copyright 2015, 2016 OpenMarket Ltd +Copyright 2018 New Vector Ltd +Copyright 2019 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 React from "react"; + +import {Key} from "../../Keyboard"; +import StyledCheckbox from "../../components/views/elements/StyledCheckbox"; + +interface IProps extends React.ComponentProps<typeof StyledCheckbox> { + label?: string; + onChange(); + onClose(): void; // gets called after onChange on Key.ENTER +} + +// Semantic component for representing a styled role=menuitemcheckbox +export const StyledMenuItemCheckbox: React.FC<IProps> = ({children, label, onChange, onClose, ...props}) => { + const onKeyDown = (e: React.KeyboardEvent) => { + if (e.key === Key.ENTER || e.key === Key.SPACE) { + e.stopPropagation(); + e.preventDefault(); + onChange(); + // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 + if (e.key === Key.ENTER) { + onClose(); + } + } + }; + const onKeyUp = (e: React.KeyboardEvent) => { + // prevent the input default handler as we handle it on keydown to match + // https://www.w3.org/TR/wai-aria-practices/examples/menubar/menubar-2/menubar-2.html + if (e.key === Key.SPACE || e.key === Key.ENTER) { + e.stopPropagation(); + e.preventDefault(); + } + }; + return ( + <StyledCheckbox + {...props} + role="menuitemcheckbox" + tabIndex={-1} + aria-label={label} + onChange={onChange} + onKeyDown={onKeyDown} + onKeyUp={onKeyUp} + > + { children } + </StyledCheckbox> + ); +}; diff --git a/src/accessibility/context_menu/StyledMenuItemRadio.tsx b/src/accessibility/context_menu/StyledMenuItemRadio.tsx new file mode 100644 index 0000000000..be87ccc683 --- /dev/null +++ b/src/accessibility/context_menu/StyledMenuItemRadio.tsx @@ -0,0 +1,65 @@ +/* +Copyright 2015, 2016 OpenMarket Ltd +Copyright 2018 New Vector Ltd +Copyright 2019 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 React from "react"; + +import {Key} from "../../Keyboard"; +import StyledRadioButton from "../../components/views/elements/StyledRadioButton"; + +interface IProps extends React.ComponentProps<typeof StyledRadioButton> { + label?: string; + disabled?: boolean; + onChange(): void; + onClose(): void; // gets called after onChange on Key.ENTER +} + +// Semantic component for representing a styled role=menuitemradio +export const StyledMenuItemRadio: React.FC<IProps> = ({children, label, onChange, onClose, ...props}) => { + const onKeyDown = (e: React.KeyboardEvent) => { + if (e.key === Key.ENTER || e.key === Key.SPACE) { + e.stopPropagation(); + e.preventDefault(); + onChange(); + // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 + if (e.key === Key.ENTER) { + onClose(); + } + } + }; + const onKeyUp = (e: React.KeyboardEvent) => { + // prevent the input default handler as we handle it on keydown to match + // https://www.w3.org/TR/wai-aria-practices/examples/menubar/menubar-2/menubar-2.html + if (e.key === Key.SPACE || e.key === Key.ENTER) { + e.stopPropagation(); + e.preventDefault(); + } + }; + return ( + <StyledRadioButton + {...props} + role="menuitemradio" + tabIndex={-1} + aria-label={label} + onChange={onChange} + onKeyDown={onKeyDown} + onKeyUp={onKeyUp} + > + { children } + </StyledRadioButton> + ); +}; diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 872a8b0cd9..cb1349da4b 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -21,10 +21,7 @@ import ReactDOM from "react-dom"; import classNames from "classnames"; import {Key} from "../../Keyboard"; -import AccessibleButton, { IProps as IAccessibleButtonProps, ButtonEvent } from "../views/elements/AccessibleButton"; import {Writeable} from "../../@types/common"; -import StyledCheckbox from "../views/elements/StyledCheckbox"; -import StyledRadioButton from "../views/elements/StyledRadioButton"; // Shamelessly ripped off Modal.js. There's probably a better way // of doing reusable widgets like dialog boxes & menus where we go and @@ -390,187 +387,6 @@ export class ContextMenu extends React.PureComponent<IProps, IState> { } } -interface IContextMenuButtonProps extends IAccessibleButtonProps { - label?: string; - // whether or not the context menu is currently open - isExpanded: boolean; -} - -// Semantic component for representing the AccessibleButton which launches a <ContextMenu /> -export const ContextMenuButton: React.FC<IContextMenuButtonProps> = ({ label, isExpanded, children, onClick, onContextMenu, ...props }) => { - return ( - <AccessibleButton - {...props} - onClick={onClick} - onContextMenu={onContextMenu || onClick} - title={label} - aria-label={label} - aria-haspopup={true} - aria-expanded={isExpanded} - > - { children } - </AccessibleButton> - ); -}; - -interface IMenuItemProps extends IAccessibleButtonProps { - label?: string; - className?: string; - onClick(ev: ButtonEvent); -} - -// Semantic component for representing a role=menuitem -export const MenuItem: React.FC<IMenuItemProps> = ({children, label, ...props}) => { - return ( - <AccessibleButton {...props} role="menuitem" tabIndex={-1} aria-label={label}> - { children } - </AccessibleButton> - ); -}; - -interface IMenuGroupProps extends React.HTMLAttributes<HTMLDivElement> { - label: string; - className?: string; -} - -// Semantic component for representing a role=group for grouping menu radios/checkboxes -export const MenuGroup: React.FC<IMenuGroupProps> = ({children, label, ...props}) => { - return <div {...props} role="group" aria-label={label}> - { children } - </div>; -}; - -interface IMenuItemCheckboxProps extends IAccessibleButtonProps { - label?: string; - active: boolean; - disabled?: boolean; - className?: string; - onClick(ev: ButtonEvent); -} - -// Semantic component for representing a role=menuitemcheckbox -export const MenuItemCheckbox: React.FC<IMenuItemCheckboxProps> = ({children, label, active = false, disabled = false, ...props}) => { - return ( - <AccessibleButton {...props} role="menuitemcheckbox" aria-checked={active} aria-disabled={disabled} tabIndex={-1} aria-label={label}> - { children } - </AccessibleButton> - ); -}; - -interface IStyledMenuItemCheckboxProps extends IAccessibleButtonProps { - label?: string; - active: boolean; - disabled?: boolean; - className?: string; - onChange(); - onClose(): void; // gets called after onChange on Key.ENTER -} - -// Semantic component for representing a styled role=menuitemcheckbox -export const StyledMenuItemCheckbox: React.FC<IStyledMenuItemCheckboxProps> = ({children, label, onChange, onClose, checked, disabled=false, ...props}) => { - const onKeyDown = (e) => { - if (e.key === Key.ENTER || e.key === Key.SPACE) { - e.stopPropagation(); - e.preventDefault(); - onChange(); - // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 - if (e.key === Key.ENTER) { - onClose(); - } - } - }; - const onKeyUp = (e) => { - // prevent the input default handler as we handle it on keydown to match - // https://www.w3.org/TR/wai-aria-practices/examples/menubar/menubar-2/menubar-2.html - if (e.key === Key.SPACE || e.key === Key.ENTER) { - e.stopPropagation(); - e.preventDefault(); - } - }; - return ( - <StyledCheckbox - {...props} - role="menuitemcheckbox" - aria-checked={checked} - checked={checked} - aria-disabled={disabled} - tabIndex={-1} - aria-label={label} - onChange={onChange} - onKeyDown={onKeyDown} - onKeyUp={onKeyUp} - > - { children } - </StyledCheckbox> - ); -}; - -interface IMenuItemRadioProps extends IAccessibleButtonProps { - label?: string; - active: boolean; - disabled?: boolean; - className?: string; - onClick(ev: ButtonEvent); -} - -// Semantic component for representing a role=menuitemradio -export const MenuItemRadio: React.FC<IMenuItemRadioProps> = ({children, label, active = false, disabled = false, ...props}) => { - return ( - <AccessibleButton {...props} role="menuitemradio" aria-checked={active} aria-disabled={disabled} tabIndex={-1} aria-label={label}> - { children } - </AccessibleButton> - ); -}; - - -interface IStyledMenuItemRadioProps extends IAccessibleButtonProps { - label?: string; - active: boolean; - disabled?: boolean; - className?: string; - onChange(); - onClose(): void; // gets called after onChange on Key.ENTER -} - -// Semantic component for representing a styled role=menuitemradio -export const StyledMenuItemRadio: React.FC<IStyledMenuItemRadioProps> = ({children, label, onChange, onClose, checked=false, disabled=false, ...props}) => { - const onKeyDown = (e) => { - if (e.key === Key.ENTER || e.key === Key.SPACE) { - e.stopPropagation(); - e.preventDefault(); - onChange(); - // Implements https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 - if (e.key === Key.ENTER) { - onClose(); - } - } - }; - const onKeyUp = (e) => { - // prevent the input default handler as we handle it on keydown to match - // https://www.w3.org/TR/wai-aria-practices/examples/menubar/menubar-2/menubar-2.html - if (e.key === Key.SPACE || e.key === Key.ENTER) { - e.stopPropagation(); - e.preventDefault(); - } - }; - return ( - <StyledRadioButton - {...props} - role="menuitemradio" - aria-checked={checked} - checked={checked} - aria-disabled={disabled} - tabIndex={-1} - aria-label={label} - onChange={onChange} - onKeyDown={onKeyDown} - onKeyUp={onKeyUp} - > - { children } - </StyledRadioButton> - ); -}; - // Placement method for <ContextMenu /> to position context menu to right of elementRect with chevronOffset export const toRightOf = (elementRect: DOMRect, chevronOffset = 12) => { const left = elementRect.right + window.pageXOffset + 3; @@ -639,3 +455,12 @@ export function createMenu(ElementClass, props) { return {close: onFinished}; } + +// re-export the semantic helper components for simplicity +export {ContextMenuButton} from "../../accessibility/context_menu/ContextMenuButton"; +export {MenuGroup} from "../../accessibility/context_menu/MenuGroup"; +export {MenuItem} from "../../accessibility/context_menu/MenuItem"; +export {MenuItemCheckbox} from "../../accessibility/context_menu/MenuItemCheckbox"; +export {MenuItemRadio} from "../../accessibility/context_menu/MenuItemRadio"; +export {StyledMenuItemCheckbox} from "../../accessibility/context_menu/StyledMenuItemCheckbox"; +export {StyledMenuItemRadio} from "../../accessibility/context_menu/StyledMenuItemRadio"; From eb05c86e506c7379205e752dee0969204cd117ea Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 15:32:20 +0100 Subject: [PATCH 46/72] clean-up Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/accessibility/context_menu/MenuGroup.tsx | 1 - src/accessibility/context_menu/MenuItem.tsx | 7 +++---- src/accessibility/context_menu/MenuItemCheckbox.tsx | 8 +++----- src/accessibility/context_menu/MenuItemRadio.tsx | 8 +++----- src/accessibility/context_menu/StyledMenuItemCheckbox.tsx | 2 +- src/accessibility/context_menu/StyledMenuItemRadio.tsx | 3 +-- 6 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/accessibility/context_menu/MenuGroup.tsx b/src/accessibility/context_menu/MenuGroup.tsx index f4b7b6bc56..9334e17a18 100644 --- a/src/accessibility/context_menu/MenuGroup.tsx +++ b/src/accessibility/context_menu/MenuGroup.tsx @@ -20,7 +20,6 @@ import React from "react"; interface IProps extends React.HTMLAttributes<HTMLDivElement> { label: string; - className?: string; } // Semantic component for representing a role=group for grouping menu radios/checkboxes diff --git a/src/accessibility/context_menu/MenuItem.tsx b/src/accessibility/context_menu/MenuItem.tsx index 8e33d55de4..64233e51ad 100644 --- a/src/accessibility/context_menu/MenuItem.tsx +++ b/src/accessibility/context_menu/MenuItem.tsx @@ -18,12 +18,10 @@ limitations under the License. import React from "react"; -import AccessibleButton, {ButtonEvent, IProps as IAccessibleButtonProps} from "../../components/views/elements/AccessibleButton"; +import AccessibleButton from "../../components/views/elements/AccessibleButton"; -interface IProps extends IAccessibleButtonProps { +interface IProps extends React.ComponentProps<typeof AccessibleButton> { label?: string; - className?: string; - onClick(ev: ButtonEvent); } // Semantic component for representing a role=menuitem @@ -34,3 +32,4 @@ export const MenuItem: React.FC<IProps> = ({children, label, ...props}) => { </AccessibleButton> ); }; + diff --git a/src/accessibility/context_menu/MenuItemCheckbox.tsx b/src/accessibility/context_menu/MenuItemCheckbox.tsx index e2cc04b5a6..5eb8cc4819 100644 --- a/src/accessibility/context_menu/MenuItemCheckbox.tsx +++ b/src/accessibility/context_menu/MenuItemCheckbox.tsx @@ -18,14 +18,11 @@ limitations under the License. import React from "react"; -import AccessibleButton, {ButtonEvent, IProps as IAccessibleButtonProps} from "../../components/views/elements/AccessibleButton"; +import AccessibleButton from "../../components/views/elements/AccessibleButton"; -interface IProps extends IAccessibleButtonProps { +interface IProps extends React.ComponentProps<typeof AccessibleButton> { label?: string; active: boolean; - disabled?: boolean; - className?: string; - onClick(ev: ButtonEvent); } // Semantic component for representing a role=menuitemcheckbox @@ -36,6 +33,7 @@ export const MenuItemCheckbox: React.FC<IProps> = ({children, label, active, dis role="menuitemcheckbox" aria-checked={active} aria-disabled={disabled} + disabled={disabled} tabIndex={-1} aria-label={label} > diff --git a/src/accessibility/context_menu/MenuItemRadio.tsx b/src/accessibility/context_menu/MenuItemRadio.tsx index 21732220df..472f13ff14 100644 --- a/src/accessibility/context_menu/MenuItemRadio.tsx +++ b/src/accessibility/context_menu/MenuItemRadio.tsx @@ -18,14 +18,11 @@ limitations under the License. import React from "react"; -import AccessibleButton, {ButtonEvent, IProps as IAccessibleButtonProps} from "../../components/views/elements/AccessibleButton"; +import AccessibleButton from "../../components/views/elements/AccessibleButton"; -interface IProps extends IAccessibleButtonProps { +interface IProps extends React.ComponentProps<typeof AccessibleButton> { label?: string; active: boolean; - disabled?: boolean; - className?: string; - onClick(ev: ButtonEvent); } // Semantic component for representing a role=menuitemradio @@ -36,6 +33,7 @@ export const MenuItemRadio: React.FC<IProps> = ({children, label, active, disabl role="menuitemradio" aria-checked={active} aria-disabled={disabled} + disabled={disabled} tabIndex={-1} aria-label={label} > diff --git a/src/accessibility/context_menu/StyledMenuItemCheckbox.tsx b/src/accessibility/context_menu/StyledMenuItemCheckbox.tsx index f5a510f517..d373f892c9 100644 --- a/src/accessibility/context_menu/StyledMenuItemCheckbox.tsx +++ b/src/accessibility/context_menu/StyledMenuItemCheckbox.tsx @@ -23,7 +23,7 @@ import StyledCheckbox from "../../components/views/elements/StyledCheckbox"; interface IProps extends React.ComponentProps<typeof StyledCheckbox> { label?: string; - onChange(); + onChange(); // we handle keyup/down ourselves so lose the ChangeEvent onClose(): void; // gets called after onChange on Key.ENTER } diff --git a/src/accessibility/context_menu/StyledMenuItemRadio.tsx b/src/accessibility/context_menu/StyledMenuItemRadio.tsx index be87ccc683..5e5aa90a38 100644 --- a/src/accessibility/context_menu/StyledMenuItemRadio.tsx +++ b/src/accessibility/context_menu/StyledMenuItemRadio.tsx @@ -23,8 +23,7 @@ import StyledRadioButton from "../../components/views/elements/StyledRadioButton interface IProps extends React.ComponentProps<typeof StyledRadioButton> { label?: string; - disabled?: boolean; - onChange(): void; + onChange(); // we handle keyup/down ourselves so lose the ChangeEvent onClose(): void; // gets called after onChange on Key.ENTER } From da1d1ffa09ac7a8105da9b3d0312a93db0e73802 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens <joriksch@gmail.com> Date: Tue, 7 Jul 2020 15:42:54 +0100 Subject: [PATCH 47/72] Change colour to orange and do some lints --- res/themes/light/css/_light.scss | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/res/themes/light/css/_light.scss b/res/themes/light/css/_light.scss index c4b4262642..b21fe34fba 100644 --- a/res/themes/light/css/_light.scss +++ b/res/themes/light/css/_light.scss @@ -36,7 +36,7 @@ $focus-bg-color: #dddddd; $accent-fg-color: #ffffff; $accent-color-50pct: rgba(3, 179, 129, 0.5); //#03b381 in rgb $accent-color-darker: #92caad; -$accent-color-alt: #238CF5; +$accent-color-alt: #238cf5; $selection-fg-color: $primary-bg-color; @@ -46,8 +46,8 @@ $focus-brightness: 105%; $warning-color: $notice-primary-color; // red $orange-warning-color: #ff8d13; // used for true warnings // background colour for warnings -$warning-bg-color: #DF2A8B; -$info-bg-color: #2A9EDF; +$warning-bg-color: #df2a8b; +$info-bg-color: #2a9edf; $mention-user-pill-bg-color: $warning-color; $other-user-pill-bg-color: rgba(0, 0, 0, 0.1); @@ -71,7 +71,7 @@ $tagpanel-bg-color: #27303a; $plinth-bg-color: $secondary-accent-color; // used by RoomDropTarget -$droptarget-bg-color: rgba(255,255,255,0.5); +$droptarget-bg-color: rgba(255, 255, 255, 0.5); // used by AddressSelector $selected-color: $secondary-accent-color; @@ -157,18 +157,18 @@ $rte-group-pill-color: #aaa; $topleftmenu-color: #212121; $roomheader-color: #45474a; -$roomheader-addroom-bg-color: #91A1C0; +$roomheader-addroom-bg-color: #91a1c0; $roomheader-addroom-fg-color: $accent-fg-color; -$tagpanel-button-color: #91A1C0; -$roomheader-button-color: #91A1C0; -$groupheader-button-color: #91A1C0; -$rightpanel-button-color: #91A1C0; -$composer-button-color: #91A1C0; +$tagpanel-button-color: #91a1c0; +$roomheader-button-color: #91a1c0; +$groupheader-button-color: #91a1c0; +$rightpanel-button-color: #91a1c0; +$composer-button-color: #91a1c0; $roomtopic-color: #9e9e9e; $eventtile-meta-color: $roomtopic-color; $composer-e2e-icon-color: #c9ced6; -$header-divider-color: #91A1C0; +$header-divider-color: #91a1c0; // ******************** @@ -184,11 +184,11 @@ $roomsublist2-divider-color: $primary-fg-color; $roomtile2-preview-color: #9e9e9e; $roomtile2-default-badge-bg-color: #61708b; -$roomtile2-selected-bg-color: #FFF; +$roomtile2-selected-bg-color: #fff; $presence-online: $accent-color; -$presence-away: orange; // TODO: Get color -$presence-offline: #E3E8F0; +$presence-away: #d9b072; // TODO: Get color +$presence-offline: #e3e8f0; // ******************** From 6b5eaca0b99d6f2a1f88adc735c05de0e693889d Mon Sep 17 00:00:00 2001 From: Jorik Schellekens <joriksch@gmail.com> Date: Tue, 7 Jul 2020 16:45:43 +0100 Subject: [PATCH 48/72] Remove comment --- res/themes/light/css/_light.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/res/themes/light/css/_light.scss b/res/themes/light/css/_light.scss index b21fe34fba..8469a85bfe 100644 --- a/res/themes/light/css/_light.scss +++ b/res/themes/light/css/_light.scss @@ -187,7 +187,7 @@ $roomtile2-default-badge-bg-color: #61708b; $roomtile2-selected-bg-color: #fff; $presence-online: $accent-color; -$presence-away: #d9b072; // TODO: Get color +$presence-away: #d9b072; $presence-offline: #e3e8f0; // ******************** From 92e86af162b9d37f79a0c2b1693730c8b65dd3a3 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 17:07:51 +0100 Subject: [PATCH 49/72] Show more/Show less keep focus in a relevant place Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomSublist2.tsx | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 18f8f4e2f6..9d15e64e7b 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -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) => { @@ -520,6 +530,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { </span> ); if (this.props.isMinimized) showMoreText = null; + // TODO Roving tab index / treeitem?: https://github.com/vector-im/riot-web/issues/14180 showNButton = ( <AccessibleButton onClick={this.onShowAllClick} className={showMoreBtnClasses} tabIndex={-1}> <span className='mx_RoomSublist2_showMoreButtonChevron mx_RoomSublist2_showNButtonChevron'> From f18db23cc4e7138c8b5a55598430711c59bf65fe Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 17:18:56 +0100 Subject: [PATCH 50/72] Remove some TODOs Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/LeftPanel2.tsx | 3 --- src/components/views/rooms/RoomSublist2.tsx | 1 - src/components/views/rooms/RoomTile2.tsx | 3 --- 3 files changed, 7 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 4b954d7843..b2a2384cb2 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/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")} diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 9d15e64e7b..3a12e99914 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -460,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 diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index c6cd401803..abb31a6f71 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/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, From 4edd3dfc6c9b8bc3185adaa231b82ba65fa4ea10 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 17:46:33 +0100 Subject: [PATCH 51/72] Convert RovingTabIndex to Typescript Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../{RovingTabIndex.js => RovingTabIndex.tsx} | 87 +++++++++++++------ 1 file changed, 62 insertions(+), 25 deletions(-) rename src/accessibility/{RovingTabIndex.js => RovingTabIndex.tsx} (79%) diff --git a/src/accessibility/RovingTabIndex.js b/src/accessibility/RovingTabIndex.tsx similarity index 79% rename from src/accessibility/RovingTabIndex.js rename to src/accessibility/RovingTabIndex.tsx index b481f08fe2..32780629f2 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<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 -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<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, - 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<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,8 +244,17 @@ 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}); }; From a33717a475a74f9b4bd773169fb624de450ed509 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 17:47:21 +0100 Subject: [PATCH 52/72] Wire up Room sublist show more/less as roving tabindex button using new helper Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/accessibility/RovingTabIndex.tsx | 10 ++++++++++ src/components/views/rooms/RoomSublist2.tsx | 10 +++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/accessibility/RovingTabIndex.tsx b/src/accessibility/RovingTabIndex.tsx index 32780629f2..388d67d9f3 100644 --- a/src/accessibility/RovingTabIndex.tsx +++ b/src/accessibility/RovingTabIndex.tsx @@ -259,3 +259,13 @@ export const RovingTabIndexWrapper: React.FC<IRovingTabIndexWrapperProps> = ({ch 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} />; +}; + diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 3a12e99914..56ce631604 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/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"; @@ -531,12 +531,12 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { if (this.props.isMinimized) showMoreText = null; // TODO Roving tab index / treeitem?: https://github.com/vector-im/riot-web/issues/14180 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 @@ -548,12 +548,12 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { 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> ); } From 28310cb64848713486bf4ac05ff4c83517cf1db1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 17:48:39 +0100 Subject: [PATCH 53/72] remove TODOs Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomSublist2.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 56ce631604..eefd29f0b7 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -529,7 +529,6 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { </span> ); if (this.props.isMinimized) showMoreText = null; - // TODO Roving tab index / treeitem?: https://github.com/vector-im/riot-web/issues/14180 showNButton = ( <RovingAccessibleButton onClick={this.onShowAllClick} className={showMoreBtnClasses}> <span className='mx_RoomSublist2_showMoreButtonChevron mx_RoomSublist2_showNButtonChevron'> @@ -546,7 +545,6 @@ 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 = ( <RovingAccessibleButton onClick={this.onShowLessClick} className={showMoreBtnClasses}> <span className='mx_RoomSublist2_showLessButtonChevron mx_RoomSublist2_showNButtonChevron'> From 853b2806738eeccf1a5e2e550f437b69eb6c90d7 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Jul 2020 18:30:57 +0100 Subject: [PATCH 54/72] Fix MELS summary of 3pid invite revocations Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/elements/MemberEventListSummary.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index fc79fc87d0..956b69ca7b 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -1,6 +1,6 @@ /* Copyright 2016 OpenMarket Ltd -Copyright 2019 The Matrix.org Foundation C.I.C. +Copyright 2019, 2020 The Matrix.org Foundation C.I.C. Copyright 2019 Michael Telatynski <7t3chguy@gmail.com> Licensed under the Apache License, Version 2.0 (the "License"); @@ -23,6 +23,7 @@ import { _t } from '../../../languageHandler'; import { formatCommaSeparatedList } from '../../../utils/FormattingUtils'; import * as sdk from "../../../index"; import {MatrixEvent} from "matrix-js-sdk"; +import {isValid3pidInvite} from "../../../RoomInvite"; export default createReactClass({ displayName: 'MemberEventListSummary', @@ -284,6 +285,9 @@ export default createReactClass({ _getTransition: function(e) { if (e.mxEvent.getType() === 'm.room.third_party_invite') { // Handle 3pid invites the same as invites so they get bundled together + if (!isValid3pidInvite(e.mxEvent)) { + return 'invite_withdrawal'; + } return 'invited'; } From e6b20088c0a1c87067f99444d3f90b693ad26cf4 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Tue, 7 Jul 2020 11:33:32 -0600 Subject: [PATCH 55/72] Try using requestAnimationFrame if available for sticky headers This might help performance, or it might not. Let's try it! --- src/components/structures/LeftPanel2.tsx | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index b2a2384cb2..36012a1473 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -69,6 +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; constructor(props: IProps) { super(props); @@ -113,6 +114,24 @@ 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) { + this.isDoingStickyHeaders = true; const rlRect = list.getBoundingClientRect(); const bottom = rlRect.bottom; const top = rlRect.top; From baccabeae461048e1da8d9d52f4b51c33ab170ed Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Tue, 7 Jul 2020 11:34:52 -0600 Subject: [PATCH 56/72] Remove extraneous true --- src/components/structures/LeftPanel2.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 36012a1473..7fac6cbff1 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -131,7 +131,6 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { } private doStickyHeaders(list: HTMLDivElement) { - this.isDoingStickyHeaders = true; const rlRect = list.getBoundingClientRect(); const bottom = rlRect.bottom; const top = rlRect.top; From 7963ed6d047536dfa545a0ff11c332eba7476072 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Tue, 7 Jul 2020 13:42:15 -0600 Subject: [PATCH 57/72] Mute "Unknown room caused setting update" spam See comment enclosed within. Fixes https://github.com/vector-im/riot-web/issues/14254 --- src/settings/handlers/RoomSettingsHandler.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) 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']; From be1b2fddafd7a3588bbbd3e7709540b04a550a8c Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Tue, 7 Jul 2020 13:46:10 -0600 Subject: [PATCH 58/72] Ensure DMs are not lost in the new room list Fixes https://github.com/vector-im/riot-web/issues/14236 --- src/stores/room-list/algorithms/Algorithm.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index d5f2ed0053..41066fc14b 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -530,9 +530,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); - + if (DMRoomMap.getUserIdForRoomId(room.roomId)) { + newTags[DefaultTagID.DM].push(room); + } else { + 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`); } From 2488520263d0097f6a08fefd98f1540de7bc8839 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Tue, 7 Jul 2020 13:46:29 -0600 Subject: [PATCH 59/72] Clean up tag logging in setKnownRooms We don't need this anymore --- src/stores/room-list/algorithms/Algorithm.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 41066fc14b..e70d7d468e 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -501,13 +501,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 +514,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; } @@ -535,8 +527,6 @@ export class Algorithm extends EventEmitter { } else { 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`); } } From 34ea8342fb3654f5a18ec07df5137a4159f9cc8a Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Tue, 7 Jul 2020 13:49:36 -0600 Subject: [PATCH 60/72] Remove comment claiming encrypted rooms are handled incorrectly Fixes https://github.com/vector-im/riot-web/issues/14238 The encrypted rooms are loaded on startup (eventually), so we don't need to worry about the problem described. --- src/stores/room-list/RoomListStore2.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index e5205f6051..60174b63c8 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -221,9 +221,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 From a4ef5909f93cdb1685efe7673ae601b8b6746433 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Tue, 7 Jul 2020 14:45:08 -0600 Subject: [PATCH 61/72] Respect and fix understanding of legacy options Fixes https://github.com/vector-im/riot-web/issues/14372 We read/use the options in multiple places, and those places were not in sync. Now when algorithms change and on initial load, both will come to the same conclusions about how to order & sort the rooms. --- src/settings/Settings.js | 4 +- src/stores/room-list/RoomListStore2.ts | 57 ++++++++++++++++---- src/stores/room-list/algorithms/Algorithm.ts | 2 + 3 files changed, 52 insertions(+), 11 deletions(-) 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/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index e5205f6051..ac765b1e05 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; @@ -321,6 +322,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 +360,35 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> { return <ListAlgorithm>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 +417,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; - 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..91d156d0d9 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]; } From 774e32ecf0dd3f36b467b82df4ad377c6aa924c4 Mon Sep 17 00:00:00 2001 From: Travis Ralston <travpc@gmail.com> Date: Tue, 7 Jul 2020 16:16:46 -0600 Subject: [PATCH 62/72] Fix DM handling in new room list --- src/stores/room-list/algorithms/Algorithm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 361f42eac5..8c7bbc8615 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -524,7 +524,7 @@ export class Algorithm extends EventEmitter { } if (!inTag) { - if (DMRoomMap.getUserIdForRoomId(room.roomId)) { + if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { newTags[DefaultTagID.DM].push(room); } else { newTags[DefaultTagID.Untagged].push(room); From 92dec8ddd81b326558a9032bce253138d3fcfd52 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens <joriksch@gmail.com> Date: Wed, 8 Jul 2020 00:16:24 +0100 Subject: [PATCH 63/72] Fix gaps --- res/css/views/rooms/_RoomSublist2.scss | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 0e76152f86..a8de6dd409 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -203,15 +203,15 @@ 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 28px high and 8px padding on the top this equates to 0.73 of // a tile due to how the padding calculations work. - height: 24px; + height: 28px; padding-top: 8px; // 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; From 0906da01baa1724df28814afc34d45d9e4fbeca9 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens <joriksch@gmail.com> Date: Wed, 8 Jul 2020 00:18:58 +0100 Subject: [PATCH 64/72] Fix gaps --- res/css/views/rooms/_RoomSublist2.scss | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index a8de6dd409..d08bc09031 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -203,10 +203,11 @@ limitations under the License. // Update the render() function for RoomSublist2 if these change // Update the ListLayout class for minVisibleTiles if these change. // - // At 28px high and 8px padding on the top this equates to 0.73 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: 28px; + 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. From c5e8a0b5afc30ed5b97867e826c1e3fc1847ab4d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Jul 2020 08:40:58 +0100 Subject: [PATCH 65/72] Convert HtmlUtils to TypeScript Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- package.json | 2 + src/{HtmlUtils.js => HtmlUtils.tsx} | 127 +++++++++++++++------------- yarn.lock | 14 +++ 3 files changed, 83 insertions(+), 60 deletions(-) rename src/{HtmlUtils.js => HtmlUtils.tsx} (84%) diff --git a/package.json b/package.json index 3fd7703afb..7251d76498 100644 --- a/package.json +++ b/package.json @@ -122,6 +122,7 @@ "@types/classnames": "^2.2.10", "@types/counterpart": "^0.18.1", "@types/flux": "^3.1.9", + "@types/linkifyjs": "^2.1.3", "@types/lodash": "^4.14.152", "@types/modernizr": "^3.5.3", "@types/node": "^12.12.41", @@ -129,6 +130,7 @@ "@types/react": "^16.9", "@types/react-dom": "^16.9.8", "@types/react-transition-group": "^4.4.0", + "@types/sanitize-html": "^1.23.3", "@types/zxcvbn": "^4.4.0", "babel-eslint": "^10.0.3", "babel-jest": "^24.9.0", diff --git a/src/HtmlUtils.js b/src/HtmlUtils.tsx similarity index 84% rename from src/HtmlUtils.js rename to src/HtmlUtils.tsx index 34e9e55d25..6746f68812 100644 --- a/src/HtmlUtils.js +++ b/src/HtmlUtils.tsx @@ -17,10 +17,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -'use strict'; - -import ReplyThread from "./components/views/elements/ReplyThread"; - import React from 'react'; import sanitizeHtml from 'sanitize-html'; import * as linkify from 'linkifyjs'; @@ -28,12 +24,13 @@ import linkifyMatrix from './linkify-matrix'; import _linkifyElement from 'linkifyjs/element'; import _linkifyString from 'linkifyjs/string'; import classNames from 'classnames'; -import {MatrixClientPeg} from './MatrixClientPeg'; +import EMOJIBASE_REGEX from 'emojibase-regex'; import url from 'url'; -import EMOJIBASE_REGEX from 'emojibase-regex'; +import {MatrixClientPeg} from './MatrixClientPeg'; import {tryTransformPermalinkToLocalHref} from "./utils/permalinks/Permalinks"; import {SHORTCODE_TO_EMOJI, getEmojiFromUnicode} from "./emoji"; +import ReplyThread from "./components/views/elements/ReplyThread"; linkifyMatrix(linkify); @@ -64,7 +61,7 @@ const PERMITTED_URL_SCHEMES = ['http', 'https', 'ftp', 'mailto', 'magnet']; * need emojification. * unicodeToImage uses this function. */ -function mightContainEmoji(str) { +function mightContainEmoji(str: string) { return SURROGATE_PAIR_PATTERN.test(str) || SYMBOL_PATTERN.test(str); } @@ -74,7 +71,7 @@ function mightContainEmoji(str) { * @param {String} char The emoji character * @return {String} The shortcode (such as :thumbup:) */ -export function unicodeToShortcode(char) { +export function unicodeToShortcode(char: string) { const data = getEmojiFromUnicode(char); return (data && data.shortcodes ? `:${data.shortcodes[0]}:` : ''); } @@ -85,7 +82,7 @@ export function unicodeToShortcode(char) { * @param {String} shortcode The shortcode (such as :thumbup:) * @return {String} The emoji character; null if none exists */ -export function shortcodeToUnicode(shortcode) { +export function shortcodeToUnicode(shortcode: string) { shortcode = shortcode.slice(1, shortcode.length - 1); const data = SHORTCODE_TO_EMOJI.get(shortcode); return data ? data.unicode : null; @@ -100,7 +97,7 @@ export function processHtmlForSending(html: string): string { } let contentHTML = ""; - for (let i=0; i < contentDiv.children.length; i++) { + for (let i = 0; i < contentDiv.children.length; i++) { const element = contentDiv.children[i]; if (element.tagName.toLowerCase() === 'p') { contentHTML += element.innerHTML; @@ -122,7 +119,7 @@ export function processHtmlForSending(html: string): string { * Given an untrusted HTML string, return a React node with an sanitized version * of that HTML. */ -export function sanitizedHtmlNode(insaneHtml) { +export function sanitizedHtmlNode(insaneHtml: string) { const saneHtml = sanitizeHtml(insaneHtml, sanitizeHtmlParams); return <div dangerouslySetInnerHTML={{ __html: saneHtml }} dir="auto" />; @@ -136,7 +133,7 @@ export function sanitizedHtmlNode(insaneHtml) { * other places we need to sanitise URLs. * @return true if permitted, otherwise false */ -export function isUrlPermitted(inputUrl) { +export function isUrlPermitted(inputUrl: string) { try { const parsed = url.parse(inputUrl); if (!parsed.protocol) return false; @@ -147,9 +144,9 @@ export function isUrlPermitted(inputUrl) { } } -const transformTags = { // custom to matrix +const transformTags: sanitizeHtml.IOptions["transformTags"] = { // custom to matrix // add blank targets to all hyperlinks except vector URLs - 'a': function(tagName, attribs) { + 'a': function(tagName: string, attribs: sanitizeHtml.Attributes) { if (attribs.href) { attribs.target = '_blank'; // by default @@ -162,7 +159,7 @@ const transformTags = { // custom to matrix attribs.rel = 'noreferrer noopener'; // https://mathiasbynens.github.io/rel-noopener/ return { tagName, attribs }; }, - 'img': function(tagName, attribs) { + 'img': function(tagName: string, attribs: sanitizeHtml.Attributes) { // Strip out imgs that aren't `mxc` here instead of using allowedSchemesByTag // because transformTags is used _before_ we filter by allowedSchemesByTag and // we don't want to allow images with `https?` `src`s. @@ -176,7 +173,7 @@ const transformTags = { // custom to matrix ); return { tagName, attribs }; }, - 'code': function(tagName, attribs) { + 'code': function(tagName: string, attribs: sanitizeHtml.Attributes) { if (typeof attribs.class !== 'undefined') { // Filter out all classes other than ones starting with language- for syntax highlighting. const classes = attribs.class.split(/\s/).filter(function(cl) { @@ -186,7 +183,7 @@ const transformTags = { // custom to matrix } return { tagName, attribs }; }, - '*': function(tagName, attribs) { + '*': function(tagName: string, attribs: sanitizeHtml.Attributes) { // Delete any style previously assigned, style is an allowedTag for font and span // because attributes are stripped after transforming delete attribs.style; @@ -220,7 +217,7 @@ const transformTags = { // custom to matrix }, }; -const sanitizeHtmlParams = { +const sanitizeHtmlParams: sanitizeHtml.IOptions = { allowedTags: [ 'font', // custom to matrix for IRC-style font coloring 'del', // for markdown @@ -247,16 +244,16 @@ const sanitizeHtmlParams = { }; // this is the same as the above except with less rewriting -const composerSanitizeHtmlParams = Object.assign({}, sanitizeHtmlParams); -composerSanitizeHtmlParams.transformTags = { - 'code': transformTags['code'], - '*': transformTags['*'], +const composerSanitizeHtmlParams: sanitizeHtml.IOptions = { + ...sanitizeHtmlParams, + transformTags: { + 'code': transformTags['code'], + '*': transformTags['*'], + }, }; -class BaseHighlighter { - constructor(highlightClass, highlightLink) { - this.highlightClass = highlightClass; - this.highlightLink = highlightLink; +abstract class BaseHighlighter<T extends React.ReactNode> { + constructor(public highlightClass: string, public highlightLink: string) { } /** @@ -270,47 +267,49 @@ class BaseHighlighter { * returns a list of results (strings for HtmlHighligher, react nodes for * TextHighlighter). */ - applyHighlights(safeSnippet, safeHighlights) { + public applyHighlights(safeSnippet: string, safeHighlights: string[]): T[] { let lastOffset = 0; let offset; - let nodes = []; + let nodes: T[] = []; const safeHighlight = safeHighlights[0]; while ((offset = safeSnippet.toLowerCase().indexOf(safeHighlight.toLowerCase(), lastOffset)) >= 0) { // handle preamble if (offset > lastOffset) { - var subSnippet = safeSnippet.substring(lastOffset, offset); - nodes = nodes.concat(this._applySubHighlights(subSnippet, safeHighlights)); + const subSnippet = safeSnippet.substring(lastOffset, offset); + nodes = nodes.concat(this.applySubHighlights(subSnippet, safeHighlights)); } // do highlight. use the original string rather than safeHighlight // to preserve the original casing. const endOffset = offset + safeHighlight.length; - nodes.push(this._processSnippet(safeSnippet.substring(offset, endOffset), true)); + nodes.push(this.processSnippet(safeSnippet.substring(offset, endOffset), true)); lastOffset = endOffset; } // handle postamble if (lastOffset !== safeSnippet.length) { - subSnippet = safeSnippet.substring(lastOffset, undefined); - nodes = nodes.concat(this._applySubHighlights(subSnippet, safeHighlights)); + const subSnippet = safeSnippet.substring(lastOffset, undefined); + nodes = nodes.concat(this.applySubHighlights(subSnippet, safeHighlights)); } return nodes; } - _applySubHighlights(safeSnippet, safeHighlights) { + private applySubHighlights(safeSnippet: string, safeHighlights: string[]): T[] { if (safeHighlights[1]) { // recurse into this range to check for the next set of highlight matches return this.applyHighlights(safeSnippet, safeHighlights.slice(1)); } else { // no more highlights to be found, just return the unhighlighted string - return [this._processSnippet(safeSnippet, false)]; + return [this.processSnippet(safeSnippet, false)]; } } + + protected abstract processSnippet(snippet: string, highlight: boolean): T; } -class HtmlHighlighter extends BaseHighlighter { +class HtmlHighlighter extends BaseHighlighter<string> { /* highlight the given snippet if required * * snippet: content of the span; must have been sanitised @@ -318,28 +317,23 @@ class HtmlHighlighter extends BaseHighlighter { * * returns an HTML string */ - _processSnippet(snippet, highlight) { + protected processSnippet(snippet: string, highlight: boolean): string { if (!highlight) { // nothing required here return snippet; } - let span = "<span class=\""+this.highlightClass+"\">" - + snippet + "</span>"; + let span = `<span class="${this.highlightClass}">${snippet}</span>`; if (this.highlightLink) { - span = "<a href=\""+encodeURI(this.highlightLink)+"\">" - +span+"</a>"; + span = `<a href="${encodeURI(this.highlightLink)}">${span}</a>`; } return span; } } -class TextHighlighter extends BaseHighlighter { - constructor(highlightClass, highlightLink) { - super(highlightClass, highlightLink); - this._key = 0; - } +class TextHighlighter extends BaseHighlighter<React.ReactNode> { + private key = 0; /* create a <span> node to hold the given content * @@ -348,13 +342,12 @@ class TextHighlighter extends BaseHighlighter { * * returns a React node */ - _processSnippet(snippet, highlight) { - const key = this._key++; + protected processSnippet(snippet: string, highlight: boolean): React.ReactNode { + const key = this.key++; - let node = - <span key={key} className={highlight ? this.highlightClass : null}> - { snippet } - </span>; + let node = <span key={key} className={highlight ? this.highlightClass : null}> + { snippet } + </span>; if (highlight && this.highlightLink) { node = <a key={key} href={this.highlightLink}>{ node }</a>; @@ -364,6 +357,20 @@ class TextHighlighter extends BaseHighlighter { } } +interface IContent { + format?: string; + formatted_body?: string; + body: string; +} + +interface IOpts { + highlightLink?: string; + disableBigEmoji?: boolean; + stripReplyFallback?: boolean; + returnString?: boolean; + forComposerQuote?: boolean; + ref?: React.Ref<any>; +} /* turn a matrix event body into html * @@ -378,7 +385,7 @@ class TextHighlighter extends BaseHighlighter { * opts.forComposerQuote: optional param to lessen the url rewriting done by sanitization, for quoting into composer * opts.ref: React ref to attach to any React components returned (not compatible with opts.returnString) */ -export function bodyToHtml(content, highlights, opts={}) { +export function bodyToHtml(content: IContent, highlights: string[], opts: IOpts = {}) { const isHtmlMessage = content.format === "org.matrix.custom.html" && content.formatted_body; let bodyHasEmoji = false; @@ -387,9 +394,9 @@ export function bodyToHtml(content, highlights, opts={}) { sanitizeParams = composerSanitizeHtmlParams; } - let strippedBody; - let safeBody; - let isDisplayedWithHtml; + let strippedBody: string; + let safeBody: string; + let isDisplayedWithHtml: boolean; // XXX: We sanitize the HTML whilst also highlighting its text nodes, to avoid accidentally trying // to highlight HTML tags themselves. However, this does mean that we don't highlight textnodes which // are interrupted by HTML tags (not that we did before) - e.g. foo<span/>bar won't get highlighted @@ -471,7 +478,7 @@ export function bodyToHtml(content, highlights, opts={}) { * @param {object} [options] Options for linkifyString. Default: linkifyMatrix.options * @returns {string} Linkified string */ -export function linkifyString(str, options = linkifyMatrix.options) { +export function linkifyString(str: string, options = linkifyMatrix.options) { return _linkifyString(str, options); } @@ -482,7 +489,7 @@ export function linkifyString(str, options = linkifyMatrix.options) { * @param {object} [options] Options for linkifyElement. Default: linkifyMatrix.options * @returns {object} */ -export function linkifyElement(element, options = linkifyMatrix.options) { +export function linkifyElement(element: HTMLElement, options = linkifyMatrix.options) { return _linkifyElement(element, options); } @@ -493,7 +500,7 @@ export function linkifyElement(element, options = linkifyMatrix.options) { * @param {object} [options] Options for linkifyString. Default: linkifyMatrix.options * @returns {string} */ -export function linkifyAndSanitizeHtml(dirtyHtml, options = linkifyMatrix.options) { +export function linkifyAndSanitizeHtml(dirtyHtml: string, options = linkifyMatrix.options) { return sanitizeHtml(linkifyString(dirtyHtml, options), sanitizeHtmlParams); } @@ -504,7 +511,7 @@ export function linkifyAndSanitizeHtml(dirtyHtml, options = linkifyMatrix.option * @param {Node} node * @returns {bool} */ -export function checkBlockNode(node) { +export function checkBlockNode(node: Node) { switch (node.nodeName) { case "H1": case "H2": diff --git a/yarn.lock b/yarn.lock index d8106febab..972891f4ca 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1308,6 +1308,13 @@ resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.4.tgz#38fd73ddfd9b55abb1e1b2ed578cb55bd7b7d339" integrity sha512-8+KAKzEvSUdeo+kmqnKrqgeE+LcA0tjYWFY7RPProVYwnqDjukzO+3b6dLD56rYX5TdWejnEOLJYOIeh4CXKuA== +"@types/linkifyjs@^2.1.3": + version "2.1.3" + resolved "https://registry.yarnpkg.com/@types/linkifyjs/-/linkifyjs-2.1.3.tgz#80195c3c88c5e75d9f660e3046ce4a42be2c2fa4" + integrity sha512-V3Xt9wgaOvDPXcpOy3dC8qXCxy3cs0Lr/Hqgd9Bi6m3sf/vpbpTtfmVR0LJklrqYEjaAmc7e3Xh/INT2rCAKjQ== + dependencies: + "@types/react" "*" + "@types/lodash@^4.14.152": version "4.14.155" resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.155.tgz#e2b4514f46a261fd11542e47519c20ebce7bc23a" @@ -1372,6 +1379,13 @@ "@types/prop-types" "*" csstype "^2.2.0" +"@types/sanitize-html@^1.23.3": + version "1.23.3" + resolved "https://registry.yarnpkg.com/@types/sanitize-html/-/sanitize-html-1.23.3.tgz#26527783aba3bf195ad8a3c3e51bd3713526fc0d" + integrity sha512-Isg8N0ifKdDq6/kaNlIcWfapDXxxquMSk2XC5THsOICRyOIhQGds95XH75/PL/g9mExi4bL8otIqJM/Wo96WxA== + dependencies: + htmlparser2 "^4.1.0" + "@types/stack-utils@^1.0.1": version "1.0.1" resolved "https://registry.yarnpkg.com/@types/stack-utils/-/stack-utils-1.0.1.tgz#0a851d3bd96498fa25c33ab7278ed3bd65f06c3e" From 8d5d3b1c926da3246119e8c6f8d51fda6580825c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Jul 2020 08:50:25 +0100 Subject: [PATCH 66/72] Use html innerText for org.matrix.custom.html m.room.message room list previews Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/HtmlUtils.tsx | 7 +++++++ .../room-list/previews/MessageEventPreview.ts | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/HtmlUtils.tsx b/src/HtmlUtils.tsx index 6746f68812..6dba041685 100644 --- a/src/HtmlUtils.tsx +++ b/src/HtmlUtils.tsx @@ -125,6 +125,13 @@ export function sanitizedHtmlNode(insaneHtml: string) { return <div dangerouslySetInnerHTML={{ __html: saneHtml }} dir="auto" />; } +export function sanitizedHtmlNodeInnerText(insaneHtml: string) { + const saneHtml = sanitizeHtml(insaneHtml, sanitizeHtmlParams); + const contentDiv = document.createElement("div"); + contentDiv.innerHTML = saneHtml; + return contentDiv.innerText; +} + /** * Tests if a URL from an untrusted source may be safely put into the DOM * The biggest threat here is javascript: URIs. diff --git a/src/stores/room-list/previews/MessageEventPreview.ts b/src/stores/room-list/previews/MessageEventPreview.ts index 86ec4c539b..86cb51ef15 100644 --- a/src/stores/room-list/previews/MessageEventPreview.ts +++ b/src/stores/room-list/previews/MessageEventPreview.ts @@ -20,6 +20,7 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { _t } from "../../../languageHandler"; import { getSenderName, isSelf, shouldPrefixMessagesIn } from "./utils"; import ReplyThread from "../../../components/views/elements/ReplyThread"; +import { sanitizedHtmlNodeInnerText } from "../../../HtmlUtils"; export class MessageEventPreview implements IPreview { public getTextFor(event: MatrixEvent, tagId?: TagID): string { @@ -36,14 +37,27 @@ export class MessageEventPreview implements IPreview { const msgtype = eventContent['msgtype']; if (!body || !msgtype) return null; // invalid event, no preview + const hasHtml = eventContent.format === "org.matrix.custom.html" && eventContent.formatted_body; + if (hasHtml) { + body = eventContent.formatted_body; + } + // XXX: Newer relations have a getRelation() function which is not compatible with replies. const mRelatesTo = event.getWireContent()['m.relates_to']; if (mRelatesTo && mRelatesTo['m.in_reply_to']) { // If this is a reply, get the real reply and use that - body = (ReplyThread.stripPlainReply(body) || '').trim(); + if (hasHtml) { + body = (ReplyThread.stripHTMLReply(body) || '').trim(); + } else { + body = (ReplyThread.stripPlainReply(body) || '').trim(); + } if (!body) return null; // invalid event, no preview } + if (hasHtml) { + body = sanitizedHtmlNodeInnerText(body); + } + if (msgtype === 'm.emote') { return _t("%(senderName)s %(emote)s", {senderName: getSenderName(event), emote: body}); } From 7b115056b0ec64dba01f1758aa6aab7c88b418b0 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Jul 2020 09:21:33 +0100 Subject: [PATCH 67/72] Fix sticky headers being left on display:none if they change too quickly Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/LeftPanel2.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 7fac6cbff1..e67a85db25 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -146,6 +146,7 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { const slRect = sublist.getBoundingClientRect(); const header = sublist.querySelector<HTMLDivElement>(".mx_RoomSublist2_stickable"); + header.style.removeProperty("display"); // always clear display:none first if (slRect.top + headerHeight > bottom && !gotBottom) { header.classList.add("mx_RoomSublist2_headerContainer_sticky"); @@ -161,8 +162,6 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { if (lastTopHeader) { lastTopHeader.style.display = "none"; } - // first unset it, if set in last iteration - header.style.removeProperty("display"); lastTopHeader = header; } else { header.classList.remove("mx_RoomSublist2_headerContainer_sticky"); From ec54d509e52e225e47406b3ed52085a00c3f0d5c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Jul 2020 13:24:40 +0100 Subject: [PATCH 68/72] remove stale debug log Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomTile2.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 90edbfb895..67d7ae34ba 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -170,7 +170,6 @@ export default class RoomTile2 extends React.Component<IProps, IState> { }; private scrollIntoView = () => { - console.log("DEBUG scrollIntoView", this.roomTileRef.current); if (!this.roomTileRef.current) return; this.roomTileRef.current.scrollIntoView({ block: "nearest", From 75751abc60d9b75438c1a55d00053e5f10e40008 Mon Sep 17 00:00:00 2001 From: Bruno Windels <brunow@matrix.org> Date: Wed, 8 Jul 2020 14:49:04 +0200 Subject: [PATCH 69/72] add wrapper we can then add padding to when sticking headers --- res/css/structures/_LeftPanel2.scss | 8 ++++++++ src/components/structures/LeftPanel2.tsx | 20 +++++++++++--------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/res/css/structures/_LeftPanel2.scss b/res/css/structures/_LeftPanel2.scss index a73658d916..57f690346b 100644 --- a/res/css/structures/_LeftPanel2.scss +++ b/res/css/structures/_LeftPanel2.scss @@ -121,6 +121,14 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations } } + .mx_LeftPanel2_roomListWrapper { + display: flex; + flex-grow: 1; + overflow: hidden; + min-height: 0; + + } + .mx_LeftPanel2_actualRoomListContainer { flex-grow: 1; // fill the available space overflow-y: auto; diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 7fac6cbff1..037c3bd4ff 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -325,15 +325,17 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { <aside className="mx_LeftPanel2_roomListContainer"> {this.renderHeader()} {this.renderSearchExplore()} - <div - className={roomListClasses} - onScroll={this.onScroll} - ref={this.listContainerRef} - // Firefox sometimes makes this element focusable due to - // overflow:scroll;, so force it out of tab order. - tabIndex={-1} - > - {roomList} + <div className="mx_LeftPanel2_roomListWrapper"> + <div + className={roomListClasses} + onScroll={this.onScroll} + ref={this.listContainerRef} + // Firefox sometimes makes this element focusable due to + // overflow:scroll;, so force it out of tab order. + tabIndex={-1} + > + {roomList} + </div> </div> </aside> </div> From 0d94cfa97ad7971d43332709e5023d7d4a6cc894 Mon Sep 17 00:00:00 2001 From: Bruno Windels <brunow@matrix.org> Date: Wed, 8 Jul 2020 14:49:38 +0200 Subject: [PATCH 70/72] put sticky headers in padding of wrapper this way they don't need a background, as the list is already clipped --- res/css/structures/_LeftPanel2.scss | 7 ++++++ src/components/structures/LeftPanel2.tsx | 29 ++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/res/css/structures/_LeftPanel2.scss b/res/css/structures/_LeftPanel2.scss index 57f690346b..eaa22a3efa 100644 --- a/res/css/structures/_LeftPanel2.scss +++ b/res/css/structures/_LeftPanel2.scss @@ -127,6 +127,13 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations overflow: hidden; min-height: 0; + &.stickyBottom { + padding-bottom: 32px; + } + + &.stickyTop { + padding-top: 32px; + } } .mx_LeftPanel2_actualRoomListContainer { diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 037c3bd4ff..51dc4c0c4c 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -153,11 +153,16 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { header.style.width = `${headerStickyWidth}px`; header.style.removeProperty("top"); gotBottom = true; - } else if ((slRect.top - (headerHeight / 3)) < top) { + } else if (((slRect.top - (headerHeight * 0.6) + headerHeight) < top) || sublist === sublists[0]) { + // the header should become sticky once it is 60% or less out of view at the top. + // We also add headerHeight because the sticky header is put above the scrollable area, + // into the padding of .mx_LeftPanel2_roomListWrapper, + // by subtracting headerHeight from the top below. + // We also always try to make the first sublist header sticky. header.classList.add("mx_RoomSublist2_headerContainer_sticky"); header.classList.add("mx_RoomSublist2_headerContainer_stickyTop"); header.style.width = `${headerStickyWidth}px`; - header.style.top = `${rlRect.top}px`; + header.style.top = `${rlRect.top - headerHeight}px`; if (lastTopHeader) { lastTopHeader.style.display = "none"; } @@ -172,6 +177,26 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { header.style.removeProperty("top"); } } + + // add appropriate sticky classes to wrapper so it has + // the necessary top/bottom padding to put the sticky header in + const listWrapper = list.parentElement; + if (gotBottom) { + listWrapper.classList.add("stickyBottom"); + } else { + listWrapper.classList.remove("stickyBottom"); + } + if (lastTopHeader) { + listWrapper.classList.add("stickyTop"); + } else { + listWrapper.classList.remove("stickyTop"); + } + + // ensure scroll doesn't go above the gap left by the header of + // the first sublist always being sticky if no other header is sticky + if (list.scrollTop < headerHeight) { + list.scrollTop = headerHeight; + } } // TODO: Improve header reliability: https://github.com/vector-im/riot-web/issues/14232 From a8085f4e3bcd3e0967f3e68b5f2b63db322abac4 Mon Sep 17 00:00:00 2001 From: Bruno Windels <brunow@matrix.org> Date: Wed, 8 Jul 2020 14:50:08 +0200 Subject: [PATCH 71/72] remove background on sticky headers --- res/css/views/rooms/_RoomSublist2.scss | 3 --- 1 file changed, 3 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index d08bc09031..194e615099 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -54,9 +54,6 @@ limitations under the License. max-width: 100%; z-index: 2; // Prioritize headers in the visible list over sticky ones - // Set the same background color as the room list for sticky headers - background-color: $roomlist2-bg-color; - // Create a flexbox to make ordering easy display: flex; align-items: center; From a361ac3f83b42a4e3cea2608aa4ea91b8760cc99 Mon Sep 17 00:00:00 2001 From: Bruno Windels <brunow@matrix.org> Date: Wed, 8 Jul 2020 15:11:47 +0200 Subject: [PATCH 72/72] make collapsing/expanding the first header work again --- src/components/structures/LeftPanel2.tsx | 16 ++++++++-------- src/components/views/rooms/RoomSublist2.tsx | 7 ++++++- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 51dc4c0c4c..6da70ed0ae 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -21,6 +21,7 @@ import classNames from "classnames"; import dis from "../../dispatcher/dispatcher"; import { _t } from "../../languageHandler"; import RoomList2 from "../views/rooms/RoomList2"; +import { HEADER_HEIGHT } from "../views/rooms/RoomSublist2"; import { Action } from "../../dispatcher/actions"; import UserMenu from "./UserMenu"; import RoomSearch from "./RoomSearch"; @@ -135,7 +136,6 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { const bottom = rlRect.bottom; const top = rlRect.top; const sublists = list.querySelectorAll<HTMLDivElement>(".mx_RoomSublist2"); - const headerHeight = 32; // Note: must match the CSS! const headerRightMargin = 24; // calculated from margins and widths to align with non-sticky tiles const headerStickyWidth = rlRect.width - headerRightMargin; @@ -147,22 +147,22 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { const header = sublist.querySelector<HTMLDivElement>(".mx_RoomSublist2_stickable"); - if (slRect.top + headerHeight > bottom && !gotBottom) { + if (slRect.top + HEADER_HEIGHT > bottom && !gotBottom) { header.classList.add("mx_RoomSublist2_headerContainer_sticky"); header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); header.style.width = `${headerStickyWidth}px`; header.style.removeProperty("top"); gotBottom = true; - } else if (((slRect.top - (headerHeight * 0.6) + headerHeight) < top) || sublist === sublists[0]) { + } else if (((slRect.top - (HEADER_HEIGHT * 0.6) + HEADER_HEIGHT) < top) || sublist === sublists[0]) { // the header should become sticky once it is 60% or less out of view at the top. - // We also add headerHeight because the sticky header is put above the scrollable area, + // We also add HEADER_HEIGHT because the sticky header is put above the scrollable area, // into the padding of .mx_LeftPanel2_roomListWrapper, - // by subtracting headerHeight from the top below. + // by subtracting HEADER_HEIGHT from the top below. // We also always try to make the first sublist header sticky. header.classList.add("mx_RoomSublist2_headerContainer_sticky"); header.classList.add("mx_RoomSublist2_headerContainer_stickyTop"); header.style.width = `${headerStickyWidth}px`; - header.style.top = `${rlRect.top - headerHeight}px`; + header.style.top = `${rlRect.top - HEADER_HEIGHT}px`; if (lastTopHeader) { lastTopHeader.style.display = "none"; } @@ -194,8 +194,8 @@ export default class LeftPanel2 extends React.Component<IProps, IState> { // ensure scroll doesn't go above the gap left by the header of // the first sublist always being sticky if no other header is sticky - if (list.scrollTop < headerHeight) { - list.scrollTop = headerHeight; + if (list.scrollTop < HEADER_HEIGHT) { + list.scrollTop = HEADER_HEIGHT; } } diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index eefd29f0b7..9a97aac320 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -55,6 +55,7 @@ import StyledCheckbox from "../elements/StyledCheckbox"; const SHOW_N_BUTTON_HEIGHT = 32; // As defined by CSS const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS +export const HEADER_HEIGHT = 32; // As defined by CSS const MAX_PADDING_HEIGHT = SHOW_N_BUTTON_HEIGHT + RESIZE_HANDLE_HEIGHT; @@ -233,7 +234,11 @@ export default class RoomSublist2 extends React.Component<IProps, IState> { const possibleSticky = target.parentElement; const sublist = possibleSticky.parentElement.parentElement; - if (possibleSticky.classList.contains('mx_RoomSublist2_headerContainer_sticky')) { + const list = sublist.parentElement.parentElement; + // the scrollTop is capped at the height of the header in LeftPanel2 + const isAtTop = list.scrollTop <= HEADER_HEIGHT; + const isSticky = possibleSticky.classList.contains('mx_RoomSublist2_headerContainer_sticky'); + if (isSticky && !isAtTop) { // is sticky - jump to list sublist.scrollIntoView({behavior: 'smooth'}); } else {