From d6a25d493abd23d24ba2225f500762cb36b12c17 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 14 May 2021 10:11:59 +0100 Subject: [PATCH 01/30] Create performance monitoring abstraction --- src/components/structures/MatrixChat.tsx | 46 +++------ src/performance/entry-names.ts | 57 +++++++++++ src/performance/index.ts | 120 +++++++++++++++++++++++ 3 files changed, 192 insertions(+), 31 deletions(-) create mode 100644 src/performance/entry-names.ts create mode 100644 src/performance/index.ts diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 288acc108a..64d205c89c 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -86,6 +86,8 @@ import {RoomUpdateCause} from "../../stores/room-list/models"; import defaultDispatcher from "../../dispatcher/dispatcher"; import SecurityCustomisations from "../../customisations/Security"; +import PerformanceMonitor, { PerformanceEntryNames } from "../../performance"; + /** constants for MatrixChat.state.view */ export enum Views { // a special initial state which is only used at startup, while we are @@ -484,42 +486,20 @@ export default class MatrixChat extends React.PureComponent { } startPageChangeTimer() { - // Tor doesn't support performance - if (!performance || !performance.mark) return null; - - // This shouldn't happen because UNSAFE_componentWillUpdate and componentDidUpdate - // are used. - if (this.pageChanging) { - console.warn('MatrixChat.startPageChangeTimer: timer already started'); - return; - } - this.pageChanging = true; - performance.mark('element_MatrixChat_page_change_start'); + PerformanceMonitor.start(PerformanceEntryNames.SWITCH_ROOM); } stopPageChangeTimer() { - // Tor doesn't support performance - if (!performance || !performance.mark) return null; + PerformanceMonitor.stop(PerformanceEntryNames.SWITCH_ROOM); - if (!this.pageChanging) { - console.warn('MatrixChat.stopPageChangeTimer: timer not started'); - return; - } - this.pageChanging = false; - performance.mark('element_MatrixChat_page_change_stop'); - performance.measure( - 'element_MatrixChat_page_change_delta', - 'element_MatrixChat_page_change_start', - 'element_MatrixChat_page_change_stop', - ); - performance.clearMarks('element_MatrixChat_page_change_start'); - performance.clearMarks('element_MatrixChat_page_change_stop'); - const measurement = performance.getEntriesByName('element_MatrixChat_page_change_delta').pop(); + const entries = PerformanceMonitor.getEntries({ + name: PerformanceEntryNames.SWITCH_ROOM, + }); + const measurement = entries.pop(); - // In practice, sometimes the entries list is empty, so we get no measurement - if (!measurement) return null; - - return measurement.duration; + return measurement + ? measurement.duration + : null; } shouldTrackPageChange(prevState: IState, state: IState) { @@ -1632,11 +1612,13 @@ export default class MatrixChat extends React.PureComponent { action: 'start_registration', params: params, }); + Performance.start(PerformanceEntryNames.REGISTER); } else if (screen === 'login') { dis.dispatch({ action: 'start_login', params: params, }); + Performance.start(PerformanceEntryNames.LOGIN); } else if (screen === 'forgot_password') { dis.dispatch({ action: 'start_password_recovery', @@ -1876,6 +1858,7 @@ export default class MatrixChat extends React.PureComponent { // returns a promise which resolves to the new MatrixClient onRegistered(credentials: IMatrixClientCreds) { + Performance.stop(PerformanceEntryNames.REGISTER); return Lifecycle.setLoggedIn(credentials); } @@ -1965,6 +1948,7 @@ export default class MatrixChat extends React.PureComponent { // Create and start the client await Lifecycle.setLoggedIn(credentials); await this.postLoginSetup(); + Performance.stop(PerformanceEntryNames.LOGIN); }; // complete security / e2e setup has finished diff --git a/src/performance/entry-names.ts b/src/performance/entry-names.ts new file mode 100644 index 0000000000..effd9506f6 --- /dev/null +++ b/src/performance/entry-names.ts @@ -0,0 +1,57 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +export enum PerformanceEntryNames { + + /** + * Application wide + */ + + APP_STARTUP = "mx_AppStartup", + PAGE_CHANGE = "mx_PageChange", + + /** + * Events + */ + + RESEND_EVENT = "mx_ResendEvent", + SEND_E2EE_EVENT = "mx_SendE2EEEvent", + SEND_ATTACHMENT = "mx_SendAttachment", + + /** + * Rooms + */ + + SWITCH_ROOM = "mx_SwithRoom", + JUMP_TO_ROOM = "mx_JumpToRoom", + JOIN_ROOM = "mx_JoinRoom", + CREATE_DM = "mx_CreateDM", + PEEK_ROOM = "mx_PeekRoom", + + /** + * User + */ + + VERIFY_E2EE_USER = "mx_VerifyE2EEUser", + LOGIN = "mx_Login", + REGISTER = "mx_Register", + + /** + * VoIP + */ + + SETUP_VOIP_CALL = "mx_SetupVoIPCall", +} diff --git a/src/performance/index.ts b/src/performance/index.ts new file mode 100644 index 0000000000..4379ba77e3 --- /dev/null +++ b/src/performance/index.ts @@ -0,0 +1,120 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { string } from "prop-types"; +import { PerformanceEntryNames } from "./entry-names"; + +const START_PREFIX = "start:"; +const STOP_PREFIX = "stop:"; + +export { + PerformanceEntryNames, +} + +interface GetEntriesOptions { + name?: string, + type?: string, +} + +export default class PerformanceMonitor { + /** + * Starts a performance recording + * @param name Name of the recording + * @param id Specify an identifier appended to the measurement name + * @returns {void} + */ + static start(name: string, id?: string): void { + if (!supportsPerformanceApi()) { + return; + } + const key = buildKey(name, id); + + if (!performance.getEntriesByName(key).length) { + console.warn(`Recording already started for: ${name}`); + return; + } + + performance.mark(START_PREFIX + key); + } + + /** + * Stops a performance recording and stores delta duration + * with the start marker + * @param name Name of the recording + * @param id Specify an identifier appended to the measurement name + * @returns {void} + */ + static stop(name: string, id?: string): void { + if (!supportsPerformanceApi()) { + return; + } + const key = buildKey(name, id); + if (!performance.getEntriesByName(START_PREFIX + key).length) { + console.warn(`No recording started for: ${name}`); + return; + } + + performance.mark(STOP_PREFIX + key); + performance.measure( + key, + START_PREFIX + key, + STOP_PREFIX + key, + ); + + this.clear(name, id); + } + + static clear(name: string, id?: string): void { + if (!supportsPerformanceApi()) { + return; + } + const key = buildKey(name, id); + performance.clearMarks(START_PREFIX + key); + performance.clearMarks(STOP_PREFIX + key); + } + + static getEntries({ name, type }: GetEntriesOptions = {}): PerformanceEntry[] { + if (!supportsPerformanceApi()) { + return; + } + + if (!name && !type) { + return performance.getEntries(); + } else if (!name) { + return performance.getEntriesByType(type); + } else { + return performance.getEntriesByName(name, type); + } + } +} + +/** + * Tor browser does not support the Performance API + * @returns {boolean} true if the Performance API is supported + */ +function supportsPerformanceApi(): boolean { + return performance !== undefined && performance.mark !== undefined; +} + +/** + * Internal utility to ensure consistent name for the recording + * @param name Name of the recording + * @param id Specify an identifier appended to the measurement name + * @returns {string} a compound of the name and identifier if present + */ +function buildKey(name: string, id?: string): string { + return `${name}${id ? `:${id}` : ''}`; +} From 6804a26e74267925637296a94ea9e2c927f00aa0 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 14 May 2021 11:04:58 +0100 Subject: [PATCH 02/30] Add performance data collection mechanism --- src/components/structures/MatrixChat.tsx | 14 ++--- src/performance/index.ts | 65 +++++++++++++++++++----- 2 files changed, 59 insertions(+), 20 deletions(-) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 64d205c89c..81381c56d3 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -486,14 +486,14 @@ export default class MatrixChat extends React.PureComponent { } startPageChangeTimer() { - PerformanceMonitor.start(PerformanceEntryNames.SWITCH_ROOM); + PerformanceMonitor.start(PerformanceEntryNames.PAGE_CHANGE); } stopPageChangeTimer() { - PerformanceMonitor.stop(PerformanceEntryNames.SWITCH_ROOM); + PerformanceMonitor.stop(PerformanceEntryNames.PAGE_CHANGE); const entries = PerformanceMonitor.getEntries({ - name: PerformanceEntryNames.SWITCH_ROOM, + name: PerformanceEntryNames.PAGE_CHANGE, }); const measurement = entries.pop(); @@ -1612,13 +1612,13 @@ export default class MatrixChat extends React.PureComponent { action: 'start_registration', params: params, }); - Performance.start(PerformanceEntryNames.REGISTER); + PerformanceMonitor.start(PerformanceEntryNames.REGISTER); } else if (screen === 'login') { dis.dispatch({ action: 'start_login', params: params, }); - Performance.start(PerformanceEntryNames.LOGIN); + PerformanceMonitor.start(PerformanceEntryNames.LOGIN); } else if (screen === 'forgot_password') { dis.dispatch({ action: 'start_password_recovery', @@ -1858,7 +1858,7 @@ export default class MatrixChat extends React.PureComponent { // returns a promise which resolves to the new MatrixClient onRegistered(credentials: IMatrixClientCreds) { - Performance.stop(PerformanceEntryNames.REGISTER); + PerformanceMonitor.stop(PerformanceEntryNames.REGISTER); return Lifecycle.setLoggedIn(credentials); } @@ -1948,7 +1948,7 @@ export default class MatrixChat extends React.PureComponent { // Create and start the client await Lifecycle.setLoggedIn(credentials); await this.postLoginSetup(); - Performance.stop(PerformanceEntryNames.LOGIN); + PerformanceMonitor.stop(PerformanceEntryNames.LOGIN); }; // complete security / e2e setup has finished diff --git a/src/performance/index.ts b/src/performance/index.ts index 4379ba77e3..3d903537a6 100644 --- a/src/performance/index.ts +++ b/src/performance/index.ts @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { string } from "prop-types"; import { PerformanceEntryNames } from "./entry-names"; const START_PREFIX = "start:"; @@ -29,6 +28,16 @@ interface GetEntriesOptions { type?: string, } +type PerformanceCallbackFunction = (entry: PerformanceEntry) => void; + +interface PerformanceDataListener { + entryTypes?: string[], + callback: PerformanceCallbackFunction +} + +let listeners: PerformanceDataListener[] = []; +const entries: PerformanceEntry[] = []; + export default class PerformanceMonitor { /** * Starts a performance recording @@ -42,7 +51,7 @@ export default class PerformanceMonitor { } const key = buildKey(name, id); - if (!performance.getEntriesByName(key).length) { + if (performance.getEntriesByName(key).length > 0) { console.warn(`Recording already started for: ${name}`); return; } @@ -57,12 +66,12 @@ export default class PerformanceMonitor { * @param id Specify an identifier appended to the measurement name * @returns {void} */ - static stop(name: string, id?: string): void { + static stop(name: string, id?: string): PerformanceEntry { if (!supportsPerformanceApi()) { return; } const key = buildKey(name, id); - if (!performance.getEntriesByName(START_PREFIX + key).length) { + if (performance.getEntriesByName(START_PREFIX + key).length === 0) { console.warn(`No recording started for: ${name}`); return; } @@ -75,6 +84,17 @@ export default class PerformanceMonitor { ); this.clear(name, id); + + const measurement = performance.getEntriesByName(key).pop(); + + // Keeping a reference to all PerformanceEntry created + // by this abstraction for historical events collection + // when adding a data callback + entries.push(measurement); + + listeners.forEach(listener => emitPerformanceData(listener, measurement)); + + return measurement; } static clear(name: string, id?: string): void { @@ -87,18 +107,37 @@ export default class PerformanceMonitor { } static getEntries({ name, type }: GetEntriesOptions = {}): PerformanceEntry[] { - if (!supportsPerformanceApi()) { - return; - } + return entries.filter(entry => { + const satisfiesName = !name || entry.name === name; + const satisfiedType = !type || entry.entryType === type; + return satisfiesName && satisfiedType; + }); + } - if (!name && !type) { - return performance.getEntries(); - } else if (!name) { - return performance.getEntriesByType(type); - } else { - return performance.getEntriesByName(name, type); + static addPerformanceDataCallback(listener: PerformanceDataListener, buffer = false) { + listeners.push(listener); + + if (buffer) { + entries.forEach(entry => emitPerformanceData(listener, entry)); } } + + static removePerformanceDataCallback(callback?: PerformanceCallbackFunction) { + if (!callback) { + listeners = []; + } else { + listeners.splice( + listeners.findIndex(listener => listener.callback === callback), + 1, + ); + } + } +} + +function emitPerformanceData(listener, entry): void { + if (!listener.entryTypes || listener.entryTypes.includes(entry.entryType)) { + listener.callback(entry) + } } /** From 89832eff9ef9dbe9cf3896d541797e294dd1e840 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 14 May 2021 12:23:23 +0100 Subject: [PATCH 03/30] Add data collection mechanism in end to end test suite --- src/@types/global.d.ts | 3 +++ src/components/structures/MatrixChat.tsx | 2 +- src/performance/index.ts | 25 +++++++++++++++--------- test/end-to-end-tests/.gitignore | 1 + test/end-to-end-tests/src/session.js | 2 +- test/end-to-end-tests/start.js | 22 +++++++++++++++++++-- 6 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index f04a2ff237..dec8559320 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -42,6 +42,7 @@ import {SpaceStoreClass} from "../stores/SpaceStore"; import TypingStore from "../stores/TypingStore"; import { EventIndexPeg } from "../indexing/EventIndexPeg"; import {VoiceRecordingStore} from "../stores/VoiceRecordingStore"; +import PerformanceMonitor, { PerformanceEntryNames } from "../performance"; declare global { interface Window { @@ -79,6 +80,8 @@ declare global { mxVoiceRecordingStore: VoiceRecordingStore; mxTypingStore: TypingStore; mxEventIndexPeg: EventIndexPeg; + mxPerformanceMonitor: PerformanceMonitor; + mxPerformanceEntryNames: PerformanceEntryNames; } interface Document { diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 81381c56d3..5a275b9a99 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -1858,7 +1858,6 @@ export default class MatrixChat extends React.PureComponent { // returns a promise which resolves to the new MatrixClient onRegistered(credentials: IMatrixClientCreds) { - PerformanceMonitor.stop(PerformanceEntryNames.REGISTER); return Lifecycle.setLoggedIn(credentials); } @@ -1949,6 +1948,7 @@ export default class MatrixChat extends React.PureComponent { await Lifecycle.setLoggedIn(credentials); await this.postLoginSetup(); PerformanceMonitor.stop(PerformanceEntryNames.LOGIN); + PerformanceMonitor.stop(PerformanceEntryNames.REGISTER); }; // complete security / e2e setup has finished diff --git a/src/performance/index.ts b/src/performance/index.ts index 3d903537a6..bbe8a207fe 100644 --- a/src/performance/index.ts +++ b/src/performance/index.ts @@ -28,10 +28,10 @@ interface GetEntriesOptions { type?: string, } -type PerformanceCallbackFunction = (entry: PerformanceEntry) => void; +type PerformanceCallbackFunction = (entry: PerformanceEntry[]) => void; interface PerformanceDataListener { - entryTypes?: string[], + entryNames?: string[], callback: PerformanceCallbackFunction } @@ -92,7 +92,11 @@ export default class PerformanceMonitor { // when adding a data callback entries.push(measurement); - listeners.forEach(listener => emitPerformanceData(listener, measurement)); + listeners.forEach(listener => { + if (shouldEmit(listener, measurement)) { + listener.callback([measurement]) + } + }); return measurement; } @@ -116,9 +120,11 @@ export default class PerformanceMonitor { static addPerformanceDataCallback(listener: PerformanceDataListener, buffer = false) { listeners.push(listener); - if (buffer) { - entries.forEach(entry => emitPerformanceData(listener, entry)); + const toEmit = entries.filter(entry => shouldEmit(listener, entry)); + if (toEmit.length > 0) { + listener.callback(toEmit); + } } } @@ -134,10 +140,8 @@ export default class PerformanceMonitor { } } -function emitPerformanceData(listener, entry): void { - if (!listener.entryTypes || listener.entryTypes.includes(entry.entryType)) { - listener.callback(entry) - } +function shouldEmit(listener: PerformanceDataListener, entry: PerformanceEntry): boolean { + return !listener.entryNames || listener.entryNames.includes(entry.name); } /** @@ -157,3 +161,6 @@ function supportsPerformanceApi(): boolean { function buildKey(name: string, id?: string): string { return `${name}${id ? `:${id}` : ''}`; } + +window.mxPerformanceMonitor = PerformanceMonitor; +window.mxPerformanceEntryNames = PerformanceEntryNames; diff --git a/test/end-to-end-tests/.gitignore b/test/end-to-end-tests/.gitignore index 61f9012393..9180d32e90 100644 --- a/test/end-to-end-tests/.gitignore +++ b/test/end-to-end-tests/.gitignore @@ -1,3 +1,4 @@ node_modules *.png element/env +performance-entries.json diff --git a/test/end-to-end-tests/src/session.js b/test/end-to-end-tests/src/session.js index 4c611ef877..6c68929a0b 100644 --- a/test/end-to-end-tests/src/session.js +++ b/test/end-to-end-tests/src/session.js @@ -208,7 +208,7 @@ module.exports = class ElementSession { this.log.done(); } - close() { + async close() { return this.browser.close(); } diff --git a/test/end-to-end-tests/start.js b/test/end-to-end-tests/start.js index 234d60da9f..ac06dcd989 100644 --- a/test/end-to-end-tests/start.js +++ b/test/end-to-end-tests/start.js @@ -22,7 +22,7 @@ const fs = require("fs"); const program = require('commander'); program .option('--no-logs', "don't output logs, document html on error", false) - .option('--app-url [url]', "url to test", "http://localhost:5000") + .option('--app-url [url]', "url to test", "http://localhost:8080") .option('--windowed', "dont run tests headless", false) .option('--slow-mo', "type at a human speed", false) .option('--dev-tools', "open chrome devtools in browser window", false) @@ -79,8 +79,26 @@ async function runTests() { await new Promise((resolve) => setTimeout(resolve, 5 * 60 * 1000)); } - await Promise.all(sessions.map((session) => session.close())); + const performanceEntries = {}; + await Promise.all(sessions.map(async (session) => { + // Collecting all performance monitoring data before closing the session + const measurements = await session.page.evaluate(() => { + let measurements = []; + window.mxPerformanceMonitor.addPerformanceDataCallback({ + entryNames: [ + window.mxPerformanceEntryNames.REGISTER, + ], + callback: (events) => { + measurements = JSON.stringify(events); + }, + }, true); + return measurements; + }); + performanceEntries[session.username] = JSON.parse(measurements); + return session.close(); + })); + fs.writeFileSync(`performance-entries.json`, JSON.stringify(performanceEntries)); if (failure) { process.exit(-1); } else { From cbf645785772daa5f49ea05cb9ee1f07cedebafc Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 14 May 2021 12:49:32 +0100 Subject: [PATCH 04/30] Revert app url to use the default that CI relies on --- test/end-to-end-tests/start.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/end-to-end-tests/start.js b/test/end-to-end-tests/start.js index ac06dcd989..f29b485c84 100644 --- a/test/end-to-end-tests/start.js +++ b/test/end-to-end-tests/start.js @@ -22,7 +22,7 @@ const fs = require("fs"); const program = require('commander'); program .option('--no-logs', "don't output logs, document html on error", false) - .option('--app-url [url]', "url to test", "http://localhost:8080") + .option('--app-url [url]', "url to test", "http://localhost:5000") .option('--windowed', "dont run tests headless", false) .option('--slow-mo', "type at a human speed", false) .option('--dev-tools', "open chrome devtools in browser window", false) From 781c0dca68d293c67bfc2f125d1e08fbefaf5b06 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 17 May 2021 09:30:53 +0100 Subject: [PATCH 05/30] Refactor performance monitor to use instance pattern --- src/@types/global.d.ts | 2 +- src/components/structures/MatrixChat.tsx | 16 +-- src/performance/index.ts | 130 +++++++++++++---------- 3 files changed, 81 insertions(+), 67 deletions(-) diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index dec8559320..fb3b92e45a 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -81,7 +81,7 @@ declare global { mxTypingStore: TypingStore; mxEventIndexPeg: EventIndexPeg; mxPerformanceMonitor: PerformanceMonitor; - mxPerformanceEntryNames: PerformanceEntryNames; + mxPerformanceEntryNames: any; } interface Document { diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 5a275b9a99..4c7fca2fec 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -486,13 +486,15 @@ export default class MatrixChat extends React.PureComponent { } startPageChangeTimer() { - PerformanceMonitor.start(PerformanceEntryNames.PAGE_CHANGE); + PerformanceMonitor.instance.start(PerformanceEntryNames.PAGE_CHANGE); } stopPageChangeTimer() { - PerformanceMonitor.stop(PerformanceEntryNames.PAGE_CHANGE); + const perfMonitor = PerformanceMonitor.instance; - const entries = PerformanceMonitor.getEntries({ + perfMonitor.stop(PerformanceEntryNames.PAGE_CHANGE); + + const entries = perfMonitor.getEntries({ name: PerformanceEntryNames.PAGE_CHANGE, }); const measurement = entries.pop(); @@ -1612,13 +1614,13 @@ export default class MatrixChat extends React.PureComponent { action: 'start_registration', params: params, }); - PerformanceMonitor.start(PerformanceEntryNames.REGISTER); + PerformanceMonitor.instance.start(PerformanceEntryNames.REGISTER); } else if (screen === 'login') { dis.dispatch({ action: 'start_login', params: params, }); - PerformanceMonitor.start(PerformanceEntryNames.LOGIN); + PerformanceMonitor.instance.start(PerformanceEntryNames.LOGIN); } else if (screen === 'forgot_password') { dis.dispatch({ action: 'start_password_recovery', @@ -1947,8 +1949,8 @@ export default class MatrixChat extends React.PureComponent { // Create and start the client await Lifecycle.setLoggedIn(credentials); await this.postLoginSetup(); - PerformanceMonitor.stop(PerformanceEntryNames.LOGIN); - PerformanceMonitor.stop(PerformanceEntryNames.REGISTER); + PerformanceMonitor.instance.stop(PerformanceEntryNames.LOGIN); + PerformanceMonitor.instance.stop(PerformanceEntryNames.REGISTER); }; // complete security / e2e setup has finished diff --git a/src/performance/index.ts b/src/performance/index.ts index bbe8a207fe..13ad0a55bb 100644 --- a/src/performance/index.ts +++ b/src/performance/index.ts @@ -16,13 +16,6 @@ limitations under the License. import { PerformanceEntryNames } from "./entry-names"; -const START_PREFIX = "start:"; -const STOP_PREFIX = "stop:"; - -export { - PerformanceEntryNames, -} - interface GetEntriesOptions { name?: string, type?: string, @@ -35,28 +28,40 @@ interface PerformanceDataListener { callback: PerformanceCallbackFunction } -let listeners: PerformanceDataListener[] = []; -const entries: PerformanceEntry[] = []; - export default class PerformanceMonitor { + static _instance: PerformanceMonitor; + + private START_PREFIX = "start:" + private STOP_PREFIX = "stop:" + + private listeners: PerformanceDataListener[] = [] + private entries: PerformanceEntry[] = [] + + public static get instance(): PerformanceMonitor { + if (!PerformanceMonitor._instance) { + PerformanceMonitor._instance = new PerformanceMonitor(); + } + return PerformanceMonitor._instance; + } + /** * Starts a performance recording * @param name Name of the recording * @param id Specify an identifier appended to the measurement name * @returns {void} */ - static start(name: string, id?: string): void { - if (!supportsPerformanceApi()) { + start(name: string, id?: string): void { + if (!this.supportsPerformanceApi()) { return; } - const key = buildKey(name, id); + const key = this.buildKey(name, id); if (performance.getEntriesByName(key).length > 0) { console.warn(`Recording already started for: ${name}`); return; } - performance.mark(START_PREFIX + key); + performance.mark(this.START_PREFIX + key); } /** @@ -66,21 +71,21 @@ export default class PerformanceMonitor { * @param id Specify an identifier appended to the measurement name * @returns {void} */ - static stop(name: string, id?: string): PerformanceEntry { - if (!supportsPerformanceApi()) { + stop(name: string, id?: string): PerformanceEntry { + if (!this.supportsPerformanceApi()) { return; } - const key = buildKey(name, id); - if (performance.getEntriesByName(START_PREFIX + key).length === 0) { + const key = this.buildKey(name, id); + if (performance.getEntriesByName(this.START_PREFIX + key).length === 0) { console.warn(`No recording started for: ${name}`); return; } - performance.mark(STOP_PREFIX + key); + performance.mark(this.STOP_PREFIX + key); performance.measure( key, - START_PREFIX + key, - STOP_PREFIX + key, + this.START_PREFIX + key, + this.STOP_PREFIX + key, ); this.clear(name, id); @@ -90,10 +95,10 @@ export default class PerformanceMonitor { // Keeping a reference to all PerformanceEntry created // by this abstraction for historical events collection // when adding a data callback - entries.push(measurement); + this.entries.push(measurement); - listeners.forEach(listener => { - if (shouldEmit(listener, measurement)) { + this.listeners.forEach(listener => { + if (this.shouldEmit(listener, measurement)) { listener.callback([measurement]) } }); @@ -101,66 +106,73 @@ export default class PerformanceMonitor { return measurement; } - static clear(name: string, id?: string): void { - if (!supportsPerformanceApi()) { + clear(name: string, id?: string): void { + if (!this.supportsPerformanceApi()) { return; } - const key = buildKey(name, id); - performance.clearMarks(START_PREFIX + key); - performance.clearMarks(STOP_PREFIX + key); + const key = this.buildKey(name, id); + performance.clearMarks(this.START_PREFIX + key); + performance.clearMarks(this.STOP_PREFIX + key); } - static getEntries({ name, type }: GetEntriesOptions = {}): PerformanceEntry[] { - return entries.filter(entry => { + getEntries({ name, type }: GetEntriesOptions = {}): PerformanceEntry[] { + return this.entries.filter(entry => { const satisfiesName = !name || entry.name === name; const satisfiedType = !type || entry.entryType === type; return satisfiesName && satisfiedType; }); } - static addPerformanceDataCallback(listener: PerformanceDataListener, buffer = false) { - listeners.push(listener); + addPerformanceDataCallback(listener: PerformanceDataListener, buffer = false) { + this.listeners.push(listener); if (buffer) { - const toEmit = entries.filter(entry => shouldEmit(listener, entry)); + const toEmit = this.entries.filter(entry => this.shouldEmit(listener, entry)); if (toEmit.length > 0) { listener.callback(toEmit); } } } - static removePerformanceDataCallback(callback?: PerformanceCallbackFunction) { + removePerformanceDataCallback(callback?: PerformanceCallbackFunction) { if (!callback) { - listeners = []; + this.listeners = []; } else { - listeners.splice( - listeners.findIndex(listener => listener.callback === callback), + this.listeners.splice( + this.listeners.findIndex(listener => listener.callback === callback), 1, ); } } + + /** + * Tor browser does not support the Performance API + * @returns {boolean} true if the Performance API is supported + */ + private supportsPerformanceApi(): boolean { + return performance !== undefined && performance.mark !== undefined; + } + + private shouldEmit(listener: PerformanceDataListener, entry: PerformanceEntry): boolean { + return !listener.entryNames || listener.entryNames.includes(entry.name); + } + + /** + * Internal utility to ensure consistent name for the recording + * @param name Name of the recording + * @param id Specify an identifier appended to the measurement name + * @returns {string} a compound of the name and identifier if present + */ + private buildKey(name: string, id?: string): string { + return `${name}${id ? `:${id}` : ''}`; + } } -function shouldEmit(listener: PerformanceDataListener, entry: PerformanceEntry): boolean { - return !listener.entryNames || listener.entryNames.includes(entry.name); + +// Convienience exports +export { + PerformanceEntryNames, } -/** - * Tor browser does not support the Performance API - * @returns {boolean} true if the Performance API is supported - */ -function supportsPerformanceApi(): boolean { - return performance !== undefined && performance.mark !== undefined; -} - -/** - * Internal utility to ensure consistent name for the recording - * @param name Name of the recording - * @param id Specify an identifier appended to the measurement name - * @returns {string} a compound of the name and identifier if present - */ -function buildKey(name: string, id?: string): string { - return `${name}${id ? `:${id}` : ''}`; -} - -window.mxPerformanceMonitor = PerformanceMonitor; +// Exposing those to the window object to bridge them from tests +window.mxPerformanceMonitor = PerformanceMonitor.instance; window.mxPerformanceEntryNames = PerformanceEntryNames; From f3bebdbc8733f07556f79609e8afdc58778738f4 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 17 May 2021 09:44:36 +0100 Subject: [PATCH 06/30] remove unused import --- src/@types/global.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index fb3b92e45a..63966d96fa 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -42,7 +42,7 @@ import {SpaceStoreClass} from "../stores/SpaceStore"; import TypingStore from "../stores/TypingStore"; import { EventIndexPeg } from "../indexing/EventIndexPeg"; import {VoiceRecordingStore} from "../stores/VoiceRecordingStore"; -import PerformanceMonitor, { PerformanceEntryNames } from "../performance"; +import PerformanceMonitor from "../performance"; declare global { interface Window { From 5b6c5aac16f21be4cb604b93f981120f1fe1b828 Mon Sep 17 00:00:00 2001 From: Germain Date: Mon, 17 May 2021 12:02:24 +0100 Subject: [PATCH 07/30] Fix comment typo Co-authored-by: J. Ryan Stinnett --- src/performance/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/performance/index.ts b/src/performance/index.ts index 13ad0a55bb..7eb6d26567 100644 --- a/src/performance/index.ts +++ b/src/performance/index.ts @@ -168,7 +168,7 @@ export default class PerformanceMonitor { } -// Convienience exports +// Convenience exports export { PerformanceEntryNames, } From 07d74693af2e3e2c11d4e455e35e9ee1267e3272 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Tue, 18 May 2021 16:00:39 +0100 Subject: [PATCH 08/30] Start decryption process if needed --- src/Notifier.ts | 2 ++ src/stores/widgets/StopGapWidget.ts | 1 + 2 files changed, 3 insertions(+) diff --git a/src/Notifier.ts b/src/Notifier.ts index 3e927cea0c..4f55046e72 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -331,6 +331,8 @@ export const Notifier = { if (!this.isSyncing) return; // don't alert for any messages initially if (ev.sender && ev.sender.userId === MatrixClientPeg.get().credentials.userId) return; + MatrixClientPeg.get().decryptEventIfNeeded(ev); + // If it's an encrypted event and the type is still 'm.room.encrypted', // it hasn't yet been decrypted, so wait until it is. if (ev.isBeingDecrypted() || ev.isDecryptionFailure()) { diff --git a/src/stores/widgets/StopGapWidget.ts b/src/stores/widgets/StopGapWidget.ts index 17371d6d45..b0a76a35af 100644 --- a/src/stores/widgets/StopGapWidget.ts +++ b/src/stores/widgets/StopGapWidget.ts @@ -395,6 +395,7 @@ export class StopGapWidget extends EventEmitter { } private onEvent = (ev: MatrixEvent) => { + MatrixClientPeg.get().decryptEventIfNeeded(ev); if (ev.isBeingDecrypted() || ev.isDecryptionFailure()) return; if (ev.getRoomId() !== this.eventListenerRoomId) return; this.feedEvent(ev); From bcbfbd508d110ae163597bdbdc384cc2b5ed1264 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Wed, 19 May 2021 09:45:37 +0100 Subject: [PATCH 09/30] Fix event start check --- src/performance/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/performance/index.ts b/src/performance/index.ts index 7eb6d26567..bfb5b4a9c7 100644 --- a/src/performance/index.ts +++ b/src/performance/index.ts @@ -56,7 +56,7 @@ export default class PerformanceMonitor { } const key = this.buildKey(name, id); - if (performance.getEntriesByName(key).length > 0) { + if (performance.getEntriesByName(this.START_PREFIX + key).length > 0) { console.warn(`Recording already started for: ${name}`); return; } From 382a08bdb1bd510ac6aa0b7fce7e9ff8ef8e48cd Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Wed, 19 May 2021 11:38:10 +0100 Subject: [PATCH 10/30] Delete RoomView dead code --- src/components/structures/RoomView.tsx | 51 ++------------------------ 1 file changed, 3 insertions(+), 48 deletions(-) diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index dbfba13297..fb9c0eb3a3 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -811,7 +811,7 @@ export default class RoomView extends React.Component { }; private onEvent = (ev) => { - if (ev.isBeingDecrypted() || ev.isDecryptionFailure() || ev.shouldAttemptDecryption()) return; + if (ev.isBeingDecrypted() || ev.isDecryptionFailure()) return; this.handleEffects(ev); }; @@ -831,14 +831,14 @@ export default class RoomView extends React.Component { private onRoomName = (room: Room) => { if (this.state.room && room.roomId == this.state.room.roomId) { - this.forceUpdate(); + // this.forceUpdate(); } }; private onKeyBackupStatus = () => { // Key backup status changes affect whether the in-room recovery // reminder is displayed. - this.forceUpdate(); + // this.forceUpdate(); }; public canResetTimeline = () => { @@ -1598,33 +1598,6 @@ export default class RoomView extends React.Component { this.setState({auxPanelMaxHeight: auxPanelMaxHeight}); }; - private onFullscreenClick = () => { - dis.dispatch({ - action: 'video_fullscreen', - fullscreen: true, - }, true); - }; - - private onMuteAudioClick = () => { - const call = this.getCallForRoom(); - if (!call) { - return; - } - const newState = !call.isMicrophoneMuted(); - call.setMicrophoneMuted(newState); - this.forceUpdate(); // TODO: just update the voip buttons - }; - - private onMuteVideoClick = () => { - const call = this.getCallForRoom(); - if (!call) { - return; - } - const newState = !call.isLocalVideoMuted(); - call.setLocalVideoMuted(newState); - this.forceUpdate(); // TODO: just update the voip buttons - }; - private onStatusBarVisible = () => { if (this.unmounted) return; this.setState({ @@ -1640,24 +1613,6 @@ export default class RoomView extends React.Component { }); }; - /** - * called by the parent component when PageUp/Down/etc is pressed. - * - * We pass it down to the scroll panel. - */ - private handleScrollKey = ev => { - let panel; - if (this.searchResultsPanel.current) { - panel = this.searchResultsPanel.current; - } else if (this.messagePanel) { - panel = this.messagePanel; - } - - if (panel) { - panel.handleScrollKey(ev); - } - }; - /** * get any current call for this room */ From 8f945ce8467d1f4b6984c544e29e30e0c8b6b5d1 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Wed, 19 May 2021 11:57:32 +0100 Subject: [PATCH 11/30] Render nothin rather than an empty div --- src/components/views/elements/AccessibleTooltipButton.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/AccessibleTooltipButton.tsx b/src/components/views/elements/AccessibleTooltipButton.tsx index 3bb264fb3e..c98a7c3156 100644 --- a/src/components/views/elements/AccessibleTooltipButton.tsx +++ b/src/components/views/elements/AccessibleTooltipButton.tsx @@ -73,7 +73,7 @@ export default class AccessibleTooltipButton extends React.PureComponent :
; + /> : null; return ( Date: Wed, 19 May 2021 14:32:49 +0100 Subject: [PATCH 12/30] prevent unwarranted RoomView re-render --- src/components/structures/RoomView.tsx | 15 ++++++++++++- src/components/structures/TimelinePanel.js | 25 ---------------------- 2 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index fb9c0eb3a3..25ebbcf223 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -83,6 +83,7 @@ import { objectHasDiff } from "../../utils/objects"; import SpaceRoomView from "./SpaceRoomView"; import { IOpts } from "../../createRoom"; import {replaceableComponent} from "../../utils/replaceableComponent"; +import _ from 'lodash'; const DEBUG = false; let debuglog = function(msg: string) {}; @@ -175,6 +176,7 @@ export interface IState { statusBarVisible: boolean; // We load this later by asking the js-sdk to suggest a version for us. // This object is the result of Room#getRecommendedVersion() + upgradeRecommendation?: { version: string; needsUpgrade: boolean; @@ -528,7 +530,18 @@ export default class RoomView extends React.Component { } shouldComponentUpdate(nextProps, nextState) { - return (objectHasDiff(this.props, nextProps) || objectHasDiff(this.state, nextState)); + const hasPropsDiff = objectHasDiff(this.props, nextProps); + + const newUpgradeRecommendation = nextState.upgradeRecommendation || {} + + const state = _.omit(this.state, ['upgradeRecommendation']); + const newState = _.omit(nextState, ['upgradeRecommendation']) + + const hasStateDiff = + objectHasDiff(state, newState) || + (newUpgradeRecommendation && newUpgradeRecommendation.needsUpgrade === true) + + return hasPropsDiff || hasStateDiff; } componentDidUpdate() { diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index af20c31cb2..832043d3c6 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -36,7 +36,6 @@ import shouldHideEvent from '../../shouldHideEvent'; import EditorStateTransfer from '../../utils/EditorStateTransfer'; import {haveTileForEvent} from "../views/rooms/EventTile"; import {UIFeature} from "../../settings/UIFeature"; -import {objectHasDiff} from "../../utils/objects"; import {replaceableComponent} from "../../utils/replaceableComponent"; import { arrayFastClone } from "../../utils/arrays"; @@ -265,30 +264,6 @@ class TimelinePanel extends React.Component { } } - shouldComponentUpdate(nextProps, nextState) { - if (objectHasDiff(this.props, nextProps)) { - if (DEBUG) { - console.group("Timeline.shouldComponentUpdate: props change"); - console.log("props before:", this.props); - console.log("props after:", nextProps); - console.groupEnd(); - } - return true; - } - - if (objectHasDiff(this.state, nextState)) { - if (DEBUG) { - console.group("Timeline.shouldComponentUpdate: state change"); - console.log("state before:", this.state); - console.log("state after:", nextState); - console.groupEnd(); - } - return true; - } - - return false; - } - componentWillUnmount() { // set a boolean to say we've been unmounted, which any pending // promises can use to throw away their results. From d3623217068ca9d2bc8cb170ac1f9f8329b9de3a Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 20 May 2021 15:25:20 +0100 Subject: [PATCH 13/30] Simplify SenderProfile DOM structure --- res/css/views/rooms/_IRCLayout.scss | 15 ++++++--------- src/components/views/messages/SenderProfile.js | 17 +++++------------ 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/res/css/views/rooms/_IRCLayout.scss b/res/css/views/rooms/_IRCLayout.scss index b6b901757c..48505fbb53 100644 --- a/res/css/views/rooms/_IRCLayout.scss +++ b/res/css/views/rooms/_IRCLayout.scss @@ -177,16 +177,13 @@ $irc-line-height: $font-18px; .mx_SenderProfile_hover { background-color: $primary-bg-color; overflow: hidden; + display: flex; - > span { - display: flex; - - > .mx_SenderProfile_name { - overflow: hidden; - text-overflow: ellipsis; - min-width: var(--name-width); - text-align: end; - } + > .mx_SenderProfile_name { + overflow: hidden; + text-overflow: ellipsis; + min-width: var(--name-width); + text-align: end; } } diff --git a/src/components/views/messages/SenderProfile.js b/src/components/views/messages/SenderProfile.js index bd10526799..f1855de99e 100644 --- a/src/components/views/messages/SenderProfile.js +++ b/src/components/views/messages/SenderProfile.js @@ -110,19 +110,12 @@ export default class SenderProfile extends React.Component { const nameElem = name || ''; - // Name + flair - const nameFlair = - - { nameElem } - - { flair } - ; - return ( -
-
- { nameFlair } -
+
+ + { nameElem } + + { flair }
); } From 171539d42d560bf37abbfdfba86bb0f22986f84e Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 20 May 2021 15:26:02 +0100 Subject: [PATCH 14/30] Simplify EventTile structure Only render MessageTimestamp to the DOM when a tile is hovered --- src/components/structures/MessagePanel.js | 65 +++++++++++------------ src/components/views/rooms/EventTile.tsx | 46 ++++++++++------ 2 files changed, 61 insertions(+), 50 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index d1071a9e19..2fb9a2df29 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -645,39 +645,36 @@ export default class MessagePanel extends React.Component { // use txnId as key if available so that we don't remount during sending ret.push( -
  • - - - -
    • , + + + , ); return ret; @@ -779,7 +776,7 @@ export default class MessagePanel extends React.Component { } _collectEventNode = (eventId, node) => { - this.eventNodes[eventId] = node; + this.eventNodes[eventId] = node?.ref?.current; } // once dynamic content in the events load, make the scrollPanel check the diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index 19c5a7acaa..884669e398 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -277,6 +277,9 @@ interface IProps { // Helper to build permalinks for the room permalinkCreator?: RoomPermalinkCreator; + + // Symbol of the root node + as?: string } interface IState { @@ -291,6 +294,8 @@ interface IState { previouslyRequestedKeys: boolean; // The Relations model from the JS SDK for reactions to `mxEvent` reactions: Relations; + + hover: boolean; } @replaceableComponent("views.rooms.EventTile") @@ -322,6 +327,8 @@ export default class EventTile extends React.Component { previouslyRequestedKeys: false, // The Relations model from the JS SDK for reactions to `mxEvent` reactions: this.getReactions(), + + hover: false, }; // don't do RR animations until we are mounted @@ -333,6 +340,8 @@ export default class EventTile extends React.Component { // to determine if we've already subscribed and use a combination of other flags to find // out if we should even be subscribed at all. this.isListeningForReceipts = false; + + this.ref = React.createRef(); } /** @@ -960,7 +969,7 @@ export default class EventTile extends React.Component { onFocusChange={this.onActionBarFocusChange} /> : undefined; - const timestamp = this.props.mxEvent.getTs() ? + const timestamp = this.props.mxEvent.getTs() && this.state.hover ? : null; const keyRequestHelpText = @@ -1131,11 +1140,20 @@ export default class EventTile extends React.Component { // tab-index=-1 to allow it to be focusable but do not add tab stop for it, primarily for screen readers return ( -
    - { ircTimestamp } - { sender } - { ircPadlock } -
    + React.createElement(this.props.as || "div", { + "ref": this.ref, + "className": classes, + "tabIndex": -1, + "aria-live": ariaLive, + "aria-atomic": "true", + "data-scroll-tokens": this.props["data-scroll-tokens"], + "onMouseEnter": () => this.setState({ hover: true }), + "onMouseLeave": () => this.setState({ hover: false }), + }, [ + ircTimestamp, + sender, + ircPadlock, +
    { groupTimestamp } { groupPadlock } { thread } @@ -1152,16 +1170,12 @@ export default class EventTile extends React.Component { { keyRequestInfo } { reactionsRow } { actionBar } -
    - {msgOption} - { - // The avatar goes after the event tile as it's absolutely positioned to be over the - // event tile line, so needs to be later in the DOM so it appears on top (this avoids - // the need for further z-indexing chaos) - } - { avatar } -
    - ); +
    , + msgOption, + avatar, + + ]) + ) } } } From f058fd8869dca84b05d70fc1390da262d2e38439 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 20 May 2021 15:39:25 +0100 Subject: [PATCH 15/30] Reduce amount of DOM nodes --- res/css/views/rooms/_IRCLayout.scss | 3 +-- src/components/views/elements/ReplyThread.js | 2 +- src/components/views/rooms/EventTile.tsx | 23 ++++++++++++-------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/res/css/views/rooms/_IRCLayout.scss b/res/css/views/rooms/_IRCLayout.scss index 48505fbb53..cf61ce569d 100644 --- a/res/css/views/rooms/_IRCLayout.scss +++ b/res/css/views/rooms/_IRCLayout.scss @@ -115,8 +115,7 @@ $irc-line-height: $font-18px; .mx_EventTile_line { .mx_EventTile_e2eIcon, .mx_TextualEvent, - .mx_MTextBody, - .mx_ReplyThread_wrapper_empty { + .mx_MTextBody { display: inline-block; } } diff --git a/src/components/views/elements/ReplyThread.js b/src/components/views/elements/ReplyThread.js index 870803995d..c336f34b51 100644 --- a/src/components/views/elements/ReplyThread.js +++ b/src/components/views/elements/ReplyThread.js @@ -214,7 +214,7 @@ export default class ReplyThread extends React.Component { static makeThread(parentEv, onHeightChanged, permalinkCreator, ref, layout) { if (!ReplyThread.getParentEventId(parentEv)) { - return
    ; + return null; } return { let left = 0; const receipts = this.props.readReceipts || []; + + if (receipts.length === 0) { + return null; + } + for (let i = 0; i < receipts.length; ++i) { const receipt = receipts[i]; @@ -699,10 +704,14 @@ export default class EventTile extends React.Component { } } - return - { remText } - { avatars } - ; + return ( +
    + + { remText } + { avatars } + ; +
    + ) } onSenderProfileClick = event => { @@ -1032,11 +1041,7 @@ export default class EventTile extends React.Component { let msgOption; if (this.props.showReadReceipts) { const readAvatars = this.getReadAvatars(); - msgOption = ( -
    - { readAvatars } -
    - ); + msgOption = readAvatars; } switch (this.props.tileShape) { From 9e55f2409214f97014143549997e5d445d9787ed Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 20 May 2021 16:11:33 +0100 Subject: [PATCH 16/30] Remove extraenous DOM nodes --- src/components/structures/ContextMenu.tsx | 2 +- src/components/structures/ToastContainer.tsx | 22 ++++++++++--------- src/components/views/rooms/RoomHeader.js | 18 +++++++-------- .../views/rooms/SimpleRoomHeader.js | 12 +++++----- src/components/views/rooms/WhoIsTypingTile.js | 2 +- 5 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index ad0f75e162..f9de113d07 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -389,7 +389,7 @@ export class ContextMenu extends React.PureComponent { } render(): React.ReactChild { - return ReactDOM.createPortal(this.renderMenu(), getOrCreateContainer()); + return ReactDOM.createPortal(this.renderMenu(), document.body); } } diff --git a/src/components/structures/ToastContainer.tsx b/src/components/structures/ToastContainer.tsx index 1fd3e3419f..273c8a079f 100644 --- a/src/components/structures/ToastContainer.tsx +++ b/src/components/structures/ToastContainer.tsx @@ -55,6 +55,7 @@ export default class ToastContainer extends React.Component<{}, IState> { const totalCount = this.state.toasts.length; const isStacked = totalCount > 1; let toast; + let containerClasses; if (totalCount !== 0) { const topToast = this.state.toasts[0]; const {title, icon, key, component, className, props} = topToast; @@ -79,16 +80,17 @@ export default class ToastContainer extends React.Component<{}, IState> {
    {React.createElement(component, toastProps)}
    ); + + containerClasses = classNames("mx_ToastContainer", { + "mx_ToastContainer_stacked": isStacked, + }); } - - const containerClasses = classNames("mx_ToastContainer", { - "mx_ToastContainer_stacked": isStacked, - }); - - return ( -
    - {toast} -
    - ); + return toast + ? ( +
    + {toast} +
    + ) + : null; } } diff --git a/src/components/views/rooms/RoomHeader.js b/src/components/views/rooms/RoomHeader.js index 6d3b50c10d..a527d7625c 100644 --- a/src/components/views/rooms/RoomHeader.js +++ b/src/components/views/rooms/RoomHeader.js @@ -257,16 +257,14 @@ export default class RoomHeader extends React.Component { const e2eIcon = this.props.e2eStatus ? : undefined; return ( -
    -
    -
    { roomAvatar }
    -
    { e2eIcon }
    - { name } - { topicElement } - { cancelButton } - { rightRow } - -
    +
    +
    { roomAvatar }
    +
    { e2eIcon }
    + { name } + { topicElement } + { cancelButton } + { rightRow } +
    ); } diff --git a/src/components/views/rooms/SimpleRoomHeader.js b/src/components/views/rooms/SimpleRoomHeader.js index b2a66f6670..9aedb38654 100644 --- a/src/components/views/rooms/SimpleRoomHeader.js +++ b/src/components/views/rooms/SimpleRoomHeader.js @@ -62,13 +62,11 @@ export default class SimpleRoomHeader extends React.Component { } return ( -
    -
    -
    - { icon } - { this.props.title } - { cancelButton } -
    +
    +
    + { icon } + { this.props.title } + { cancelButton }
    ); diff --git a/src/components/views/rooms/WhoIsTypingTile.js b/src/components/views/rooms/WhoIsTypingTile.js index a25b43fc3a..396d64a6f8 100644 --- a/src/components/views/rooms/WhoIsTypingTile.js +++ b/src/components/views/rooms/WhoIsTypingTile.js @@ -204,7 +204,7 @@ export default class WhoIsTypingTile extends React.Component { this.props.whoIsTypingLimit, ); if (!typingString) { - return (
    ); + return null; } return ( From 229c4b98b44d14117a272c817ffb95ed622ec15d Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 20 May 2021 18:01:38 +0100 Subject: [PATCH 17/30] use userGroups cached value to avoid re-render --- src/components/views/elements/Flair.js | 2 +- .../views/messages/SenderProfile.js | 35 ++++++++++++------- src/stores/FlairStore.js | 4 +++ 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js index 73d5b91511..23858b860d 100644 --- a/src/components/views/elements/Flair.js +++ b/src/components/views/elements/Flair.js @@ -116,7 +116,7 @@ export default class Flair extends React.Component { render() { if (this.state.profiles.length === 0) { - return ; + return null; } const avatars = this.state.profiles.map((profile, index) => { return ; diff --git a/src/components/views/messages/SenderProfile.js b/src/components/views/messages/SenderProfile.js index f1855de99e..8f10954370 100644 --- a/src/components/views/messages/SenderProfile.js +++ b/src/components/views/messages/SenderProfile.js @@ -31,21 +31,23 @@ export default class SenderProfile extends React.Component { static contextType = MatrixClientContext; - state = { - userGroups: null, - relatedGroups: [], - }; + constructor(props) { + super(props); + const senderId = this.props.mxEvent.getSender(); + this.state = { + userGroups: FlairStore.cachedPublicisedGroups(senderId) || [], + relatedGroups: [], + }; + } componentDidMount() { this.unmounted = false; this._updateRelatedGroups(); - FlairStore.getPublicisedGroupsCached( - this.context, this.props.mxEvent.getSender(), - ).then((userGroups) => { - if (this.unmounted) return; - this.setState({userGroups}); - }); + if (this.state.userGroups.length === 0) { + this.getPublicisedGroups(); + } + this.context.on('RoomState.events', this.onRoomStateEvents); } @@ -55,6 +57,15 @@ export default class SenderProfile extends React.Component { this.context.removeListener('RoomState.events', this.onRoomStateEvents); } + async getPublicisedGroups() { + if (!this.unmounted) { + const userGroups = await FlairStore.getPublicisedGroupsCached( + this.context, this.props.mxEvent.getSender(), + ); + this.setState({userGroups}); + } + } + onRoomStateEvents = event => { if (event.getType() === 'm.room.related_groups' && event.getRoomId() === this.props.mxEvent.getRoomId() @@ -93,10 +104,10 @@ export default class SenderProfile extends React.Component { const {msgtype} = mxEvent.getContent(); if (msgtype === 'm.emote') { - return ; // emote message must include the name so don't duplicate it + return null; // emote message must include the name so don't duplicate it } - let flair =
    ; + let flair = null; if (this.props.enableFlair) { const displayedGroups = this._getDisplayedGroups( this.state.userGroups, this.state.relatedGroups, diff --git a/src/stores/FlairStore.js b/src/stores/FlairStore.js index 53d07d0452..23254b98ab 100644 --- a/src/stores/FlairStore.js +++ b/src/stores/FlairStore.js @@ -65,6 +65,10 @@ class FlairStore extends EventEmitter { delete this._userGroups[userId]; } + cachedPublicisedGroups(userId) { + return this._userGroups[userId]; + } + getPublicisedGroupsCached(matrixClient, userId) { if (this._userGroups[userId]) { return Promise.resolve(this._userGroups[userId]); From 0f63098c59674623b754e2b74a22b58cf985d443 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 20 May 2021 18:02:44 +0100 Subject: [PATCH 18/30] Remove typo semicolon --- src/components/views/rooms/EventTile.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index c85945be7a..82d97bff98 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -709,7 +709,7 @@ export default class EventTile extends React.Component { { remText } { avatars } - ; +
    ) } From 47e007e08f9bedaf47cf59a63c9bd04219195d76 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 21 May 2021 10:20:24 +0100 Subject: [PATCH 19/30] batch load events in ReplyThread before adding them to the state --- src/components/views/elements/ReplyThread.js | 42 +++++++++----------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/components/views/elements/ReplyThread.js b/src/components/views/elements/ReplyThread.js index c336f34b51..bbced5328f 100644 --- a/src/components/views/elements/ReplyThread.js +++ b/src/components/views/elements/ReplyThread.js @@ -269,36 +269,27 @@ export default class ReplyThread extends React.Component { const {parentEv} = this.props; // at time of making this component we checked that props.parentEv has a parentEventId const ev = await this.getEvent(ReplyThread.getParentEventId(parentEv)); + if (this.unmounted) return; if (ev) { + const loadedEv = await this.getNextEvent(ev); this.setState({ events: [ev], - }, this.loadNextEvent); + loadedEv, + loading: false, + }); } else { this.setState({err: true}); } } - async loadNextEvent() { - if (this.unmounted) return; - const ev = this.state.events[0]; - const inReplyToEventId = ReplyThread.getParentEventId(ev); - - if (!inReplyToEventId) { - this.setState({ - loading: false, - }); - return; - } - - const loadedEv = await this.getEvent(inReplyToEventId); - if (this.unmounted) return; - - if (loadedEv) { - this.setState({loadedEv}); - } else { - this.setState({err: true}); + async getNextEvent(ev) { + try { + const inReplyToEventId = ReplyThread.getParentEventId(ev); + return await this.getEvent(inReplyToEventId); + } catch (e) { + return null; } } @@ -326,13 +317,18 @@ export default class ReplyThread extends React.Component { this.initialize(); } - onQuoteClick() { + async onQuoteClick() { const events = [this.state.loadedEv, ...this.state.events]; + let loadedEv = null; + if (events.length > 0) { + loadedEv = await this.getNextEvent(events[0]); + } + this.setState({ - loadedEv: null, + loadedEv, events, - }, this.loadNextEvent); + }); dis.fire(Action.FocusComposer); } From 5ba419db54476ea8a268e3816401c68c1745ee75 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 21 May 2021 10:21:54 +0100 Subject: [PATCH 20/30] split room header and header wrapper --- src/components/views/rooms/RoomHeader.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/components/views/rooms/RoomHeader.js b/src/components/views/rooms/RoomHeader.js index a527d7625c..6d3b50c10d 100644 --- a/src/components/views/rooms/RoomHeader.js +++ b/src/components/views/rooms/RoomHeader.js @@ -257,14 +257,16 @@ export default class RoomHeader extends React.Component { const e2eIcon = this.props.e2eStatus ? : undefined; return ( -
    -
    { roomAvatar }
    -
    { e2eIcon }
    - { name } - { topicElement } - { cancelButton } - { rightRow } - +
    +
    +
    { roomAvatar }
    +
    { e2eIcon }
    + { name } + { topicElement } + { cancelButton } + { rightRow } + +
    ); } From ccfd6ba4b11c649e0655c72e683e4afb2bee0b11 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 21 May 2021 12:53:26 +0100 Subject: [PATCH 21/30] fix linting issues --- src/components/structures/RoomView.tsx | 4 ++-- src/components/views/rooms/EventTile.tsx | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index 25ebbcf223..bb4e06bcfd 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -844,14 +844,14 @@ export default class RoomView extends React.Component { private onRoomName = (room: Room) => { if (this.state.room && room.roomId == this.state.room.roomId) { - // this.forceUpdate(); + this.forceUpdate(); } }; private onKeyBackupStatus = () => { // Key backup status changes affect whether the in-room recovery // reminder is displayed. - // this.forceUpdate(); + this.forceUpdate(); }; public canResetTimeline = () => { diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index 82d97bff98..bd89acaef8 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -302,6 +302,7 @@ interface IState { export default class EventTile extends React.Component { private suppressReadReceiptAnimation: boolean; private isListeningForReceipts: boolean; + private ref: React.RefObject; private tile = React.createRef(); private replyThread = React.createRef(); From c428736191837360ed3842acb965b242c1c2c4f0 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 21 May 2021 14:59:26 +0100 Subject: [PATCH 22/30] Update MessagePanel test to account for new DOM structure --- test/components/structures/MessagePanel-test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index dc70e3f7f6..5b466b4bb0 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -309,7 +309,7 @@ describe('MessagePanel', function() { const rm = TestUtils.findRenderedDOMComponentWithClass(res, 'mx_RoomView_myReadMarker_container'); // it should follow the
  • which wraps the event tile for event 4 - const eventContainer = ReactDOM.findDOMNode(tiles[4]).parentNode; + const eventContainer = ReactDOM.findDOMNode(tiles[4]); expect(rm.previousSibling).toEqual(eventContainer); }); @@ -365,7 +365,7 @@ describe('MessagePanel', function() { const tiles = TestUtils.scryRenderedComponentsWithType( mp, sdk.getComponent('rooms.EventTile')); const tileContainers = tiles.map(function(t) { - return ReactDOM.findDOMNode(t).parentNode; + return ReactDOM.findDOMNode(t); }); // find the
  • which wraps the read marker @@ -460,7 +460,7 @@ describe('MessagePanel', function() { />, ); const Dates = res.find(sdk.getComponent('messages.DateSeparator')); - + expect(Dates.length).toEqual(1); }); }); From 4851e962973f2607a5dd278feec7aee83566c647 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 24 May 2021 09:17:29 +0100 Subject: [PATCH 23/30] Switch rooms documentation and polishing --- src/components/structures/MessagePanel.js | 1 + src/components/structures/RoomView.tsx | 10 ++++++---- src/components/views/rooms/EventTile.tsx | 6 +++++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 2fb9a2df29..388c248a61 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -650,6 +650,7 @@ export default class MessagePanel extends React.Component { as="li" data-scroll-tokens={scrollToken} ref={this._collectEventNode.bind(this, eventId)} + alwaysShowTimestamps={this.props.alwaysShowTimestamps} mxEvent={mxEv} continuation={continuation} isRedacted={mxEv.isRedacted()} diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index bb4e06bcfd..2b50309d52 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -83,7 +83,7 @@ import { objectHasDiff } from "../../utils/objects"; import SpaceRoomView from "./SpaceRoomView"; import { IOpts } from "../../createRoom"; import {replaceableComponent} from "../../utils/replaceableComponent"; -import _ from 'lodash'; +import { omit } from 'lodash'; const DEBUG = false; let debuglog = function(msg: string) {}; @@ -532,14 +532,16 @@ export default class RoomView extends React.Component { shouldComponentUpdate(nextProps, nextState) { const hasPropsDiff = objectHasDiff(this.props, nextProps); + // React only shallow comparison and we only want to trigger + // a component re-render if a room requires an upgrade const newUpgradeRecommendation = nextState.upgradeRecommendation || {} - const state = _.omit(this.state, ['upgradeRecommendation']); - const newState = _.omit(nextState, ['upgradeRecommendation']) + const state = omit(this.state, ['upgradeRecommendation']); + const newState = omit(nextState, ['upgradeRecommendation']) const hasStateDiff = objectHasDiff(state, newState) || - (newUpgradeRecommendation && newUpgradeRecommendation.needsUpgrade === true) + (newUpgradeRecommendation.needsUpgrade === true) return hasPropsDiff || hasStateDiff; } diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index bd89acaef8..54f5be7f21 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -280,6 +280,9 @@ interface IProps { // Symbol of the root node as?: string + + // whether or not to always show timestamps + alwaysShowTimestamps?: boolean } interface IState { @@ -979,7 +982,8 @@ export default class EventTile extends React.Component { onFocusChange={this.onActionBarFocusChange} /> : undefined; - const timestamp = this.props.mxEvent.getTs() && this.state.hover ? + const showTimestamp = this.props.mxEvent.getTs() && (this.props.alwaysShowTimestamps || this.state.hover); + const timestamp = showTimestamp ? : null; const keyRequestHelpText = From 1a51ed9ffd78618c6bf21850eff397f3126c9fc9 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 28 May 2021 09:34:08 +0100 Subject: [PATCH 24/30] Make breadcrumb animation run on the compositing layer --- res/css/structures/_RoomView.scss | 1 + res/css/views/rooms/_RoomBreadcrumbs.scss | 6 +++--- src/components/structures/ContextMenu.tsx | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/res/css/structures/_RoomView.scss b/res/css/structures/_RoomView.scss index cdbe47178d..3d3654bda0 100644 --- a/res/css/structures/_RoomView.scss +++ b/res/css/structures/_RoomView.scss @@ -237,6 +237,7 @@ hr.mx_RoomView_myReadMarker { position: relative; top: -1px; z-index: 1; + will-change: width; transition: width 400ms easeinsine 1s, opacity 400ms easeinsine 1s; width: 99%; opacity: 1; diff --git a/res/css/views/rooms/_RoomBreadcrumbs.scss b/res/css/views/rooms/_RoomBreadcrumbs.scss index 6512797401..152b0a45cd 100644 --- a/res/css/views/rooms/_RoomBreadcrumbs.scss +++ b/res/css/views/rooms/_RoomBreadcrumbs.scss @@ -32,14 +32,14 @@ limitations under the License. // first triggering the enter state with the newest breadcrumb off screen (-40px) then // sliding it into view. &.mx_RoomBreadcrumbs-enter { - margin-left: -40px; // 32px for the avatar, 8px for the margin + transform: translateX(-40px); // 32px for the avatar, 8px for the margin } &.mx_RoomBreadcrumbs-enter-active { - margin-left: 0; + transform: translateX(0); // Timing function is as-requested by design. // NOTE: The transition time MUST match the value passed to CSSTransition! - transition: margin-left 640ms cubic-bezier(0.66, 0.02, 0.36, 1); + transition: transform 640ms cubic-bezier(0.66, 0.02, 0.36, 1); } .mx_RoomBreadcrumbs_placeholder { diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 78a9d26934..9d8665c176 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -390,7 +390,7 @@ export class ContextMenu extends React.PureComponent { } render(): React.ReactChild { - return ReactDOM.createPortal(this.renderMenu(), document.body); + return ReactDOM.createPortal(this.renderMenu(), getOrCreateContainer()); } } From 2c4fa73a4571ff3da3ae55e60ae8facc7748006e Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 2 Jun 2021 17:39:13 +0100 Subject: [PATCH 25/30] Map phone number lookup results to their native rooms When dialing a phone number, also look to see if there's a corresponding native user for the resulting user, and if so, go to the native room for that user. --- src/CallHandler.tsx | 33 +++++++++- src/VoipUserMapper.ts | 6 +- src/components/views/voip/DialPadModal.tsx | 23 ++----- src/dispatcher/actions.ts | 8 +++ test/CallHandler-test.ts | 73 ++++++++++++++-------- 5 files changed, 95 insertions(+), 48 deletions(-) diff --git a/src/CallHandler.tsx b/src/CallHandler.tsx index a05d3a25c8..ba0178fa59 100644 --- a/src/CallHandler.tsx +++ b/src/CallHandler.tsx @@ -521,7 +521,9 @@ export default class CallHandler extends EventEmitter { let newNativeAssertedIdentity = newAssertedIdentity; if (newAssertedIdentity) { const response = await this.sipNativeLookup(newAssertedIdentity); - if (response.length) newNativeAssertedIdentity = response[0].userid; + if (response.length && response[0].fields.lookup_success) { + newNativeAssertedIdentity = response[0].userid; + } } console.log(`Asserted identity ${newAssertedIdentity} mapped to ${newNativeAssertedIdentity}`); @@ -862,9 +864,38 @@ export default class CallHandler extends EventEmitter { }); break; } + case Action.DialNumber: + this.dialNumber(payload.number); + break; } } + private async dialNumber(number: string) { + const results = await this.pstnLookup(number); + if (!results || results.length === 0 || !results[0].userid) { + Modal.createTrackedDialog('', '', ErrorDialog, { + title: _t("Unable to look up phone number"), + description: _t("There was an error looking up the phone number"), + }); + return; + } + const userId = results[0].userid; + + // Now check to see if this is a virtual user, in which case we should find the + // native user + const nativeLookupResults = await this.sipNativeLookup(userId); + const lookupSuccess = nativeLookupResults.length > 0 && nativeLookupResults[0].fields.lookup_success; + const nativeUserId = lookupSuccess ? nativeLookupResults[0].userid : userId; + console.log("Looked up " + number + " to " + userId + " and mapped to native user " + nativeUserId); + + const roomId = await ensureDMExists(MatrixClientPeg.get(), nativeUserId); + + dis.dispatch({ + action: 'view_room', + room_id: roomId, + }); + } + setActiveCallRoomId(activeCallRoomId: string) { logger.info("Setting call in room " + activeCallRoomId + " active"); diff --git a/src/VoipUserMapper.ts b/src/VoipUserMapper.ts index e5bed2e812..d576a5434c 100644 --- a/src/VoipUserMapper.ts +++ b/src/VoipUserMapper.ts @@ -33,7 +33,7 @@ export default class VoipUserMapper { private async userToVirtualUser(userId: string): Promise { const results = await CallHandler.sharedInstance().sipVirtualLookup(userId); - if (results.length === 0) return null; + if (results.length === 0 || !results[0].fields.lookup_success) return null; return results[0].userid; } @@ -82,14 +82,14 @@ export default class VoipUserMapper { return Boolean(claimedNativeRoomId); } - public async onNewInvitedRoom(invitedRoom: Room) { + public async onNewInvitedRoom(invitedRoom: Room): Promise { if (!CallHandler.sharedInstance().getSupportsVirtualRooms()) return; const inviterId = invitedRoom.getDMInviter(); console.log(`Checking virtual-ness of room ID ${invitedRoom.roomId}, invited by ${inviterId}`); const result = await CallHandler.sharedInstance().sipNativeLookup(inviterId); if (result.length === 0) { - return true; + return; } if (result[0].fields.is_virtual) { diff --git a/src/components/views/voip/DialPadModal.tsx b/src/components/views/voip/DialPadModal.tsx index cdd5bc6641..f3ee49e4ac 100644 --- a/src/components/views/voip/DialPadModal.tsx +++ b/src/components/views/voip/DialPadModal.tsx @@ -15,17 +15,13 @@ limitations under the License. */ import * as React from "react"; -import { ensureDMExists } from "../../../createRoom"; import { _t } from "../../../languageHandler"; -import { MatrixClientPeg } from "../../../MatrixClientPeg"; import AccessibleButton from "../elements/AccessibleButton"; import Field from "../elements/Field"; import DialPad from './DialPad'; import dis from '../../../dispatcher/dispatcher'; -import Modal from "../../../Modal"; -import ErrorDialog from "../../views/dialogs/ErrorDialog"; -import CallHandler from "../../../CallHandler"; import {replaceableComponent} from "../../../utils/replaceableComponent"; +import { Action } from "../../../dispatcher/actions"; interface IProps { onFinished: (boolean) => void; @@ -67,21 +63,10 @@ export default class DialpadModal extends React.PureComponent { } onDialPress = async () => { - const results = await CallHandler.sharedInstance().pstnLookup(this.state.value); - if (!results || results.length === 0 || !results[0].userid) { - Modal.createTrackedDialog('', '', ErrorDialog, { - title: _t("Unable to look up phone number"), - description: _t("There was an error looking up the phone number"), - }); - } - const userId = results[0].userid; - - const roomId = await ensureDMExists(MatrixClientPeg.get(), userId); - dis.dispatch({ - action: 'view_room', - room_id: roomId, - }); + action: Action.DialNumber, + number: this.state.value, + }) this.props.onFinished(true); } diff --git a/src/dispatcher/actions.ts b/src/dispatcher/actions.ts index 9fc0b54eea..073e82bef6 100644 --- a/src/dispatcher/actions.ts +++ b/src/dispatcher/actions.ts @@ -100,6 +100,14 @@ export enum Action { */ OpenDialPad = "open_dial_pad", + /** + * Dial the phone number in the payload + * payload: { + * number: , + * } + */ + DialNumber = "dial_number", + /** * Fired when CallHandler has checked for PSTN protocol support * payload: none diff --git a/test/CallHandler-test.ts b/test/CallHandler-test.ts index 1e3f92e788..a93a134133 100644 --- a/test/CallHandler-test.ts +++ b/test/CallHandler-test.ts @@ -23,8 +23,8 @@ import dis from '../src/dispatcher/dispatcher'; import { CallEvent, CallState } from 'matrix-js-sdk/src/webrtc/call'; import DMRoomMap from '../src/utils/DMRoomMap'; import EventEmitter from 'events'; -import { Action } from '../src/dispatcher/actions'; import SdkConfig from '../src/SdkConfig'; +import { ActionPayload } from '../src/dispatcher/payloads'; const REAL_ROOM_ID = '$room1:example.org'; const MAPPED_ROOM_ID = '$room2:example.org'; @@ -75,6 +75,18 @@ class FakeCall extends EventEmitter { } } +function untilDispatch(waitForAction: string): Promise { + let dispatchHandle; + return new Promise(resolve => { + dispatchHandle = dis.register(payload => { + if (payload.action === waitForAction) { + dis.unregister(dispatchHandle); + resolve(payload); + } + }); + }); +} + describe('CallHandler', () => { let dmRoomMap; let callHandler; @@ -94,6 +106,21 @@ describe('CallHandler', () => { callHandler = new CallHandler(); callHandler.start(); + const realRoom = mkStubDM(REAL_ROOM_ID, '@user1:example.org'); + const mappedRoom = mkStubDM(MAPPED_ROOM_ID, '@user2:example.org'); + const mappedRoom2 = mkStubDM(MAPPED_ROOM_ID_2, '@user3:example.org'); + + MatrixClientPeg.get().getRoom = roomId => { + switch (roomId) { + case REAL_ROOM_ID: + return realRoom; + case MAPPED_ROOM_ID: + return mappedRoom; + case MAPPED_ROOM_ID_2: + return mappedRoom2; + } + }; + dmRoomMap = { getUserIdForRoomId: roomId => { if (roomId === REAL_ROOM_ID) { @@ -134,38 +161,34 @@ describe('CallHandler', () => { SdkConfig.unset(); }); + it('should look up the correct user and open the room when a phone number is dialled', async () => { + MatrixClientPeg.get().getThirdpartyUser = jest.fn().mockResolvedValue([{ + userid: '@user2:example.org', + protocol: "im.vector.protocol.sip_native", + fields: { + is_native: true, + lookup_success: true, + }, + }]); + + dis.dispatch({ + action: 'dial_number', + number: '01818118181', + }, true); + + const viewRoomPayload = await untilDispatch('view_room'); + expect(viewRoomPayload.room_id).toEqual(MAPPED_ROOM_ID); + }); + it('should move calls between rooms when remote asserted identity changes', async () => { - const realRoom = mkStubDM(REAL_ROOM_ID, '@user1:example.org'); - const mappedRoom = mkStubDM(MAPPED_ROOM_ID, '@user2:example.org'); - const mappedRoom2 = mkStubDM(MAPPED_ROOM_ID_2, '@user3:example.org'); - - MatrixClientPeg.get().getRoom = roomId => { - switch (roomId) { - case REAL_ROOM_ID: - return realRoom; - case MAPPED_ROOM_ID: - return mappedRoom; - case MAPPED_ROOM_ID_2: - return mappedRoom2; - } - }; - dis.dispatch({ action: 'place_call', type: PlaceCallType.Voice, room_id: REAL_ROOM_ID, }, true); - let dispatchHandle; // wait for the call to be set up - await new Promise(resolve => { - dispatchHandle = dis.register(payload => { - if (payload.action === 'call_state') { - resolve(); - } - }); - }); - dis.unregister(dispatchHandle); + await untilDispatch('call_state'); // should start off in the actual room ID it's in at the protocol level expect(callHandler.getCallForRoom(REAL_ROOM_ID)).toBe(fakeCall); From 0aeddea30f2f54d7cb06ae0a1c58be52b654d75a Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 2 Jun 2021 17:47:29 +0100 Subject: [PATCH 26/30] Only do native lookup if it's supported Also fix as bug where we were checking the wrong field to check for native/virtual support: oops. --- src/CallHandler.tsx | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/CallHandler.tsx b/src/CallHandler.tsx index ba0178fa59..0d87451b5f 100644 --- a/src/CallHandler.tsx +++ b/src/CallHandler.tsx @@ -264,7 +264,7 @@ export default class CallHandler extends EventEmitter { } public getSupportsVirtualRooms() { - return this.supportsPstnProtocol; + return this.supportsSipNativeVirtual; } public pstnLookup(phoneNumber: string): Promise { @@ -883,10 +883,15 @@ export default class CallHandler extends EventEmitter { // Now check to see if this is a virtual user, in which case we should find the // native user - const nativeLookupResults = await this.sipNativeLookup(userId); - const lookupSuccess = nativeLookupResults.length > 0 && nativeLookupResults[0].fields.lookup_success; - const nativeUserId = lookupSuccess ? nativeLookupResults[0].userid : userId; - console.log("Looked up " + number + " to " + userId + " and mapped to native user " + nativeUserId); + let nativeUserId; + if (this.getSupportsVirtualRooms()) { + const nativeLookupResults = await this.sipNativeLookup(userId); + const lookupSuccess = nativeLookupResults.length > 0 && nativeLookupResults[0].fields.lookup_success; + nativeUserId = lookupSuccess ? nativeLookupResults[0].userid : userId; + console.log("Looked up " + number + " to " + userId + " and mapped to native user " + nativeUserId); + } else { + nativeUserId = userId; + } const roomId = await ensureDMExists(MatrixClientPeg.get(), nativeUserId); From 58652152c13039c485ccd4a72a92a7915698a6e2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 2 Jun 2021 17:52:26 +0100 Subject: [PATCH 27/30] Strings are now in a different place... --- src/i18n/strings/en_EN.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 3d6fcb8643..ada264d110 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -63,6 +63,8 @@ "Already in call": "Already in call", "You're already in a call with this person.": "You're already in a call with this person.", "You cannot place a call with yourself.": "You cannot place a call with yourself.", + "Unable to look up phone number": "Unable to look up phone number", + "There was an error looking up the phone number": "There was an error looking up the phone number", "Call in Progress": "Call in Progress", "A call is currently being placed!": "A call is currently being placed!", "Permission Required": "Permission Required", @@ -898,8 +900,6 @@ "Fill Screen": "Fill Screen", "Return to call": "Return to call", "%(name)s on hold": "%(name)s on hold", - "Unable to look up phone number": "Unable to look up phone number", - "There was an error looking up the phone number": "There was an error looking up the phone number", "Dial pad": "Dial pad", "Unknown caller": "Unknown caller", "Incoming voice call": "Incoming voice call", From a34f8a29f4a58ce11ff8bbbf160ae292ac752ed0 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 3 Jun 2021 08:41:12 +0100 Subject: [PATCH 28/30] fix mx_Event containment rules and empty read avatar row --- res/css/structures/_RoomView.scss | 1 - src/components/views/rooms/EventTile.tsx | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/res/css/structures/_RoomView.scss b/res/css/structures/_RoomView.scss index b019fe7e01..09e1d58617 100644 --- a/res/css/structures/_RoomView.scss +++ b/res/css/structures/_RoomView.scss @@ -222,7 +222,6 @@ limitations under the License. .mx_RoomView_MessageList li { clear: both; - contain: content; } li.mx_RoomView_myReadMarker_container { diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index 67df5a84ba..7b652cbba2 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -631,7 +631,7 @@ export default class EventTile extends React.Component { // return early if there are no read receipts if (!this.props.readReceipts || this.props.readReceipts.length === 0) { - return (); + return null; } const ReadReceiptMarker = sdk.getComponent('rooms.ReadReceiptMarker'); From 8ef95a62378a8fca563aa094ee897da06e05c438 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 3 Jun 2021 14:38:13 +0100 Subject: [PATCH 29/30] Interface dispatcher payload & use constant in test --- src/components/views/voip/DialPadModal.tsx | 6 +++-- src/dispatcher/actions.ts | 4 +--- src/dispatcher/payloads/DialNumberPayload.ts | 23 ++++++++++++++++++++ test/CallHandler-test.ts | 4 +++- 4 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 src/dispatcher/payloads/DialNumberPayload.ts diff --git a/src/components/views/voip/DialPadModal.tsx b/src/components/views/voip/DialPadModal.tsx index f3ee49e4ac..8c0af5e81a 100644 --- a/src/components/views/voip/DialPadModal.tsx +++ b/src/components/views/voip/DialPadModal.tsx @@ -21,6 +21,7 @@ import Field from "../elements/Field"; import DialPad from './DialPad'; import dis from '../../../dispatcher/dispatcher'; import {replaceableComponent} from "../../../utils/replaceableComponent"; +import { DialNumberPayload } from "../../../dispatcher/payloads/DialNumberPayload"; import { Action } from "../../../dispatcher/actions"; interface IProps { @@ -63,10 +64,11 @@ export default class DialpadModal extends React.PureComponent { } onDialPress = async () => { - dis.dispatch({ + const payload: DialNumberPayload = { action: Action.DialNumber, number: this.state.value, - }) + }; + dis.dispatch(payload); this.props.onFinished(true); } diff --git a/src/dispatcher/actions.ts b/src/dispatcher/actions.ts index 073e82bef6..300eed2b98 100644 --- a/src/dispatcher/actions.ts +++ b/src/dispatcher/actions.ts @@ -102,9 +102,7 @@ export enum Action { /** * Dial the phone number in the payload - * payload: { - * number: , - * } + * payload: DialNumberPayload */ DialNumber = "dial_number", diff --git a/src/dispatcher/payloads/DialNumberPayload.ts b/src/dispatcher/payloads/DialNumberPayload.ts new file mode 100644 index 0000000000..1b591b9f6b --- /dev/null +++ b/src/dispatcher/payloads/DialNumberPayload.ts @@ -0,0 +1,23 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { ActionPayload } from "../payloads"; +import { Action } from "../actions"; + +export interface DialNumberPayload extends ActionPayload { + action: Action.DialNumber; + number: string; +} diff --git a/test/CallHandler-test.ts b/test/CallHandler-test.ts index a93a134133..12316ac01c 100644 --- a/test/CallHandler-test.ts +++ b/test/CallHandler-test.ts @@ -25,6 +25,8 @@ import DMRoomMap from '../src/utils/DMRoomMap'; import EventEmitter from 'events'; import SdkConfig from '../src/SdkConfig'; import { ActionPayload } from '../src/dispatcher/payloads'; +import { Actions } from '../src/notifications/types'; +import { Action } from '../src/dispatcher/actions'; const REAL_ROOM_ID = '$room1:example.org'; const MAPPED_ROOM_ID = '$room2:example.org'; @@ -172,7 +174,7 @@ describe('CallHandler', () => { }]); dis.dispatch({ - action: 'dial_number', + action: Action.DialNumber, number: '01818118181', }, true); From 5ef78f43a42005ed09f3f1bc02eb55170d8c0487 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 3 Jun 2021 16:26:20 +0100 Subject: [PATCH 30/30] fix containment rule to keep height when resizing vertically --- res/css/views/rooms/_RoomTile.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/res/css/views/rooms/_RoomTile.scss b/res/css/views/rooms/_RoomTile.scss index 421fbfe39b..03146e0325 100644 --- a/res/css/views/rooms/_RoomTile.scss +++ b/res/css/views/rooms/_RoomTile.scss @@ -19,7 +19,7 @@ limitations under the License. margin-bottom: 4px; padding: 4px; - contain: strict; + contain: content; // Not strict as it will break when resizing a sublist vertically height: 40px; box-sizing: border-box;