From 111d4adab2b87c5ed019b4aff6bd7df85c61f1b1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jul 2020 22:38:11 +0100 Subject: [PATCH 01/38] Convert createRoom over to typescript Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/{createRoom.js => createRoom.ts} | 68 +++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 7 deletions(-) rename src/{createRoom.js => createRoom.ts} (81%) diff --git a/src/createRoom.js b/src/createRoom.ts similarity index 81% rename from src/createRoom.js rename to src/createRoom.ts index affdf196a7..b863450065 100644 --- a/src/createRoom.js +++ b/src/createRoom.ts @@ -15,6 +15,9 @@ See the License for the specific language governing permissions and limitations under the License. */ +import {MatrixClient} from "matrix-js-sdk/src/client"; +import {Room} from "matrix-js-sdk/src/models/Room"; + import {MatrixClientPeg} from './MatrixClientPeg'; import Modal from './Modal'; import * as sdk from './index'; @@ -26,6 +29,56 @@ import {getAddressType} from "./UserAddress"; const E2EE_WK_KEY = "im.vector.riot.e2ee"; +// TODO move these interfaces over to js-sdk once it has been typescripted enough to accept them +enum Visibility { + Public = "public", + Private = "private", +} + +enum Preset { + PrivateChat = "private_chat", + TrustedPrivateChat = "trusted_private_chat", + PublicChat = "public_chat", +} + +interface Invite3PID { + id_server: string; + id_access_token?: string; // this gets injected by the js-sdk + medium: string; + address: string; +} + +interface IStateEvent { + type: string; + state_key?: string; // defaults to an empty string + content: object; +} + +interface ICreateOpts { + visibility?: Visibility; + room_alias_name?: string; + name?: string; + topic?: string; + invite?: string[]; + invite_3pid?: Invite3PID[]; + room_version?: string; + creation_content?: object; + initial_state?: IStateEvent[]; + preset?: Preset; + is_direct?: boolean; + power_level_content_override?: object; +} + +interface IOpts { + dmUserId?: string; + createOpts?: ICreateOpts; + spinner?: boolean; + guestAccess?: boolean; + encryption?: boolean; + inlineErrors?: boolean; + andView?: boolean; +} + /** * Create a new room, and switch to it. * @@ -40,11 +93,12 @@ const E2EE_WK_KEY = "im.vector.riot.e2ee"; * Default: False * @param {bool=} opts.inlineErrors True to raise errors off the promise instead of resolving to null. * Default: False + * @param {bool=} opts.andView True to dispatch an action to view the room once it has been created. * * @returns {Promise} which resolves to the room id, or null if the * action was aborted or failed. */ -export default function createRoom(opts) { +export default function createRoom(opts: IOpts): Promise { opts = opts || {}; if (opts.spinner === undefined) opts.spinner = true; if (opts.guestAccess === undefined) opts.guestAccess = true; @@ -59,12 +113,12 @@ export default function createRoom(opts) { return Promise.resolve(null); } - const defaultPreset = opts.dmUserId ? 'trusted_private_chat' : 'private_chat'; + const defaultPreset = opts.dmUserId ? Preset.TrustedPrivateChat : Preset.PrivateChat; // set some defaults for the creation const createOpts = opts.createOpts || {}; createOpts.preset = createOpts.preset || defaultPreset; - createOpts.visibility = createOpts.visibility || 'private'; + createOpts.visibility = createOpts.visibility || Visibility.Private; if (opts.dmUserId && createOpts.invite === undefined) { switch (getAddressType(opts.dmUserId)) { case 'mx-user-id': @@ -166,7 +220,7 @@ export default function createRoom(opts) { }); } -export function findDMForUser(client, userId) { +export function findDMForUser(client: MatrixClient, userId: string): Room { const roomIds = DMRoomMap.shared().getDMRoomsForUserId(userId); const rooms = roomIds.map(id => client.getRoom(id)); const suitableDMRooms = rooms.filter(r => { @@ -189,7 +243,7 @@ export function findDMForUser(client, userId) { * NOTE: this assumes you've just created the room and there's not been an opportunity * for other code to run, so we shouldn't miss RoomState.newMember when it comes by. */ -export async function _waitForMember(client, roomId, userId, opts = { timeout: 1500 }) { +export async function _waitForMember(client: MatrixClient, roomId: string, userId: string, opts = { timeout: 1500 }) { const { timeout } = opts; let handler; return new Promise((resolve) => { @@ -212,7 +266,7 @@ export async function _waitForMember(client, roomId, userId, opts = { timeout: 1 * Ensure that for every user in a room, there is at least one device that we * can encrypt to. */ -export async function canEncryptToAllUsers(client, userIds) { +export async function canEncryptToAllUsers(client: MatrixClient, userIds: string[]) { const usersDeviceMap = await client.downloadKeys(userIds); // { "@user:host": { "DEVICE": {...}, ... }, ... } return Object.values(usersDeviceMap).every((userDevices) => @@ -221,7 +275,7 @@ export async function canEncryptToAllUsers(client, userIds) { ); } -export async function ensureDMExists(client, userId) { +export async function ensureDMExists(client: MatrixClient, userId: string): Promise { const existingDMRoom = findDMForUser(client, userId); let roomId; if (existingDMRoom) { From 97711032d8bb94848fabe959edc3ccf7daff2df9 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jul 2020 22:38:21 +0100 Subject: [PATCH 02/38] Fix signature of sleep utility Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/utils/promise.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/promise.ts b/src/utils/promise.ts index c5c1cb9a56..d3ae2c3d1b 100644 --- a/src/utils/promise.ts +++ b/src/utils/promise.ts @@ -15,7 +15,7 @@ limitations under the License. */ // Returns a promise which resolves with a given value after the given number of ms -export function sleep(ms: number, value: T): Promise { +export function sleep(ms: number, value?: T): Promise { return new Promise((resolve => { setTimeout(resolve, ms, value); })); } From 7322aaf6023784ae2bb1dbd556a911611bcd0af6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jul 2020 22:42:28 +0100 Subject: [PATCH 03/38] Convert PlatformPeg to Typescript Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/@types/global.d.ts | 2 ++ src/{PlatformPeg.js => PlatformPeg.ts} | 15 ++++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) rename src/{PlatformPeg.js => PlatformPeg.ts} (82%) diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index ffd3277892..8486016cd0 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -20,6 +20,7 @@ import { IMatrixClientPeg } from "../MatrixClientPeg"; import ToastStore from "../stores/ToastStore"; import DeviceListener from "../DeviceListener"; import { RoomListStore2 } from "../stores/room-list/RoomListStore2"; +import { PlatformPeg } from "../PlatformPeg"; declare global { interface Window { @@ -33,6 +34,7 @@ declare global { mx_ToastStore: ToastStore; mx_DeviceListener: DeviceListener; mx_RoomListStore2: RoomListStore2; + mxPlatformPeg: PlatformPeg; } // workaround for https://github.com/microsoft/TypeScript/issues/30933 diff --git a/src/PlatformPeg.js b/src/PlatformPeg.ts similarity index 82% rename from src/PlatformPeg.js rename to src/PlatformPeg.ts index 34131fde7d..42cb7acaf7 100644 --- a/src/PlatformPeg.js +++ b/src/PlatformPeg.ts @@ -1,5 +1,6 @@ /* Copyright 2016 OpenMarket Ltd +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. @@ -14,6 +15,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +import BasePlatform from "./BasePlatform"; + /* * Holds the current Platform object used by the code to do anything * specific to the platform we're running on (eg. web, electron) @@ -21,10 +24,8 @@ limitations under the License. * This allows the app layer to set a Platform without necessarily * having to have a MatrixChat object */ -class PlatformPeg { - constructor() { - this.platform = null; - } +export class PlatformPeg { + platform: BasePlatform = null; /** * Returns the current Platform object for the application. @@ -44,7 +45,7 @@ class PlatformPeg { } } -if (!global.mxPlatformPeg) { - global.mxPlatformPeg = new PlatformPeg(); +if (!window.mxPlatformPeg) { + window.mxPlatformPeg = new PlatformPeg(); } -export default global.mxPlatformPeg; +export default window.mxPlatformPeg; From 1ab0a1a1defc1c75c54302ef5ac51c4b71d5ddf0 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jul 2020 23:14:31 +0100 Subject: [PATCH 04/38] First step towards a11y in the new room list Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/BasePlatform.ts | 4 ++++ src/PlatformPeg.ts | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index 1d11495e61..acf72a986c 100644 --- a/src/BasePlatform.ts +++ b/src/BasePlatform.ts @@ -53,6 +53,10 @@ export default abstract class BasePlatform { this.startUpdateCheck = this.startUpdateCheck.bind(this); } + abstract async getConfig(): Promise<{}>; + + abstract getDefaultDeviceDisplayName(): string; + protected onAction = (payload: ActionPayload) => { switch (payload.action) { case 'on_client_not_viable': diff --git a/src/PlatformPeg.ts b/src/PlatformPeg.ts index 42cb7acaf7..1d2b813ebc 100644 --- a/src/PlatformPeg.ts +++ b/src/PlatformPeg.ts @@ -40,7 +40,7 @@ export class PlatformPeg { * application. * This should be an instance of a class extending BasePlatform. */ - set(plaf) { + set(plaf: BasePlatform) { this.platform = plaf; } } From 48ce294a497211eff1405b47ca1fc2219857b0db Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jul 2020 23:15:08 +0100 Subject: [PATCH 05/38] Transition languageHandler to Typescript Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- package.json | 1 + src/@types/global.d.ts | 4 ++ src/components/views/toasts/GenericToast.tsx | 4 +- ...languageHandler.js => languageHandler.tsx} | 72 ++++++++++++------- yarn.lock | 5 ++ 5 files changed, 57 insertions(+), 29 deletions(-) rename src/{languageHandler.js => languageHandler.tsx} (87%) diff --git a/package.json b/package.json index e8719520c4..a79a0467ff 100644 --- a/package.json +++ b/package.json @@ -120,6 +120,7 @@ "@babel/register": "^7.7.4", "@peculiar/webcrypto": "^1.0.22", "@types/classnames": "^2.2.10", + "@types/counterpart": "^0.18.1", "@types/flux": "^3.1.9", "@types/lodash": "^4.14.152", "@types/modernizr": "^3.5.3", diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index 8486016cd0..2c2fec759c 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -47,6 +47,10 @@ declare global { hasStorageAccess?: () => Promise; } + interface Navigator { + userLanguage?: string; + } + interface StorageEstimate { usageDetails?: {[key: string]: number}; } diff --git a/src/components/views/toasts/GenericToast.tsx b/src/components/views/toasts/GenericToast.tsx index 9f8885ba47..6cd881b9eb 100644 --- a/src/components/views/toasts/GenericToast.tsx +++ b/src/components/views/toasts/GenericToast.tsx @@ -14,13 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, {ReactChild} from "react"; +import React, {ReactNode} from "react"; import FormButton from "../elements/FormButton"; import {XOR} from "../../../@types/common"; export interface IProps { - description: ReactChild; + description: ReactNode; acceptLabel: string; onAccept(); diff --git a/src/languageHandler.js b/src/languageHandler.tsx similarity index 87% rename from src/languageHandler.js rename to src/languageHandler.tsx index 79a172015a..91d90d4e6c 100644 --- a/src/languageHandler.js +++ b/src/languageHandler.tsx @@ -1,7 +1,7 @@ /* Copyright 2017 MTRNord and Cooperative EITA Copyright 2017 Vector Creations 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"); @@ -20,10 +20,11 @@ limitations under the License. import request from 'browser-request'; import counterpart from 'counterpart'; import React from 'react'; + import SettingsStore, {SettingLevel} from "./settings/SettingsStore"; import PlatformPeg from "./PlatformPeg"; -// $webapp is a webpack resolve alias pointing to the output directory, see webpack config +// @ts-ignore - $webapp is a webpack resolve alias pointing to the output directory, see webpack config import webpackLangJsonUrl from "$webapp/i18n/languages.json"; const i18nFolder = 'i18n/'; @@ -37,27 +38,31 @@ counterpart.setSeparator('|'); // Fall back to English counterpart.setFallbackLocale('en'); +interface ITranslatableError extends Error { + translatedMessage: string; +} + /** * Helper function to create an error which has an English message * with a translatedMessage property for use by the consumer. * @param {string} message Message to translate. * @returns {Error} The constructed error. */ -export function newTranslatableError(message) { - const error = new Error(message); +export function newTranslatableError(message: string) { + const error = new Error(message) as ITranslatableError; error.translatedMessage = _t(message); return error; } // Function which only purpose is to mark that a string is translatable // Does not actually do anything. It's helpful for automatic extraction of translatable strings -export function _td(s) { +export function _td(s: string): string { return s; } // Wrapper for counterpart's translation function so that it handles nulls and undefineds properly // Takes the same arguments as counterpart.translate() -function safeCounterpartTranslate(text, options) { +function safeCounterpartTranslate(text: string, options?: object) { // Horrible hack to avoid https://github.com/vector-im/riot-web/issues/4191 // The interpolation library that counterpart uses does not support undefined/null // values and instead will throw an error. This is a problem since everywhere else @@ -89,6 +94,13 @@ function safeCounterpartTranslate(text, options) { return translated; } +interface IVariables { + count?: number; + [key: string]: number | string; +} + +type Tags = Record React.ReactNode>; + /* * Translates text and optionally also replaces XML-ish elements in the text with e.g. React components * @param {string} text The untranslated text, e.g "click here now to %(foo)s". @@ -105,7 +117,9 @@ function safeCounterpartTranslate(text, options) { * * @return a React component if any non-strings were used in substitutions, otherwise a string */ -export function _t(text, variables, tags) { +export function _t(text: string, variables?: IVariables): string; +export function _t(text: string, variables: IVariables, tags: Tags): React.ReactNode; +export function _t(text: string, variables?: IVariables, tags?: Tags): string | React.ReactNode { // Don't do substitutions in counterpart. We handle it ourselves so we can replace with React components // However, still pass the variables to counterpart so that it can choose the correct plural if count is given // It is enough to pass the count variable, but in the future counterpart might make use of other information too @@ -141,23 +155,25 @@ export function _t(text, variables, tags) { * * @return a React component if any non-strings were used in substitutions, otherwise a string */ -export function substitute(text, variables, tags) { - let result = text; +export function substitute(text: string, variables?: IVariables): string; +export function substitute(text: string, variables: IVariables, tags: Tags): string; +export function substitute(text: string, variables?: IVariables, tags?: Tags): string | React.ReactNode { + let result: React.ReactNode | string = text; if (variables !== undefined) { - const regexpMapping = {}; + const regexpMapping: IVariables = {}; for (const variable in variables) { regexpMapping[`%\\(${variable}\\)s`] = variables[variable]; } - result = replaceByRegexes(result, regexpMapping); + result = replaceByRegexes(result as string, regexpMapping); } if (tags !== undefined) { - const regexpMapping = {}; + const regexpMapping: Tags = {}; for (const tag in tags) { regexpMapping[`(<${tag}>(.*?)<\\/${tag}>|<${tag}>|<${tag}\\s*\\/>)`] = tags[tag]; } - result = replaceByRegexes(result, regexpMapping); + result = replaceByRegexes(result as string, regexpMapping); } return result; @@ -172,7 +188,9 @@ export function substitute(text, variables, tags) { * * @return a React component if any non-strings were used in substitutions, otherwise a string */ -export function replaceByRegexes(text, mapping) { +export function replaceByRegexes(text: string, mapping: IVariables): string; +export function replaceByRegexes(text: string, mapping: Tags): React.ReactNode; +export function replaceByRegexes(text: string, mapping: IVariables | Tags): string | React.ReactNode { // We initially store our output as an array of strings and objects (e.g. React components). // This will then be converted to a string or a at the end const output = [text]; @@ -189,7 +207,7 @@ export function replaceByRegexes(text, mapping) { // and everything after the match. Insert all three into the output. We need to do this because we can insert objects. // Otherwise there would be no need for the splitting and we could do simple replacement. let matchFoundSomewhere = false; // If we don't find a match anywhere we want to log it - for (const outputIndex in output) { + for (let outputIndex = 0; outputIndex < output.length; outputIndex++) { const inputText = output[outputIndex]; if (typeof inputText !== 'string') { // We might have inserted objects earlier, don't try to replace them continue; @@ -216,7 +234,7 @@ export function replaceByRegexes(text, mapping) { let replaced; // If substitution is a function, call it if (mapping[regexpString] instanceof Function) { - replaced = mapping[regexpString].apply(null, capturedGroups); + replaced = (mapping as Tags)[regexpString].apply(null, capturedGroups); } else { replaced = mapping[regexpString]; } @@ -277,11 +295,11 @@ export function replaceByRegexes(text, mapping) { // Allow overriding the text displayed when no translation exists // Currently only used in unit tests to avoid having to load // the translations in riot-web -export function setMissingEntryGenerator(f) { +export function setMissingEntryGenerator(f: (value: string) => void) { counterpart.setMissingEntryGenerator(f); } -export function setLanguage(preferredLangs) { +export function setLanguage(preferredLangs: string | string[]) { if (!Array.isArray(preferredLangs)) { preferredLangs = [preferredLangs]; } @@ -358,8 +376,8 @@ export function getLanguageFromBrowser() { * @param {string} language The input language string * @return {string[]} List of normalised languages */ -export function getNormalizedLanguageKeys(language) { - const languageKeys = []; +export function getNormalizedLanguageKeys(language: string) { + const languageKeys: string[] = []; const normalizedLanguage = normalizeLanguageKey(language); const languageParts = normalizedLanguage.split('-'); if (languageParts.length === 2 && languageParts[0] === languageParts[1]) { @@ -380,7 +398,7 @@ export function getNormalizedLanguageKeys(language) { * @param {string} language The language string to be normalized * @returns {string} The normalized language string */ -export function normalizeLanguageKey(language) { +export function normalizeLanguageKey(language: string) { return language.toLowerCase().replace("_", "-"); } @@ -396,7 +414,7 @@ export function getCurrentLanguage() { * @param {string[]} langs List of language codes to pick from * @returns {string} The most appropriate language code from langs */ -export function pickBestLanguage(langs) { +export function pickBestLanguage(langs: string[]): string { const currentLang = getCurrentLanguage(); const normalisedLangs = langs.map(normalizeLanguageKey); @@ -408,13 +426,13 @@ export function pickBestLanguage(langs) { { // Failing that, a different dialect of the same language - const closeLangIndex = normalisedLangs.find((l) => l.substr(0, 2) === currentLang.substr(0, 2)); + const closeLangIndex = normalisedLangs.findIndex((l) => l.substr(0, 2) === currentLang.substr(0, 2)); if (closeLangIndex > -1) return langs[closeLangIndex]; } { // Neither of those? Try an english variant. - const enIndex = normalisedLangs.find((l) => l.startsWith('en')); + const enIndex = normalisedLangs.findIndex((l) => l.startsWith('en')); if (enIndex > -1) return langs[enIndex]; } @@ -422,7 +440,7 @@ export function pickBestLanguage(langs) { return langs[0]; } -function getLangsJson() { +function getLangsJson(): Promise { return new Promise(async (resolve, reject) => { let url; if (typeof(webpackLangJsonUrl) === 'string') { // in Jest this 'url' isn't a URL, so just fall through @@ -443,7 +461,7 @@ function getLangsJson() { }); } -function weblateToCounterpart(inTrs) { +function weblateToCounterpart(inTrs: object): object { const outTrs = {}; for (const key of Object.keys(inTrs)) { @@ -463,7 +481,7 @@ function weblateToCounterpart(inTrs) { return outTrs; } -function getLanguage(langPath) { +function getLanguage(langPath: string): object { return new Promise((resolve, reject) => { request( { method: "GET", url: langPath }, diff --git a/yarn.lock b/yarn.lock index c20658f014..b51f7af45b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1257,6 +1257,11 @@ resolved "https://registry.yarnpkg.com/@types/classnames/-/classnames-2.2.10.tgz#cc658ca319b6355399efc1f5b9e818f1a24bf999" integrity sha512-1UzDldn9GfYYEsWWnn/P4wkTlkZDH7lDb0wBMGbtIQc9zXEQq7FlKBdZUn6OBqD8sKZZ2RQO2mAjGpXiDGoRmQ== +"@types/counterpart@^0.18.1": + version "0.18.1" + resolved "https://registry.yarnpkg.com/@types/counterpart/-/counterpart-0.18.1.tgz#b1b784d9e54d9879f0a8cb12f2caedab65430fe8" + integrity sha512-PRuFlBBkvdDOtxlIASzTmkEFar+S66Ek48NVVTWMUjtJAdn5vyMSN8y6IZIoIymGpR36q2nZbIYazBWyFxL+IQ== + "@types/fbemitter@*": version "2.0.32" resolved "https://registry.yarnpkg.com/@types/fbemitter/-/fbemitter-2.0.32.tgz#8ed204da0f54e9c8eaec31b1eec91e25132d082c" From f69a090d3d438eca7ec09c7da9b2ac2719205fd4 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jul 2020 23:22:36 +0100 Subject: [PATCH 06/38] delint Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/createRoom.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/createRoom.ts b/src/createRoom.ts index b863450065..c436196c27 100644 --- a/src/createRoom.ts +++ b/src/createRoom.ts @@ -16,7 +16,7 @@ limitations under the License. */ import {MatrixClient} from "matrix-js-sdk/src/client"; -import {Room} from "matrix-js-sdk/src/models/Room"; +import {Room} from "matrix-js-sdk/src/models/room"; import {MatrixClientPeg} from './MatrixClientPeg'; import Modal from './Modal'; From 33612398bed6383501bb1c2327f5fdc41a35c11b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jul 2020 23:26:39 +0100 Subject: [PATCH 07/38] fix import Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/groups.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/groups.js b/src/groups.js index 860cf71fff..e73af15c79 100644 --- a/src/groups.js +++ b/src/groups.js @@ -15,7 +15,8 @@ limitations under the License. */ import PropTypes from 'prop-types'; -import { _t } from './languageHandler.js'; + +import { _t } from './languageHandler'; export const GroupMemberType = PropTypes.shape({ userId: PropTypes.string.isRequired, From 96cfd26bd42ea1437e4cc4486e1338ce45becc59 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jul 2020 23:32:21 +0100 Subject: [PATCH 08/38] fix imports some more Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/elements/EditableItemList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/EditableItemList.js b/src/components/views/elements/EditableItemList.js index 50d5a3d10f..34e53906a2 100644 --- a/src/components/views/elements/EditableItemList.js +++ b/src/components/views/elements/EditableItemList.js @@ -16,7 +16,7 @@ limitations under the License. import React from 'react'; import PropTypes from 'prop-types'; -import {_t} from '../../../languageHandler.js'; +import {_t} from '../../../languageHandler'; import Field from "./Field"; import AccessibleButton from "./AccessibleButton"; From d725cc338924421c2ef5d2f231cad699a8298a6c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jul 2020 23:39:27 +0100 Subject: [PATCH 09/38] convert MatrixClientContext to Typescript Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../{MatrixClientContext.js => MatrixClientContext.ts} | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) rename src/contexts/{MatrixClientContext.js => MatrixClientContext.ts} (85%) diff --git a/src/contexts/MatrixClientContext.js b/src/contexts/MatrixClientContext.ts similarity index 85% rename from src/contexts/MatrixClientContext.js rename to src/contexts/MatrixClientContext.ts index 54a23ca132..7e8a92064d 100644 --- a/src/contexts/MatrixClientContext.js +++ b/src/contexts/MatrixClientContext.ts @@ -15,7 +15,8 @@ limitations under the License. */ import { createContext } from "react"; +import { MatrixClient } from "matrix-js-sdk/src/client"; -const MatrixClientContext = createContext(undefined); +const MatrixClientContext = createContext(undefined); MatrixClientContext.displayName = "MatrixClientContext"; export default MatrixClientContext; From 8233dec72e2ed69df381a38421e639633886aa96 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 21:05:06 +0100 Subject: [PATCH 10/38] Fix some room list sticky header instabilities Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/LeftPanel2.tsx | 1 + src/components/views/rooms/RoomList2.tsx | 6 +++++- src/components/views/rooms/RoomSublist2.tsx | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 23a9e74646..aed0434b7b 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -266,6 +266,7 @@ export default class LeftPanel2 extends React.Component { onFocus={this.onFocus} onBlur={this.onBlur} isMinimized={this.props.isMinimized} + onResize={this.onResize} />; // TODO: Conference handling / calls: https://github.com/vector-im/riot-web/issues/14177 diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index b0bb70c9a0..0c754b2c8d 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -55,6 +55,7 @@ interface IProps { collapsed: boolean; searchFilter: string; isMinimized: boolean; + onResize(); } interface IState { @@ -183,7 +184,9 @@ export default class RoomList2 extends React.Component { layoutMap.set(tagId, new ListLayout(tagId)); } - this.setState({sublists: newLists, layouts: layoutMap}); + this.setState({sublists: newLists, layouts: layoutMap}, () => { + this.props.onResize(); + }); }; private renderCommunityInvites(): React.ReactElement[] { @@ -256,6 +259,7 @@ export default class RoomList2 extends React.Component { isInvite={aesthetics.isInvite} layout={this.state.layouts.get(orderedTagId)} isMinimized={this.props.isMinimized} + onResize={this.props.onResize} extraBadTilesThatShouldntExist={extraTiles} /> ); diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 21e7c581f0..a1abfb8c7c 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -66,6 +66,7 @@ interface IProps { layout: ListLayout; isMinimized: boolean; tagId: TagID; + onResize(); // TODO: Don't use this. It's for community invites, and community invites shouldn't be here. // You should feel bad if you use this. @@ -228,6 +229,7 @@ export default class RoomSublist2 extends React.Component { private toggleCollapsed = () => { this.props.layout.isCollapsed = !this.props.layout.isCollapsed; this.forceUpdate(); // because the layout doesn't trigger an update + setImmediate(() => this.props.onResize()); // needs to happen when the DOM is updated }; private onHeaderKeyDown = (ev: React.KeyboardEvent) => { From c9bc318ca7a4798d04198ec9fe117d7dafed8ba3 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 21:32:46 +0100 Subject: [PATCH 11/38] Allow vertical scrolling on the new room list breadcrumbs Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/LeftPanel2.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 23a9e74646..26c95a1ad1 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -32,6 +32,7 @@ import ResizeNotifier from "../../utils/ResizeNotifier"; import SettingsStore from "../../settings/SettingsStore"; import RoomListStore, { LISTS_UPDATE_EVENT } from "../../stores/room-list/RoomListStore2"; import {Key} from "../../Keyboard"; +import IndicatorScrollbar from "../structures/IndicatorScrollbar"; // 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 @@ -217,11 +218,14 @@ export default class LeftPanel2 extends React.Component { private renderHeader(): React.ReactNode { let breadcrumbs; - if (this.state.showBreadcrumbs) { + if (this.state.showBreadcrumbs && !this.props.isMinimized) { breadcrumbs = ( -
- {this.props.isMinimized ? null : } -
+ + + ); } From 61a5807fd1f8d4b302f3b72478fc7587e0408f12 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 3 Jul 2020 12:17:54 +0200 Subject: [PATCH 12/38] only show topmost top sticky header --- src/components/structures/LeftPanel2.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 23a9e74646..88446e02f5 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -115,6 +115,7 @@ export default class LeftPanel2 extends React.Component { const headerStickyWidth = rlRect.width - headerRightMargin; let gotBottom = false; + let lastTopHeader; for (const sublist of sublists) { const slRect = sublist.getBoundingClientRect(); @@ -131,6 +132,12 @@ export default class LeftPanel2 extends React.Component { header.classList.add("mx_RoomSublist2_headerContainer_stickyTop"); header.style.width = `${headerStickyWidth}px`; header.style.top = `${rlRect.top}px`; + 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"); header.classList.remove("mx_RoomSublist2_headerContainer_stickyTop"); From a2cf641c0e7d69656dcd80fc0662fcea2e0583f6 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 3 Jul 2020 16:52:01 +0200 Subject: [PATCH 13/38] don't need to set width with javascript? --- res/css/views/rooms/_RoomSublist2.scss | 1 + src/components/structures/LeftPanel2.tsx | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 0e76152f86..e080f0efb4 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -70,6 +70,7 @@ limitations under the License. z-index: 1; // over top of other elements, but still under the ones in the visible list height: 32px; // to match the header container // width set by JS + width: calc(100% - 22px); } &.mx_RoomSublist2_headerContainer_stickyBottom { diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 88446e02f5..ccd4049c88 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -124,13 +124,11 @@ export default class LeftPanel2 extends React.Component { if (slRect.top + headerHeight > bottom && !gotBottom) { header.classList.add("mx_RoomSublist2_headerContainer_sticky"); header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); - header.style.width = `${headerStickyWidth}px`; header.style.top = `unset`; gotBottom = true; } else if (slRect.top < top) { 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`; if (lastTopHeader) { lastTopHeader.style.display = "none"; @@ -142,7 +140,6 @@ export default class LeftPanel2 extends React.Component { header.classList.remove("mx_RoomSublist2_headerContainer_sticky"); header.classList.remove("mx_RoomSublist2_headerContainer_stickyTop"); header.classList.remove("mx_RoomSublist2_headerContainer_stickyBottom"); - header.style.width = `unset`; header.style.top = `unset`; } } From 201f6ebe83f6c5095e67e1005fef3394c0822ade Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 3 Jul 2020 16:52:28 +0200 Subject: [PATCH 14/38] make stick headers jump in a bit later so the transition is less jumpy --- src/components/structures/LeftPanel2.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index ccd4049c88..db58535b50 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -126,7 +126,7 @@ export default class LeftPanel2 extends React.Component { header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); header.style.top = `unset`; gotBottom = true; - } else if (slRect.top < top) { + } else if ((slRect.top - (headerHeight / 3)) < top) { header.classList.add("mx_RoomSublist2_headerContainer_sticky"); header.classList.add("mx_RoomSublist2_headerContainer_stickyTop"); header.style.top = `${rlRect.top}px`; From 7647072b05839f03d3c6ae3c3c6e9e534be1258f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 3 Jul 2020 16:52:52 +0200 Subject: [PATCH 15/38] remove prop instead of assigning unset --- src/components/structures/LeftPanel2.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index db58535b50..e31b71e97c 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -124,7 +124,7 @@ export default class LeftPanel2 extends React.Component { if (slRect.top + headerHeight > bottom && !gotBottom) { header.classList.add("mx_RoomSublist2_headerContainer_sticky"); header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); - header.style.top = `unset`; + header.style.removeProperty("top"); gotBottom = true; } else if ((slRect.top - (headerHeight / 3)) < top) { header.classList.add("mx_RoomSublist2_headerContainer_sticky"); @@ -140,7 +140,7 @@ export default class LeftPanel2 extends React.Component { header.classList.remove("mx_RoomSublist2_headerContainer_sticky"); header.classList.remove("mx_RoomSublist2_headerContainer_stickyTop"); header.classList.remove("mx_RoomSublist2_headerContainer_stickyBottom"); - header.style.top = `unset`; + header.style.removeProperty("top"); } } } From 5bf14d8427094ecf49ee3254e7cae1bde7ccd2f9 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 6 Jul 2020 14:37:38 -0600 Subject: [PATCH 16/38] Minor cleanup of sticky header CSS --- res/css/views/rooms/_RoomSublist2.scss | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index e080f0efb4..3c9f21d311 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -69,8 +69,7 @@ limitations under the License. position: fixed; z-index: 1; // over top of other elements, but still under the ones in the visible list height: 32px; // to match the header container - // width set by JS - width: calc(100% - 22px); + width: calc(100% - 22px); // 22px is an offset for aligning the header to the container } &.mx_RoomSublist2_headerContainer_stickyBottom { From f6aa6208ee0495ea6802ac76b2ae02f9fbc858c8 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 21:53:20 +0100 Subject: [PATCH 17/38] null-guard against groups with a null name :(( Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomList2.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index b0bb70c9a0..82531cb77d 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -190,7 +190,7 @@ export default class RoomList2 extends React.Component { // TODO: Put community invites in a more sensible place (not in the room list) return MatrixClientPeg.get().getGroups().filter(g => { if (g.myMembership !== 'invite') return false; - return !this.searchFilter || this.searchFilter.matches(g.name); + return !this.searchFilter || this.searchFilter.matches(g.name || ""); }).map(g => { const avatar = ( Date: Mon, 6 Jul 2020 21:58:44 +0100 Subject: [PATCH 18/38] delint Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomSublist2.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index a1abfb8c7c..88f0f662c4 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -66,12 +66,13 @@ interface IProps { layout: ListLayout; isMinimized: boolean; tagId: TagID; - onResize(); // TODO: Don't use this. It's for community invites, and community invites shouldn't be here. // You should feel bad if you use this. extraBadTilesThatShouldntExist?: React.ReactElement[]; + onResize(); + // TODO: Account for https://github.com/vector-im/riot-web/issues/14179 } From abfbcf409008c53e642370f1fdb323a388816df2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jul 2020 22:04:30 +0100 Subject: [PATCH 19/38] use uglier style for props but be consistent :'( Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/rooms/RoomList2.tsx | 2 +- src/components/views/rooms/RoomSublist2.tsx | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index 0c754b2c8d..421e6abd12 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -51,11 +51,11 @@ interface IProps { onKeyDown: (ev: React.KeyboardEvent) => void; onFocus: (ev: React.FocusEvent) => void; onBlur: (ev: React.FocusEvent) => void; + onResize: () => void; resizeNotifier: ResizeNotifier; collapsed: boolean; searchFilter: string; isMinimized: boolean; - onResize(); } interface IState { diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 88f0f662c4..2ca9ec5dd1 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -66,13 +66,12 @@ interface IProps { layout: ListLayout; isMinimized: boolean; tagId: TagID; + onResize: () => void; // TODO: Don't use this. It's for community invites, and community invites shouldn't be here. // You should feel bad if you use this. extraBadTilesThatShouldntExist?: React.ReactElement[]; - onResize(); - // TODO: Account for https://github.com/vector-im/riot-web/issues/14179 } From 70eebc978fdf062d0d603a3b4de0332d333d3a53 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 6 Jul 2020 14:37:38 -0600 Subject: [PATCH 20/38] Revert "Minor cleanup of sticky header CSS" This reverts commit 5bf14d8427094ecf49ee3254e7cae1bde7ccd2f9. --- res/css/views/rooms/_RoomSublist2.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 3c9f21d311..e080f0efb4 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -69,7 +69,8 @@ limitations under the License. position: fixed; z-index: 1; // over top of other elements, but still under the ones in the visible list height: 32px; // to match the header container - width: calc(100% - 22px); // 22px is an offset for aligning the header to the container + // width set by JS + width: calc(100% - 22px); } &.mx_RoomSublist2_headerContainer_stickyBottom { From d14dd777b7d4bc1f4838de2908d4c11b08d73569 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 6 Jul 2020 15:23:20 -0600 Subject: [PATCH 21/38] Revert "don't need to set width with javascript?" This reverts commit a2cf641c0e7d69656dcd80fc0662fcea2e0583f6. --- res/css/views/rooms/_RoomSublist2.scss | 1 - src/components/structures/LeftPanel2.tsx | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index e080f0efb4..0e76152f86 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -70,7 +70,6 @@ limitations under the License. z-index: 1; // over top of other elements, but still under the ones in the visible list height: 32px; // to match the header container // width set by JS - width: calc(100% - 22px); } &.mx_RoomSublist2_headerContainer_stickyBottom { diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 44116c0efd..6c790f3318 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -125,11 +125,13 @@ export default class LeftPanel2 extends React.Component { if (slRect.top + headerHeight > 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 / 3)) < top) { 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`; if (lastTopHeader) { lastTopHeader.style.display = "none"; @@ -141,6 +143,7 @@ export default class LeftPanel2 extends React.Component { header.classList.remove("mx_RoomSublist2_headerContainer_sticky"); header.classList.remove("mx_RoomSublist2_headerContainer_stickyTop"); header.classList.remove("mx_RoomSublist2_headerContainer_stickyBottom"); + header.style.removeProperty("width"); header.style.removeProperty("top"); } } From 0b6f744a58b218d5b3ca1cae573d01ecf92428ca Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 3 Jul 2020 15:36:04 -0600 Subject: [PATCH 22/38] 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 = new Map(); private allowedRoomsByFilters: Set = new Set(); + 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 { - 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 Date: Fri, 3 Jul 2020 15:36:17 -0600 Subject: [PATCH 23/38] 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 Date: Mon, 6 Jul 2020 17:49:18 -0600 Subject: [PATCH 24/38] 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 { + // 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 : ''} and last is ${this._lastStickyRoom && this._lastStickyRoom.room ? this._lastStickyRoom.room.roomId : ''}`); + 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 Date: Mon, 6 Jul 2020 19:32:31 -0600 Subject: [PATCH 25/38] 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 || {}; + // 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 : ''} and last is ${this._lastStickyRoom && this._lastStickyRoom.room ? this._lastStickyRoom.room.roomId : ''}`); @@ -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 Date: Mon, 6 Jul 2020 19:35:58 -0600 Subject: [PATCH 26/38] 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 { // 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 : ''} and last is ${this._lastStickyRoom && this._lastStickyRoom.room ? this._lastStickyRoom.room.roomId : ''}`); - 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 Date: Mon, 6 Jul 2020 19:37:57 -0600 Subject: [PATCH 27/38] 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 = new Map(); private allowedRoomsByFilters: Set = new Set(); - 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 { // 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 Date: Mon, 6 Jul 2020 20:12:25 -0600 Subject: [PATCH 28/38] 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 Date: Mon, 6 Jul 2020 20:32:09 -0600 Subject: [PATCH 29/38] 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 { 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 Date: Mon, 6 Jul 2020 20:32:37 -0600 Subject: [PATCH 30/38] 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 { } 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 { 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( Date: Mon, 6 Jul 2020 20:37:04 -0600 Subject: [PATCH 31/38] 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 { } 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(
); + otherSections.push( +
+
{_t("Unread rooms")}
+ + {_t("Always show first")} + +
, + ); + otherSections.push(
); + otherSections.push( +
+
{_t("Show")}
+ + {_t("Message preview")} + +
, + ); + } + contextMenu = ( { {_t("A-Z")} -
-
-
{_t("Unread rooms")}
- - {_t("Always show first")} - -
-
-
-
{_t("Show")}
- - {_t("Message preview")} - -
+ {otherSections}
); From 56333ae017583d149fc00fb60d4b85b62740339d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 6 Jul 2020 20:42:25 -0600 Subject: [PATCH 32/38] 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 Date: Mon, 6 Jul 2020 20:45:36 -0600 Subject: [PATCH 33/38] 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. Learn more.": "Securely back up your keys to avoid losing them. Learn more.", "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 Date: Mon, 6 Jul 2020 20:48:49 -0600 Subject: [PATCH 34/38] 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 2ca90441715978f6c2e23e2b09f818da9caf5634 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 7 Jul 2020 12:46:33 +0200 Subject: [PATCH 35/38] swap order of context menu buttons so it does not jump when muted --- src/components/views/rooms/RoomTile2.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 8a9712b5a4..4af9ef7966 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -440,8 +440,8 @@ export default class RoomTile2 extends React.Component { {roomAvatar} {nameContainer} {badge} - {this.renderNotificationsMenu(isActive)} {this.renderGeneralMenu()} + {this.renderNotificationsMenu(isActive)} } From 8f47b59de87b13893e3b9eeac0259ada3e0ecb7f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 7 Jul 2020 13:03:16 +0200 Subject: [PATCH 36/38] fix margin between buttons I think the selector wasn't working before either fwiw --- res/css/views/rooms/_RoomTile2.scss | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/res/css/views/rooms/_RoomTile2.scss b/res/css/views/rooms/_RoomTile2.scss index 7b606ab947..50d376a66f 100644 --- a/res/css/views/rooms/_RoomTile2.scss +++ b/res/css/views/rooms/_RoomTile2.scss @@ -77,7 +77,7 @@ limitations under the License. } } - .mx_RoomTile2_menuButton { + .mx_RoomTile2_notificationsButton { margin-left: 4px; // spacing between buttons } @@ -108,7 +108,8 @@ limitations under the License. width: 20px; min-width: 20px; // yay flex height: 20px; - margin: auto 0; + margin-top: auto; + margin-bottom: auto; position: relative; display: none; From 994d8708f2e62f944aec46d2c7890cb23072170a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 7 Jul 2020 06:52:44 -0600 Subject: [PATCH 37/38] 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 { 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(
); - otherSections.push( -
-
{_t("Unread rooms")}
- - {_t("Always show first")} - -
, - ); - otherSections.push(
); - otherSections.push( -
-
{_t("Show")}
- - {_t("Message preview")} - -
, + otherSections = ( + +
+
+
{_t("Unread rooms")}
+ + {_t("Always show first")} + +
+
+
+
{_t("Show")}
+ + {_t("Message preview")} + +
+
); } From 1b48b99f99b712ecbef7cc5fb5643000b5115a1a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 7 Jul 2020 06:53:17 -0600 Subject: [PATCH 38/38] 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 { 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 { } } + 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