From ce68314de96645a0e4b0c98a2579d33855bdf2be Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 9 Jul 2020 14:43:20 +0000 Subject: [PATCH 01/23] Revert "Merge pull request #4932 from matrix-org/travis/room-list/invisible-show-more" This reverts commit f58a0a753897b00b7a8ae5647c6345bb88057aa4. --- res/css/views/rooms/_RoomSublist2.scss | 20 +++++++++++++++----- src/components/views/rooms/RoomSublist2.tsx | 10 ++-------- src/stores/room-list/ListLayout.ts | 4 ++++ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 83e7e68563..1d13f25b8f 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -187,16 +187,16 @@ limitations under the License. flex-direction: column; overflow: hidden; - .mx_RoomSublist2_placeholder { - height: 44px; // Height of a room tile plus margins - } - .mx_RoomSublist2_showNButton { cursor: pointer; font-size: $font-13px; line-height: $font-18px; color: $roomtile2-preview-color; + // This is the same color as the left panel background because it needs + // to occlude the lastmost tile in the list. + background-color: $roomlist2-bg-color; + // Update the render() function for RoomSublist2 if these change // Update the ListLayout class for minVisibleTiles if these change. // @@ -209,7 +209,7 @@ limitations under the License. // We force this to the bottom so it will overlap rooms as needed. // We account for the space it takes up (24px) in the code through padding. position: absolute; - bottom: 0; + bottom: 0; // the height of the resize handle left: 0; right: 0; @@ -236,6 +236,16 @@ limitations under the License. .mx_RoomSublist2_showLessButtonChevron { mask-image: url('$(res)/img/feather-customised/chevron-up.svg'); } + + &.mx_RoomSublist2_isCutting::before { + content: ''; + position: absolute; + top: 0; + left: 0; + right: 0; + height: 4px; + box-shadow: 0px -2px 3px rgba(46, 47, 50, 0.08); + } } // Class name comes from the ResizableBox component diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index c3ac85e2de..73aa97b6e8 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -562,8 +562,10 @@ export default class RoomSublist2 extends React.Component { if (visibleTiles.length > 0) { const layout = this.layout; // to shorten calls + const maxTilesFactored = layout.tilesWithResizerBoxFactor(this.numTiles); const showMoreBtnClasses = classNames({ 'mx_RoomSublist2_showNButton': true, + 'mx_RoomSublist2_isCutting': this.state.isResizing && layout.visibleTiles < maxTilesFactored, }); // If we're hiding rooms, show a 'show more' button to the user. This button @@ -642,14 +644,6 @@ export default class RoomSublist2 extends React.Component { const tilesWithoutPadding = Math.min(relativeTiles, layout.visibleTiles); const tilesPx = layout.calculateTilesToPixelsMin(relativeTiles, tilesWithoutPadding, padding); - // Now that we know our padding constraints, let's find out if we need to chop off the - // last rendered visible tile so it doesn't collide with the 'show more' button - let visibleUnpaddedTiles = Math.round(layout.visibleTiles - layout.pixelsToTiles(padding)); - if (visibleUnpaddedTiles === visibleTiles.length - 1) { - const placeholder =
; - visibleTiles.splice(visibleUnpaddedTiles, 1, placeholder); - } - const dimensions = { height: tilesPx, }; diff --git a/src/stores/room-list/ListLayout.ts b/src/stores/room-list/ListLayout.ts index 5169c5e4e5..99674fe74f 100644 --- a/src/stores/room-list/ListLayout.ts +++ b/src/stores/room-list/ListLayout.ts @@ -109,6 +109,10 @@ export class ListLayout { return this.tilesToPixels(Math.min(maxTiles, n)) + padding; } + public tilesWithResizerBoxFactor(n: number): number { + return n + RESIZER_BOX_FACTOR; + } + public tilesWithPadding(n: number, paddingPx: number): number { return this.pixelsToTiles(this.tilesToPixelsWithPadding(n, paddingPx)); } From e2539f11cd156779d152ad19c56ea0a977ab9ec2 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 9 Jul 2020 19:24:02 +0100 Subject: [PATCH 02/23] Scroll fade for breadcrumbs --- res/css/structures/_LeftPanel2.scss | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/res/css/structures/_LeftPanel2.scss b/res/css/structures/_LeftPanel2.scss index eaa22a3efa..0b87ac0469 100644 --- a/res/css/structures/_LeftPanel2.scss +++ b/res/css/structures/_LeftPanel2.scss @@ -73,6 +73,18 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations overflow-y: hidden; overflow-x: scroll; margin-top: 8px; + + &.mx_IndicatorScrollbar_leftOverflow { + mask-image: linear-gradient(90deg, transparent, black 10%); + } + + &.mx_IndicatorScrollbar_rightOverflow { + mask-image: linear-gradient(90deg, black, black 90%, transparent); + } + + &.mx_IndicatorScrollbar_rightOverflow.mx_IndicatorScrollbar_leftOverflow { + mask-image: linear-gradient(90deg, transparent, black 10%, black 90%, transparent); + } } } From 859f65659c03d2b930f62d142651cd5116c21dd0 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Jul 2020 13:07:13 -0600 Subject: [PATCH 03/23] Attempt to support a hard cutoff with the show more button Known issues: * Causes scroll jumps when the button gets added to DOM * Resize handle is invisible when there's a show more button TODO: * Clean up comments * Clean up useless code (all the padding stuff isn't needed) --- res/css/views/rooms/_RoomSublist2.scss | 120 ++++++++++---------- src/components/views/rooms/RoomSublist2.tsx | 41 ++++--- src/stores/room-list/ListLayout.ts | 3 +- 3 files changed, 84 insertions(+), 80 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 1d13f25b8f..6a77056917 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -179,7 +179,6 @@ limitations under the License. } .mx_RoomSublist2_resizeBox { - margin-bottom: 4px; // for the resize handle position: relative; // Create another flexbox column for the tiles @@ -187,65 +186,12 @@ limitations under the License. flex-direction: column; overflow: hidden; - .mx_RoomSublist2_showNButton { - cursor: pointer; - font-size: $font-13px; - line-height: $font-18px; - color: $roomtile2-preview-color; - - // This is the same color as the left panel background because it needs - // to occlude the lastmost tile in the list. - background-color: $roomlist2-bg-color; - - // Update the render() function for RoomSublist2 if these change - // Update the ListLayout class for minVisibleTiles if these change. - // - // At 24px high, 8px padding on the top and 4px padding on the bottom this equates to 0.73 of - // a tile due to how the padding calculations work. - height: 24px; - padding-top: 8px; - padding-bottom: 4px; - - // We force this to the bottom so it will overlap rooms as needed. - // We account for the space it takes up (24px) in the code through padding. + .mx_RoomSublist2_resizerHandles_showNButton { position: absolute; - bottom: 0; // the height of the resize handle + bottom: -32px; // height of the button left: 0; right: 0; - - // We create a flexbox to cheat at alignment - display: flex; - align-items: center; - - .mx_RoomSublist2_showNButtonChevron { - position: relative; - width: 16px; - height: 16px; - margin-left: 12px; - margin-right: 18px; - mask-position: center; - mask-size: contain; - mask-repeat: no-repeat; - background: $roomtile2-preview-color; - } - - .mx_RoomSublist2_showMoreButtonChevron { - mask-image: url('$(res)/img/feather-customised/chevron-down.svg'); - } - - .mx_RoomSublist2_showLessButtonChevron { - mask-image: url('$(res)/img/feather-customised/chevron-up.svg'); - } - - &.mx_RoomSublist2_isCutting::before { - content: ''; - position: absolute; - top: 0; - left: 0; - right: 0; - height: 4px; - box-shadow: 0px -2px 3px rgba(46, 47, 50, 0.08); - } + height: 4px; // height of the handle } // Class name comes from the ResizableBox component @@ -277,6 +223,56 @@ limitations under the License. } } + .mx_RoomSublist2_showNButton { + cursor: pointer; + font-size: $font-13px; + line-height: $font-18px; + color: $roomtile2-preview-color; + + // Update the render() function for RoomSublist2 if these change + // Update the ListLayout class for minVisibleTiles if these change. + // + // At 24px high, 8px padding on the top and 4px padding on the bottom this equates to 0.73 of + // a tile due to how the padding calculations work. + height: 24px; + padding-top: 8px; + padding-bottom: 4px; + + // We create a flexbox to cheat at alignment + display: flex; + align-items: center; + + .mx_RoomSublist2_showNButtonChevron { + position: relative; + width: 16px; + height: 16px; + margin-left: 12px; + margin-right: 18px; + mask-position: center; + mask-size: contain; + mask-repeat: no-repeat; + background: $roomtile2-preview-color; + } + + .mx_RoomSublist2_showMoreButtonChevron { + mask-image: url('$(res)/img/feather-customised/chevron-down.svg'); + } + + .mx_RoomSublist2_showLessButtonChevron { + mask-image: url('$(res)/img/feather-customised/chevron-up.svg'); + } + + &.mx_RoomSublist2_isCutting::before { + content: ''; + position: absolute; + top: 0; + left: 0; + right: 0; + height: 4px; + box-shadow: 0px -2px 3px rgba(46, 47, 50, 0.08); + } + } + &.mx_RoomSublist2_hasMenuOpen, &:not(.mx_RoomSublist2_minimized) > .mx_RoomSublist2_headerContainer:focus-within, &:not(.mx_RoomSublist2_minimized) > .mx_RoomSublist2_headerContainer:hover { @@ -322,13 +318,13 @@ limitations under the License. .mx_RoomSublist2_resizeBox { align-items: center; + } - .mx_RoomSublist2_showNButton { - flex-direction: column; + .mx_RoomSublist2_showNButton { + flex-direction: column; - .mx_RoomSublist2_showNButtonChevron { - margin-right: 12px; // to center - } + .mx_RoomSublist2_showNButtonChevron { + margin-right: 12px; // to center } } diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 73aa97b6e8..caa679f1d0 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -119,7 +119,7 @@ export default class RoomSublist2 extends React.Component { } private get numVisibleTiles(): number { - const nVisible = Math.floor(this.layout.visibleTiles); + const nVisible = Math.ceil(this.layout.visibleTiles); return Math.min(nVisible, this.numTiles); } @@ -635,8 +635,8 @@ export default class RoomSublist2 extends React.Component { // The padding is variable though, so figure out what we need padding for. let padding = 0; - if (showNButton) padding += SHOW_N_BUTTON_HEIGHT; - padding += RESIZE_HANDLE_HEIGHT; // always append the handle height + //if (showNButton) padding += SHOW_N_BUTTON_HEIGHT; + //padding += RESIZE_HANDLE_HEIGHT; // always append the handle height const relativeTiles = layout.tilesWithPadding(this.numTiles, padding); const minTilesPx = layout.calculateTilesToPixelsMin(relativeTiles, layout.minVisibleTiles, padding); @@ -644,25 +644,32 @@ export default class RoomSublist2 extends React.Component { const tilesWithoutPadding = Math.min(relativeTiles, layout.visibleTiles); const tilesPx = layout.calculateTilesToPixelsMin(relativeTiles, tilesWithoutPadding, padding); + const handleWrapperClasses = classNames({ + 'mx_RoomSublist2_resizerHandles': true, + 'mx_RoomSublist2_resizerHandles_showNButton': !!showNButton, + }); + const dimensions = { height: tilesPx, }; content = ( - - {visibleTiles} + + + {visibleTiles} + {showNButton} - + ); } diff --git a/src/stores/room-list/ListLayout.ts b/src/stores/room-list/ListLayout.ts index 99674fe74f..f1900487bc 100644 --- a/src/stores/room-list/ListLayout.ts +++ b/src/stores/room-list/ListLayout.ts @@ -20,7 +20,8 @@ const TILE_HEIGHT_PX = 44; // this comes from the CSS where the show more button is // mathematically this percent of a tile when floating. -const RESIZER_BOX_FACTOR = 0.78; +//const RESIZER_BOX_FACTOR = 0.78; +const RESIZER_BOX_FACTOR = 0; interface ISerializedListLayout { numTiles: number; From 1315f34662426b0b6be9ddf5002d0719224903d5 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Jul 2020 14:52:54 -0600 Subject: [PATCH 04/23] Dedupe room list store updates by marking for updates The core of this is in the MarkedExecution class, with the remainder of the commit ensuring that the right marks and triggers are in place to do the firing. Because everything is async/await and run through the RoomListStore, we don't have to worry about self-fed updates in the algorithm classes. This also means we have to trigger pretty much all the time. Changes to tag ordering / list sorting get hit through two paths, so we mark before we do a bulk update and otherwise assume the call is coming in from outside. --- src/stores/room-list/RoomListStore2.ts | 54 +++++++++++----- src/stores/room-list/algorithms/Algorithm.ts | 10 +-- src/utils/MarkedExecution.ts | 67 ++++++++++++++++++++ 3 files changed, 108 insertions(+), 23 deletions(-) create mode 100644 src/utils/MarkedExecution.ts diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 6020e46a12..c29c0b5a20 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -33,6 +33,7 @@ import { EffectiveMembership, getEffectiveMembership } from "./membership"; import { ListLayout } from "./ListLayout"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; import RoomListLayoutStore from "./RoomListLayoutStore"; +import { MarkedExecution } from "../../utils/MarkedExecution"; interface IState { tagsEnabled?: boolean; @@ -51,7 +52,7 @@ export class RoomListStore2 extends AsyncStore { private algorithm = new Algorithm(); private filterConditions: IFilterCondition[] = []; private tagWatcher = new TagWatcher(this); - private layoutMap: Map = new Map(); + private updateFn = new MarkedExecution(() => this.emit(LISTS_UPDATE_EVENT)); private readonly watchedSettings = [ 'feature_custom_tags', @@ -91,24 +92,26 @@ export class RoomListStore2 extends AsyncStore { await this.updateAlgorithmInstances(); } - private onRVSUpdate = () => { + private onRVSUpdate = async (quiet = false) => { if (!this.enabled) return; // TODO: Remove with https://github.com/vector-im/riot-web/issues/14231 if (!this.matrixClient) return; // We assume there won't be RVS updates without a client const activeRoomId = RoomViewStore.getRoomId(); if (!activeRoomId && this.algorithm.stickyRoom) { - this.algorithm.stickyRoom = null; + await this.algorithm.setStickyRoom(null); } else if (activeRoomId) { const activeRoom = this.matrixClient.getRoom(activeRoomId); if (!activeRoom) { console.warn(`${activeRoomId} is current in RVS but missing from client - clearing sticky room`); - this.algorithm.stickyRoom = null; + await this.algorithm.setStickyRoom(null); } else if (activeRoom !== this.algorithm.stickyRoom) { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`Changing sticky room to ${activeRoomId}`); - this.algorithm.stickyRoom = activeRoom; + await this.algorithm.setStickyRoom(activeRoom); } } + + if (!quiet) this.updateFn.trigger(); }; protected async onDispatch(payload: ActionPayload) { @@ -127,8 +130,12 @@ export class RoomListStore2 extends AsyncStore { // Update any settings here, as some may have happened before we were logically ready. console.log("Regenerating room lists: Startup"); await this.readAndCacheSettingsFromStore(); - await this.regenerateAllLists(); - this.onRVSUpdate(); // fake an RVS update to adjust sticky room, if needed + await this.regenerateAllLists(true); + await this.onRVSUpdate(true); // fake an RVS update to adjust sticky room, if needed + + this.updateFn.trigger(); + + return; // no point in running the next conditions - they won't match } // TODO: Remove this once the RoomListStore becomes default @@ -137,7 +144,7 @@ export class RoomListStore2 extends AsyncStore { if (payload.action === 'on_client_not_viable' || payload.action === 'on_logged_out') { // Reset state without causing updates as the client will have been destroyed // and downstream code will throw NPE errors. - this.reset(null, true); + await this.reset(null, true); this._matrixClient = null; this.initialListsGenerated = false; // we'll want to regenerate them } @@ -151,7 +158,8 @@ export class RoomListStore2 extends AsyncStore { console.log("Regenerating room lists: Settings changed"); await this.readAndCacheSettingsFromStore(); - await this.regenerateAllLists(); // regenerate the lists now + await this.regenerateAllLists(true); // regenerate the lists now + this.updateFn.trigger(); } } @@ -172,6 +180,7 @@ export class RoomListStore2 extends AsyncStore { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[RoomListDebug] Got own read receipt in ${room.roomId}`); await this.handleRoomUpdate(room, RoomUpdateCause.ReadReceipt); + this.updateFn.trigger(); return; } } else if (payload.action === 'MatrixActions.Room.tags') { @@ -179,6 +188,7 @@ export class RoomListStore2 extends AsyncStore { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[RoomListDebug] Got tag change in ${roomPayload.room.roomId}`); await this.handleRoomUpdate(roomPayload.room, RoomUpdateCause.PossibleTagChange); + this.updateFn.trigger(); } else if (payload.action === 'MatrixActions.Room.timeline') { const eventPayload = (payload); // TODO: Type out the dispatcher types @@ -202,6 +212,7 @@ export class RoomListStore2 extends AsyncStore { } } await this.handleRoomUpdate(updatedRoom, RoomUpdateCause.Timeline); + this.updateFn.trigger(); }; if (!room) { console.warn(`Live timeline event ${eventPayload.event.getId()} received without associated room`); @@ -225,6 +236,7 @@ export class RoomListStore2 extends AsyncStore { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[RoomListDebug] Decrypted timeline event ${eventPayload.event.getId()} in ${roomId}`); await this.handleRoomUpdate(room, RoomUpdateCause.Timeline); + this.updateFn.trigger(); } else if (payload.action === 'MatrixActions.accountData' && payload.event_type === 'm.direct') { const eventPayload = (payload); // TODO: Type out the dispatcher types // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 @@ -246,6 +258,7 @@ export class RoomListStore2 extends AsyncStore { await this.handleRoomUpdate(room, RoomUpdateCause.PossibleTagChange); } } + this.updateFn.trigger(); } else if (payload.action === 'MatrixActions.Room.myMembership') { const membershipPayload = (payload); // TODO: Type out the dispatcher types const oldMembership = getEffectiveMembership(membershipPayload.oldMembership); @@ -264,7 +277,7 @@ export class RoomListStore2 extends AsyncStore { const isSticky = this.algorithm.stickyRoom === prevRoom; if (isSticky) { console.log(`[RoomListDebug] Clearing sticky room due to room upgrade`); - await this.algorithm.setStickyRoomAsync(null); + await this.algorithm.setStickyRoom(null); } // Note: we hit the algorithm instead of our handleRoomUpdate() function to @@ -276,6 +289,7 @@ export class RoomListStore2 extends AsyncStore { console.log(`[RoomListDebug] Adding new room to room list`); await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom); + this.updateFn.trigger(); return; } @@ -283,6 +297,7 @@ export class RoomListStore2 extends AsyncStore { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[RoomListDebug] Handling invite to ${membershipPayload.room.roomId}`); await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom); + this.updateFn.trigger(); return; } @@ -291,6 +306,7 @@ export class RoomListStore2 extends AsyncStore { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[RoomListDebug] Handling membership change in ${membershipPayload.room.roomId}`); await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.PossibleTagChange); + this.updateFn.trigger(); return; } } @@ -301,7 +317,7 @@ export class RoomListStore2 extends AsyncStore { if (shouldUpdate) { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log(`[DEBUG] Room "${room.name}" (${room.roomId}) triggered by ${cause} requires list update`); - this.emit(LISTS_UPDATE_EVENT, this); + this.updateFn.mark(); } } @@ -309,6 +325,7 @@ export class RoomListStore2 extends AsyncStore { await this.algorithm.setTagSorting(tagId, sort); // TODO: Per-account? https://github.com/vector-im/riot-web/issues/14114 localStorage.setItem(`mx_tagSort_${tagId}`, sort); + this.updateFn.triggerIfWillMark(); } public getTagSorting(tagId: TagID): SortAlgorithm { @@ -347,6 +364,7 @@ export class RoomListStore2 extends AsyncStore { await this.algorithm.setListOrdering(tagId, order); // TODO: Per-account? https://github.com/vector-im/riot-web/issues/14114 localStorage.setItem(`mx_listOrder_${tagId}`, order); + this.updateFn.triggerIfWillMark(); } public getListOrder(tagId: TagID): ListAlgorithm { @@ -382,6 +400,10 @@ export class RoomListStore2 extends AsyncStore { } private async updateAlgorithmInstances() { + // We'll require an update, so mark for one. Marking now also prevents the calls + // to setTagSorting and setListOrder from causing triggers. + this.updateFn.mark(); + for (const tag of Object.keys(this.orderedLists)) { const definedSort = this.getTagSorting(tag); const definedOrder = this.getListOrder(tag); @@ -406,11 +428,11 @@ export class RoomListStore2 extends AsyncStore { private onAlgorithmListUpdated = () => { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log("Underlying algorithm has triggered a list update - refiring"); - this.emit(LISTS_UPDATE_EVENT, this); + console.log("Underlying algorithm has triggered a list update - marking"); + this.updateFn.mark(); }; - private async regenerateAllLists() { + private async regenerateAllLists(quiet = false) { console.warn("Regenerating all room lists"); const sorts: ITagSortingMap = {}; @@ -435,7 +457,7 @@ export class RoomListStore2 extends AsyncStore { this.initialListsGenerated = true; - this.emit(LISTS_UPDATE_EVENT, this); + if (!quiet) this.updateFn.trigger(); } public addFilter(filter: IFilterCondition): void { @@ -445,6 +467,7 @@ export class RoomListStore2 extends AsyncStore { if (this.algorithm) { this.algorithm.addFilterCondition(filter); } + this.updateFn.trigger(); } public removeFilter(filter: IFilterCondition): void { @@ -458,6 +481,7 @@ export class RoomListStore2 extends AsyncStore { this.algorithm.removeFilterCondition(filter); } } + this.updateFn.trigger(); } /** diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 35511a461d..2cd767682b 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -87,12 +87,6 @@ export class Algorithm extends EventEmitter { return this._stickyRoom ? this._stickyRoom.room : null; } - public set stickyRoom(val: Room) { - // setters can't be async, so we call a private function to do the work - // noinspection JSIgnoredPromiseFromCall - this.updateStickyRoom(val); - } - protected get hasFilters(): boolean { return this.allowedByFilter.size > 0; } @@ -115,7 +109,7 @@ export class Algorithm extends EventEmitter { * Awaitable version of the sticky room setter. * @param val The new room to sticky. */ - public async setStickyRoomAsync(val: Room) { + public async setStickyRoom(val: Room) { await this.updateStickyRoom(val); } @@ -746,7 +740,7 @@ export class Algorithm extends EventEmitter { }; } else { // We have to clear the lock as the sticky room change will trigger updates. - await this.setStickyRoomAsync(room); + await this.setStickyRoom(room); } } } diff --git a/src/utils/MarkedExecution.ts b/src/utils/MarkedExecution.ts new file mode 100644 index 0000000000..d866720ecd --- /dev/null +++ b/src/utils/MarkedExecution.ts @@ -0,0 +1,67 @@ +/* +Copyright 2020 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/** + * A utility to ensure that a function is only called once triggered with + * a mark applied. Multiple marks can be applied to the function, however + * the function will only be called once upon trigger(). + * + * The function starts unmarked. + */ +export class MarkedExecution { + private marked = false; + + /** + * Creates a MarkedExecution for the provided function. + * @param fn The function to be called upon trigger if marked. + */ + constructor(private fn: () => void) { + } + + /** + * Resets the mark without calling the function. + */ + public reset() { + this.marked = false; + } + + /** + * Marks the function to be called upon trigger(). + */ + public mark() { + this.marked = true; + } + + /** + * If marked, the function will be called, otherwise this does nothing. + */ + public trigger() { + if (!this.marked) return; + this.fn(); + this.reset(); + } + + /** + * Triggers the function if a mark() call would mark it. If the function + * has already been marked this will do nothing. + */ + public triggerIfWillMark() { + if (!this.marked) { + this.mark(); + this.trigger(); + } + } +} From 8624e8beeb66d75350aa4ea16176de1e4409a42d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Jul 2020 15:11:21 -0600 Subject: [PATCH 05/23] Break up the event loop tasks for the room list The room list does a hefty amount of work, so instead of blocking the event loop with a `/sync` request and a bunch of room updates (as we can get multiple per sync) we can instead run it over several smaller tasks. The smaller tasks help the event loop do other things between our tasks, ensuring we don't inadvertently block the UI from rendering too slowly. On my account and machine, this cuts the time to render in half (~30ms, down from ~60ms) . --- src/stores/room-list/RoomListStore2.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index c29c0b5a20..60f888f8ed 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -114,7 +114,13 @@ export class RoomListStore2 extends AsyncStore { if (!quiet) this.updateFn.trigger(); }; - protected async onDispatch(payload: ActionPayload) { + protected onDispatch(payload: ActionPayload) { + // We do this to intentionally break out of the current event loop task, allowing + // us to instead wait for a more convenient time to run our updates. + setImmediate(() => this.onDispatchAsync(payload)); + } + + protected async onDispatchAsync(payload: ActionPayload) { if (payload.action === 'MatrixActions.sync') { // Filter out anything that isn't the first PREPARED sync. if (!(payload.prevState === 'PREPARED' && payload.state !== 'PREPARED')) { From cf154ec9cf551724c38c6fffa55be7cbcd60fe62 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Jul 2020 18:19:38 -0600 Subject: [PATCH 06/23] Add an option to disable room list logging, and improve logging For https://github.com/vector-im/riot-web/issues/14035 **This option is not recommended as it completely obliterates all chances of being able to support someone with a broken room list. It is intended for specific testing scenarios only.** --- src/@types/global.d.ts | 3 + src/components/views/rooms/RoomList2.tsx | 5 +- src/components/views/rooms/RoomTile2.tsx | 2 +- src/stores/room-list/RoomListStore2.ts | 106 +++++++++++----- src/stores/room-list/algorithms/Algorithm.ts | 116 ++++++++++-------- .../list-ordering/ImportanceAlgorithm.ts | 3 - .../list-ordering/NaturalAlgorithm.ts | 3 - .../filters/CommunityFilterCondition.ts | 2 - .../room-list/filters/NameFilterCondition.ts | 2 - 9 files changed, 149 insertions(+), 93 deletions(-) diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index fc52296d8b..3f970ea8c3 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -37,6 +37,9 @@ declare global { mx_RoomListStore2: RoomListStore2; mx_RoomListLayoutStore: RoomListLayoutStore; mxPlatformPeg: PlatformPeg; + + // TODO: Remove flag before launch: https://github.com/vector-im/riot-web/issues/14231 + mx_QuietRoomListLogging: boolean; } // workaround for https://github.com/microsoft/TypeScript/issues/30933 diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index 3a3ae3707e..3774bb1cd1 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -219,7 +219,10 @@ export default class RoomList2 extends React.Component { private updateLists = () => { const newLists = RoomListStore.instance.orderedLists; - console.log("new lists", newLists); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log("new lists", newLists); + } this.setState({sublists: newLists}, () => { this.props.onResize(); diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index db8084baa2..a7dc983fa6 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -256,7 +256,7 @@ export default class RoomTile2 extends React.Component { 0 )); } else { - console.log(`Unexpected tag ${tagId} applied to ${this.props.room.room_id}`); + console.warn(`Unexpected tag ${tagId} applied to ${this.props.room.room_id}`); } if ((ev as React.KeyboardEvent).key === Key.ENTER) { diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 6020e46a12..6876454052 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -104,8 +104,10 @@ export class RoomListStore2 extends AsyncStore { console.warn(`${activeRoomId} is current in RVS but missing from client - clearing sticky room`); this.algorithm.stickyRoom = null; } else if (activeRoom !== this.algorithm.stickyRoom) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`Changing sticky room to ${activeRoomId}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Changing sticky room to ${activeRoomId}`); + } this.algorithm.stickyRoom = activeRoom; } } @@ -169,15 +171,19 @@ export class RoomListStore2 extends AsyncStore { console.warn(`Own read receipt was in unknown room ${room.roomId}`); return; } - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Got own read receipt in ${room.roomId}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Got own read receipt in ${room.roomId}`); + } await this.handleRoomUpdate(room, RoomUpdateCause.ReadReceipt); return; } } else if (payload.action === 'MatrixActions.Room.tags') { const roomPayload = (payload); // TODO: Type out the dispatcher types - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Got tag change in ${roomPayload.room.roomId}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Got tag change in ${roomPayload.room.roomId}`); + } await this.handleRoomUpdate(roomPayload.room, RoomUpdateCause.PossibleTagChange); } else if (payload.action === 'MatrixActions.Room.timeline') { const eventPayload = (payload); // TODO: Type out the dispatcher types @@ -188,12 +194,16 @@ export class RoomListStore2 extends AsyncStore { const roomId = eventPayload.event.getRoomId(); const room = this.matrixClient.getRoom(roomId); const tryUpdate = async (updatedRoom: Room) => { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Live timeline event ${eventPayload.event.getId()}` + - ` in ${updatedRoom.roomId}`); - if (eventPayload.event.getType() === 'm.room.tombstone' && eventPayload.event.getStateKey() === '') { + if (!window.mx_QuietRoomListLogging) { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Got tombstone event - trying to remove now-dead room`); + console.log(`[RoomListDebug] Live timeline event ${eventPayload.event.getId()}` + + ` in ${updatedRoom.roomId}`); + } + if (eventPayload.event.getType() === 'm.room.tombstone' && eventPayload.event.getStateKey() === '') { + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Got tombstone event - trying to remove now-dead room`); + } const newRoom = this.matrixClient.getRoom(eventPayload.event.getContent()['replacement_room']); if (newRoom) { // If we have the new room, then the new room check will have seen the predecessor @@ -222,13 +232,17 @@ export class RoomListStore2 extends AsyncStore { console.warn(`Event ${eventPayload.event.getId()} was decrypted in an unknown room ${roomId}`); return; } - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Decrypted timeline event ${eventPayload.event.getId()} in ${roomId}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Decrypted timeline event ${eventPayload.event.getId()} in ${roomId}`); + } await this.handleRoomUpdate(room, RoomUpdateCause.Timeline); } else if (payload.action === 'MatrixActions.accountData' && payload.event_type === 'm.direct') { const eventPayload = (payload); // TODO: Type out the dispatcher types - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Received updated DM map`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Received updated DM map`); + } const dmMap = eventPayload.event.getContent(); for (const userId of Object.keys(dmMap)) { const roomIds = dmMap[userId]; @@ -251,45 +265,63 @@ export class RoomListStore2 extends AsyncStore { const oldMembership = getEffectiveMembership(membershipPayload.oldMembership); const newMembership = getEffectiveMembership(membershipPayload.membership); if (oldMembership !== EffectiveMembership.Join && newMembership === EffectiveMembership.Join) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Handling new room ${membershipPayload.room.roomId}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Handling new room ${membershipPayload.room.roomId}`); + } // If we're joining an upgraded room, we'll want to make sure we don't proliferate // the dead room in the list. const createEvent = membershipPayload.room.currentState.getStateEvents("m.room.create", ""); if (createEvent && createEvent.getContent()['predecessor']) { - console.log(`[RoomListDebug] Room has a predecessor`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Room has a predecessor`); + } const prevRoom = this.matrixClient.getRoom(createEvent.getContent()['predecessor']['room_id']); if (prevRoom) { const isSticky = this.algorithm.stickyRoom === prevRoom; if (isSticky) { - console.log(`[RoomListDebug] Clearing sticky room due to room upgrade`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Clearing sticky room due to room upgrade`); + } await this.algorithm.setStickyRoomAsync(null); } // Note: we hit the algorithm instead of our handleRoomUpdate() function to // avoid redundant updates. - console.log(`[RoomListDebug] Removing previous room from room list`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Removing previous room from room list`); + } await this.algorithm.handleRoomUpdate(prevRoom, RoomUpdateCause.RoomRemoved); } } - console.log(`[RoomListDebug] Adding new room to room list`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Adding new room to room list`); + } await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom); return; } if (oldMembership !== EffectiveMembership.Invite && newMembership === EffectiveMembership.Invite) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Handling invite to ${membershipPayload.room.roomId}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Handling invite to ${membershipPayload.room.roomId}`); + } await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom); return; } // If it's not a join, it's transitioning into a different list (possibly historical) if (oldMembership !== newMembership) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Handling membership change in ${membershipPayload.room.roomId}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[RoomListDebug] Handling membership change in ${membershipPayload.room.roomId}`); + } await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.PossibleTagChange); return; } @@ -299,8 +331,10 @@ export class RoomListStore2 extends AsyncStore { private async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { const shouldUpdate = await this.algorithm.handleRoomUpdate(room, cause); if (shouldUpdate) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[DEBUG] Room "${room.name}" (${room.roomId}) triggered by ${cause} requires list update`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[DEBUG] Room "${room.name}" (${room.roomId}) triggered by ${cause} requires list update`); + } this.emit(LISTS_UPDATE_EVENT, this); } } @@ -405,8 +439,10 @@ export class RoomListStore2 extends AsyncStore { } private onAlgorithmListUpdated = () => { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log("Underlying algorithm has triggered a list update - refiring"); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log("Underlying algorithm has triggered a list update - refiring"); + } this.emit(LISTS_UPDATE_EVENT, this); }; @@ -439,8 +475,10 @@ export class RoomListStore2 extends AsyncStore { } public addFilter(filter: IFilterCondition): void { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log("Adding filter condition:", filter); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log("Adding filter condition:", filter); + } this.filterConditions.push(filter); if (this.algorithm) { this.algorithm.addFilterCondition(filter); @@ -448,8 +486,10 @@ export class RoomListStore2 extends AsyncStore { } public removeFilter(filter: IFilterCondition): void { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log("Removing filter condition:", filter); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log("Removing filter condition:", filter); + } const idx = this.filterConditions.indexOf(filter); if (idx >= 0) { this.filterConditions.splice(idx, 1); diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 35511a461d..6147de1500 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -321,8 +321,10 @@ export class Algorithm extends EventEmitter { } newMap[tagId] = allowedRoomsInThisTag; - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[DEBUG] ${newMap[tagId].length}/${rooms.length} rooms filtered into ${tagId}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[DEBUG] ${newMap[tagId].length}/${rooms.length} rooms filtered into ${tagId}`); + } } const allowedRooms = Object.values(newMap).reduce((rv, v) => { rv.push(...v); return rv; }, []); @@ -331,26 +333,13 @@ export class Algorithm extends EventEmitter { this.emit(LIST_UPDATED_EVENT); } - // TODO: Remove or use. - protected addPossiblyFilteredRoomsToTag(tagId: TagID, added: Room[]): void { - const filters = this.allowedByFilter.keys(); - for (const room of added) { - for (const filter of filters) { - if (filter.isVisible(room)) { - this.allowedRoomsByFilters.add(room); - break; - } - } - } - - // Now that we've updated the allowed rooms, recalculate the tag - this.recalculateFilteredRoomsForTag(tagId); - } - protected recalculateFilteredRoomsForTag(tagId: TagID): void { if (!this.hasFilters) return; // don't bother doing work if there's nothing to do - console.log(`Recalculating filtered rooms for ${tagId}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Recalculating filtered rooms for ${tagId}`); + } delete this.filteredRooms[tagId]; const rooms = this.cachedRooms[tagId].map(r => r); // cheap clone this.tryInsertStickyRoomToFilterSet(rooms, tagId); @@ -359,8 +348,10 @@ export class Algorithm extends EventEmitter { this.filteredRooms[tagId] = filteredRooms; } - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[DEBUG] ${filteredRooms.length}/${rooms.length} rooms filtered into ${tagId}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`[DEBUG] ${filteredRooms.length}/${rooms.length} rooms filtered into ${tagId}`); + } } protected tryInsertStickyRoomToFilterSet(rooms: Room[], tagId: TagID) { @@ -399,8 +390,10 @@ export class Algorithm extends EventEmitter { } if (!this._cachedStickyRooms || !updatedTag) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`Generating clone of cached rooms for sticky room handling`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Generating clone of cached rooms for sticky room handling`); + } const stickiedTagMap: ITagMap = {}; for (const tagId of Object.keys(this.cachedRooms)) { stickiedTagMap[tagId] = this.cachedRooms[tagId].map(r => r); // shallow clone @@ -411,8 +404,10 @@ export class Algorithm extends EventEmitter { if (updatedTag) { // Update the tag indicated by the caller, if possible. This is mostly to ensure // our cache is up to date. - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`Replacing cached sticky rooms for ${updatedTag}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Replacing cached sticky rooms for ${updatedTag}`); + } this._cachedStickyRooms[updatedTag] = this.cachedRooms[updatedTag].map(r => r); // shallow clone } @@ -421,8 +416,10 @@ export class Algorithm extends EventEmitter { // we might have updated from the cache is also our sticky room. const sticky = this._stickyRoom; if (!updatedTag || updatedTag === sticky.tag) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`Inserting sticky room ${sticky.room.roomId} at position ${sticky.position} in ${sticky.tag}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Inserting sticky room ${sticky.room.roomId} at position ${sticky.position} in ${sticky.tag}`); + } this._cachedStickyRooms[sticky.tag].splice(sticky.position, 0, sticky.room); } @@ -647,8 +644,10 @@ 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.log(`Handle room update for ${room.roomId} called with cause ${cause}`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + 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"); // Note: check the isSticky against the room ID just in case the reference is wrong @@ -705,16 +704,20 @@ export class Algorithm extends EventEmitter { 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}`); + if (!window.mx_QuietRoomListLogging) { + // 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); this.cachedRooms[rmTag] = algorithm.orderedRooms; } 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}`); + if (!window.mx_QuietRoomListLogging) { + // 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); @@ -724,13 +727,17 @@ export class Algorithm extends EventEmitter { // 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`); + if (!window.mx_QuietRoomListLogging) { + // 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`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.log(`Received no-op update for ${room.roomId} - changing to Timeline update`); + } cause = RoomUpdateCause.Timeline; } @@ -756,20 +763,27 @@ export class Algorithm extends EventEmitter { // 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 (!window.mx_QuietRoomListLogging) { + // 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 (CAUSES_REQUIRING_ROOM.includes(cause)) { - console.warn(`Skipping tag update for ${room.roomId} because we don't know about the room`); + if (!window.mx_QuietRoomListLogging) { + // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 + console.warn(`Skipping tag update for ${room.roomId} because we don't know about the room`); + } return false; } - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Updating tags for room ${room.roomId} (${room.name})`); + if (!window.mx_QuietRoomListLogging) { + // 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])); @@ -780,12 +794,16 @@ export class Algorithm extends EventEmitter { 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); + if (!window.mx_QuietRoomListLogging) { + // 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}`); + if (!window.mx_QuietRoomListLogging) { + // 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) { @@ -807,8 +825,10 @@ export class Algorithm extends EventEmitter { 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})`); + if (!window.mx_QuietRoomListLogging) { + // 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; } } diff --git a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts index 3acd9f924e..88789d3a50 100644 --- a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts @@ -87,9 +87,6 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { super(tagId, initialSortingAlgorithm); - - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Constructed an ImportanceAlgorithm for ${tagId}`); } // noinspection JSMethodCanBeStatic diff --git a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts index 849c8a2877..ae1a2c98f6 100644 --- a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts @@ -28,9 +28,6 @@ export class NaturalAlgorithm extends OrderingAlgorithm { public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { super(tagId, initialSortingAlgorithm); - - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log(`[RoomListDebug] Constructed a NaturalAlgorithm for ${tagId}`); } public async setRooms(rooms: Room[]): Promise { diff --git a/src/stores/room-list/filters/CommunityFilterCondition.ts b/src/stores/room-list/filters/CommunityFilterCondition.ts index 9f7d8daaa3..45e65fb4f4 100644 --- a/src/stores/room-list/filters/CommunityFilterCondition.ts +++ b/src/stores/room-list/filters/CommunityFilterCondition.ts @@ -52,8 +52,6 @@ export class CommunityFilterCondition extends EventEmitter implements IFilterCon const beforeRoomIds = this.roomIds; this.roomIds = (await GroupStore.getGroupRooms(this.community.groupId)).map(r => r.roomId); if (arrayHasDiff(beforeRoomIds, this.roomIds)) { - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log("Updating filter for group: ", this.community.groupId); this.emit(FILTER_CHANGED); } }; diff --git a/src/stores/room-list/filters/NameFilterCondition.ts b/src/stores/room-list/filters/NameFilterCondition.ts index 12f147990d..6014a122f8 100644 --- a/src/stores/room-list/filters/NameFilterCondition.ts +++ b/src/stores/room-list/filters/NameFilterCondition.ts @@ -41,8 +41,6 @@ export class NameFilterCondition extends EventEmitter implements IFilterConditio public set search(val: string) { this._search = val; - // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 - console.log("Updating filter for room name search:", this._search); this.emit(FILTER_CHANGED); } From edb556f22e792b268d50fc57432d54604bb1850c Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 10 Jul 2020 02:15:46 +0100 Subject: [PATCH 07/23] Stop safari from agressivly shrinking --- res/css/structures/_LeftPanel2.scss | 3 +++ res/css/views/rooms/_RoomSublist2.scss | 2 ++ 2 files changed, 5 insertions(+) diff --git a/res/css/structures/_LeftPanel2.scss b/res/css/structures/_LeftPanel2.scss index b3f7fcc8ee..2062acf6b6 100644 --- a/res/css/structures/_LeftPanel2.scss +++ b/res/css/structures/_LeftPanel2.scss @@ -55,6 +55,7 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations .mx_LeftPanel2_userHeader { padding: 12px 12px 20px; // 12px top, 12px sides, 20px bottom + flex-shrink: 0; // Create another flexbox column for the rows to stack within display: flex; @@ -80,6 +81,8 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations margin-left: 12px; margin-right: 12px; + flex-shrink: 0; + // Create a flexbox to organize the inputs display: flex; align-items: center; diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 83e7e68563..eca738c46a 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -24,6 +24,8 @@ limitations under the License. margin-left: 8px; width: 100%; + flex-shrink: 0; + .mx_RoomSublist2_headerContainer { // Create a flexbox to make alignment easy display: flex; From e4366632cf464e82e9c43816c1caca19189d6e5c Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 10 Jul 2020 02:54:11 +0100 Subject: [PATCH 08/23] Fix search padding --- res/css/structures/_LeftPanel2.scss | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/res/css/structures/_LeftPanel2.scss b/res/css/structures/_LeftPanel2.scss index b3f7fcc8ee..7729eacc73 100644 --- a/res/css/structures/_LeftPanel2.scss +++ b/res/css/structures/_LeftPanel2.scss @@ -54,7 +54,10 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations flex-direction: column; .mx_LeftPanel2_userHeader { - padding: 12px 12px 20px; // 12px top, 12px sides, 20px bottom + /* 12px top, 12px sides, 20px bottom (using 13px bottom to account + * for internal whitespace in the breadcrumbs) + */ + padding: 12px 12px 13px; // Create another flexbox column for the rows to stack within display: flex; @@ -72,7 +75,8 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations width: 100%; overflow-y: hidden; overflow-x: scroll; - margin-top: 8px; + margin-top: 20px; + padding-bottom: 2px; } } From a5ba0cad1fa7f2814c3c9127014ac3706d6e3fcf Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 10 Jul 2020 08:13:23 -0600 Subject: [PATCH 09/23] Rename to trigger and add docs --- src/stores/room-list/RoomListStore2.ts | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 5591de67b1..f1b26f26ce 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -92,7 +92,12 @@ export class RoomListStore2 extends AsyncStore { await this.updateAlgorithmInstances(); } - private onRVSUpdate = async (quiet = false) => { + /** + * Handles suspected RoomViewStore changes. + * @param trigger Set to false to prevent a list update from being sent. Should only + * be used if the calling code will manually trigger the update. + */ + private onRVSUpdate = async ({trigger = true}) => { if (!this.enabled) return; // TODO: Remove with https://github.com/vector-im/riot-web/issues/14231 if (!this.matrixClient) return; // We assume there won't be RVS updates without a client @@ -113,7 +118,7 @@ export class RoomListStore2 extends AsyncStore { } } - if (!quiet) this.updateFn.trigger(); + if (trigger) this.updateFn.trigger(); }; protected onDispatch(payload: ActionPayload) { @@ -138,8 +143,8 @@ export class RoomListStore2 extends AsyncStore { // Update any settings here, as some may have happened before we were logically ready. console.log("Regenerating room lists: Startup"); await this.readAndCacheSettingsFromStore(); - await this.regenerateAllLists(true); - await this.onRVSUpdate(true); // fake an RVS update to adjust sticky room, if needed + await this.regenerateAllLists({trigger: false}); + await this.onRVSUpdate({trigger: false}); // fake an RVS update to adjust sticky room, if needed this.updateFn.trigger(); @@ -166,7 +171,7 @@ export class RoomListStore2 extends AsyncStore { console.log("Regenerating room lists: Settings changed"); await this.readAndCacheSettingsFromStore(); - await this.regenerateAllLists(true); // regenerate the lists now + await this.regenerateAllLists({trigger: false}); // regenerate the lists now this.updateFn.trigger(); } } @@ -474,7 +479,12 @@ export class RoomListStore2 extends AsyncStore { this.updateFn.mark(); }; - private async regenerateAllLists(quiet = false) { + /** + * Regenerates the room whole room list, discarding any previous results. + * @param trigger Set to false to prevent a list update from being sent. Should only + * be used if the calling code will manually trigger the update. + */ + private async regenerateAllLists({trigger = true}) { console.warn("Regenerating all room lists"); const sorts: ITagSortingMap = {}; @@ -499,7 +509,7 @@ export class RoomListStore2 extends AsyncStore { this.initialListsGenerated = true; - if (!quiet) this.updateFn.trigger(); + if (trigger) this.updateFn.trigger(); } public addFilter(filter: IFilterCondition): void { From 46d53e5c8c55cb82a768eb438ce4edb9fcdc321d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 10 Jul 2020 08:14:33 -0600 Subject: [PATCH 10/23] Reset before trigger just in case the function triggers too --- src/utils/MarkedExecution.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/MarkedExecution.ts b/src/utils/MarkedExecution.ts index d866720ecd..de6cf05953 100644 --- a/src/utils/MarkedExecution.ts +++ b/src/utils/MarkedExecution.ts @@ -50,8 +50,8 @@ export class MarkedExecution { */ public trigger() { if (!this.marked) return; + this.reset(); // reset first just in case the fn() causes a trigger() this.fn(); - this.reset(); } /** From 26427817f2704b674e5cea603786e3d968a53f27 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 10 Jul 2020 08:18:40 -0600 Subject: [PATCH 11/23] Fix potential listener conflict with RVS If the RVS ever emits something that contains `trigger: false`, we're pretty screwed, so avoid that. --- src/stores/room-list/RoomListStore2.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index f1b26f26ce..05f678160e 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -63,7 +63,7 @@ export class RoomListStore2 extends AsyncStore { this.checkEnabled(); for (const settingName of this.watchedSettings) SettingsStore.monitorSetting(settingName, null); - RoomViewStore.addListener(this.onRVSUpdate); + RoomViewStore.addListener(() => this.handleRVSUpdate({})); this.algorithm.on(LIST_UPDATED_EVENT, this.onAlgorithmListUpdated); } @@ -97,7 +97,7 @@ export class RoomListStore2 extends AsyncStore { * @param trigger Set to false to prevent a list update from being sent. Should only * be used if the calling code will manually trigger the update. */ - private onRVSUpdate = async ({trigger = true}) => { + private async handleRVSUpdate({trigger = true}) { if (!this.enabled) return; // TODO: Remove with https://github.com/vector-im/riot-web/issues/14231 if (!this.matrixClient) return; // We assume there won't be RVS updates without a client @@ -119,7 +119,7 @@ export class RoomListStore2 extends AsyncStore { } if (trigger) this.updateFn.trigger(); - }; + } protected onDispatch(payload: ActionPayload) { // We do this to intentionally break out of the current event loop task, allowing @@ -144,7 +144,7 @@ export class RoomListStore2 extends AsyncStore { console.log("Regenerating room lists: Startup"); await this.readAndCacheSettingsFromStore(); await this.regenerateAllLists({trigger: false}); - await this.onRVSUpdate({trigger: false}); // fake an RVS update to adjust sticky room, if needed + await this.handleRVSUpdate({trigger: false}); // fake an RVS update to adjust sticky room, if needed this.updateFn.trigger(); From 2bf2a08e7a8168e4459b7d65c9c58e3204d58349 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 10 Jul 2020 08:52:46 -0600 Subject: [PATCH 12/23] Mark safari hacks --- res/css/structures/_LeftPanel2.scss | 4 ++-- res/css/views/rooms/_RoomSublist2.scss | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/res/css/structures/_LeftPanel2.scss b/res/css/structures/_LeftPanel2.scss index 71d16aaa8f..6d9608f312 100644 --- a/res/css/structures/_LeftPanel2.scss +++ b/res/css/structures/_LeftPanel2.scss @@ -55,7 +55,7 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations .mx_LeftPanel2_userHeader { padding: 12px 12px 20px; // 12px top, 12px sides, 20px bottom - flex-shrink: 0; + flex-shrink: 0; // to convince safari's layout engine the flexbox is fine // Create another flexbox column for the rows to stack within display: flex; @@ -93,7 +93,7 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations margin-left: 12px; margin-right: 12px; - flex-shrink: 0; + flex-shrink: 0; // to convince safari's layout engine the flexbox is fine // Create a flexbox to organize the inputs display: flex; diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index eca738c46a..c656f2d15d 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -24,7 +24,7 @@ limitations under the License. margin-left: 8px; width: 100%; - flex-shrink: 0; + flex-shrink: 0; // to convince safari's layout engine the flexbox is fine .mx_RoomSublist2_headerContainer { // Create a flexbox to make alignment easy From d5a3071518a095f19b82d3fb784b6872372ae063 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 10 Jul 2020 18:29:39 +0200 Subject: [PATCH 13/23] put show more button inside resizer this way we have a flexbox layout in the resizer with: - the resize handle (fixed) - the show more/less button, if any (fixed) - the list of tiles (grabbing whatever is left) --- res/css/views/rooms/_RoomSublist2.scss | 20 +++++++++++++++----- src/components/views/rooms/RoomSublist2.tsx | 6 ++++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 6a77056917..bd00a7fc72 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -186,12 +186,22 @@ limitations under the License. flex-direction: column; overflow: hidden; + .mx_RoomSublist2_tiles { + flex: 1 0 0; + overflow: hidden; + // need this to be flex otherwise the overflow hidden from above + // sometimes vertically centers the clipped list ... no idea why it would do this + // as the box model should be top aligned. Happens in both FF and Chromium + display: flex; + flex-direction: column; + } + .mx_RoomSublist2_resizerHandles_showNButton { - position: absolute; - bottom: -32px; // height of the button - left: 0; - right: 0; - height: 4px; // height of the handle + flex: 0 0 32px; + } + + .mx_RoomSublist2_resizerHandles { + flex: 0 0 4px; } // Class name comes from the ResizableBox component diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index caa679f1d0..70d65f2437 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -666,9 +666,11 @@ export default class RoomSublist2 extends React.Component { className="mx_RoomSublist2_resizeBox" enable={handles} > - {visibleTiles} +
+ {visibleTiles} +
+ {showNButton} - {showNButton} ); } From 725b7f895015c5af42b6d4535f46e6928f9baca1 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 10 Jul 2020 18:30:52 +0200 Subject: [PATCH 14/23] make show more button a bit less tall --- res/css/views/rooms/_RoomSublist2.scss | 4 ---- src/components/views/rooms/RoomSublist2.tsx | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index bd00a7fc72..3744641390 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -241,11 +241,7 @@ limitations under the License. // Update the render() function for RoomSublist2 if these change // Update the ListLayout class for minVisibleTiles if these change. - // - // At 24px high, 8px padding on the top and 4px padding on the bottom this equates to 0.73 of - // a tile due to how the padding calculations work. height: 24px; - padding-top: 8px; padding-bottom: 4px; // We create a flexbox to cheat at alignment diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 70d65f2437..66b5a4aad1 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -59,7 +59,7 @@ import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; * warning disappears. * *******************************************************************/ -const SHOW_N_BUTTON_HEIGHT = 32; // As defined by CSS +const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS export const HEADER_HEIGHT = 32; // As defined by CSS From 49f7170d9591068bff30203bae82e8e52237bded Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 10 Jul 2020 18:31:53 +0200 Subject: [PATCH 15/23] extract type --- src/components/views/rooms/RoomSublist2.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 66b5a4aad1..c219b87c92 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -86,6 +86,12 @@ interface IProps { // TODO: Account for https://github.com/vector-im/riot-web/issues/14179 } +// TODO: Use re-resizer's NumberSize when it is exposed as the type +interface ResizeDelta { + width: number, + height: number, +} + type PartialDOMRect = Pick; interface IState { @@ -161,7 +167,7 @@ export default class RoomSublist2 extends React.Component { e: MouseEvent | TouchEvent, travelDirection: Direction, refToElement: HTMLDivElement, - delta: { width: number, height: number }, // TODO: Use re-resizer's NumberSize when it is exposed as the type + delta: ResizeDelta, ) => { // Do some sanity checks, but in reality we shouldn't need these. if (travelDirection !== "bottom") return; From 652fb9e6130323553c5fa5aa83646ed7bea589e5 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 10 Jul 2020 18:35:07 +0200 Subject: [PATCH 16/23] track height in pixels in component state --- src/components/views/rooms/RoomSublist2.tsx | 47 +++++++++++++-------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index c219b87c92..13690bd1ca 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -111,15 +111,36 @@ export default class RoomSublist2 extends React.Component { this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId); + const height = this.calculateInitialHeight(); this.state = { notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId), contextMenuPosition: null, isResizing: false, + height, }; this.state.notificationState.setRooms(this.props.rooms); this.dispatcherRef = defaultDispatcher.register(this.onAction); } + private calculateInitialHeight() { + const requestedVisibleTiles = Math.max(Math.floor(this.layout.visibleTiles), this.layout.minVisibleTiles); + const tileCount = Math.min(this.numTiles, requestedVisibleTiles); + const height = this.layout.tilesToPixelsWithPadding(tileCount, this.padding); + return height; + } + + private get padding() { + let padding = RESIZE_HANDLE_HEIGHT; + // this is used for calculating the max height of the whole container, + // and takes into account whether there should be room reserved for the show less button + // when fully expanded. Note that the show more button might still be shown when not fully expanded, + // but in this case it will take the space of a tile and we don't need to reserve space for it. + if (this.numTiles > this.layout.defaultVisibleTiles) { + padding += SHOW_N_BUTTON_HEIGHT; + } + return padding; + } + private get numTiles(): number { return (this.props.rooms || []).length + (this.props.extraBadTilesThatShouldntExist || []).length; } @@ -568,7 +589,11 @@ export default class RoomSublist2 extends React.Component { if (visibleTiles.length > 0) { const layout = this.layout; // to shorten calls - const maxTilesFactored = layout.tilesWithResizerBoxFactor(this.numTiles); + const minTiles = Math.min(layout.minVisibleTiles, this.numTiles); + const showMoreAtMinHeight = minTiles < this.numTiles; + const minHeightPadding = RESIZE_HANDLE_HEIGHT + (showMoreAtMinHeight ? SHOW_N_BUTTON_HEIGHT : 0); + const minTilesPx = layout.tilesToPixelsWithPadding(minTiles, minHeightPadding); + const maxTilesPx = layout.tilesToPixelsWithPadding(this.numTiles, this.padding); const showMoreBtnClasses = classNames({ 'mx_RoomSublist2_showNButton': true, 'mx_RoomSublist2_isCutting': this.state.isResizing && layout.visibleTiles < maxTilesFactored, @@ -578,9 +603,9 @@ export default class RoomSublist2 extends React.Component { // floats above the resize handle, if we have one present. If the user has all // tiles visible, it becomes 'show less'. let showNButton = null; - if (this.numTiles > visibleTiles.length) { // we have a cutoff condition - add the button to show all const numMissing = this.numTiles - visibleTiles.length; + if (maxTilesPx > this.state.height) { let showMoreText = ( {_t("Show %(count)s more", {count: numMissing})} @@ -595,7 +620,7 @@ export default class RoomSublist2 extends React.Component { {showMoreText} ); - } else if (this.numTiles <= visibleTiles.length && this.numTiles > this.layout.defaultVisibleTiles) { + } else if (this.numTiles > this.layout.defaultVisibleTiles) { // we have all tiles visible - add a button to show less let showLessText = ( @@ -639,29 +664,15 @@ export default class RoomSublist2 extends React.Component { // goes backwards and can become wildly incorrect (visibleTiles says 18 when there's // only mathematically 7 possible). - // The padding is variable though, so figure out what we need padding for. - let padding = 0; - //if (showNButton) padding += SHOW_N_BUTTON_HEIGHT; - //padding += RESIZE_HANDLE_HEIGHT; // always append the handle height - - const relativeTiles = layout.tilesWithPadding(this.numTiles, padding); - const minTilesPx = layout.calculateTilesToPixelsMin(relativeTiles, layout.minVisibleTiles, padding); - const maxTilesPx = layout.tilesToPixelsWithPadding(this.numTiles, padding); - const tilesWithoutPadding = Math.min(relativeTiles, layout.visibleTiles); - const tilesPx = layout.calculateTilesToPixelsMin(relativeTiles, tilesWithoutPadding, padding); - const handleWrapperClasses = classNames({ 'mx_RoomSublist2_resizerHandles': true, 'mx_RoomSublist2_resizerHandles_showNButton': !!showNButton, }); - const dimensions = { - height: tilesPx, - }; content = ( Date: Fri, 10 Jul 2020 18:36:33 +0200 Subject: [PATCH 17/23] make all height changes update component state also set visibleTiles as side-effect --- src/components/views/rooms/RoomSublist2.tsx | 46 ++++++++++----------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 13690bd1ca..713b70ce94 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -110,7 +110,7 @@ export default class RoomSublist2 extends React.Component { super(props); this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId); - + this.heightAtStart = 0; const height = this.calculateInitialHeight(); this.state = { notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId), @@ -184,47 +184,45 @@ export default class RoomSublist2 extends React.Component { if (this.props.onAddRoom) this.props.onAddRoom(); }; + private applyHeightChange(newHeight: number) { + const heightInTiles = Math.ceil(this.layout.pixelsToTiles(newHeight - this.padding)); + this.layout.visibleTiles = Math.min(this.numTiles, heightInTiles); + } + private onResize = ( e: MouseEvent | TouchEvent, travelDirection: Direction, refToElement: HTMLDivElement, delta: ResizeDelta, ) => { - // Do some sanity checks, but in reality we shouldn't need these. - if (travelDirection !== "bottom") return; - if (delta.height === 0) return; // something went wrong, so just ignore it. - - // NOTE: the movement in the MouseEvent (not present on a TouchEvent) is inaccurate - // for our purposes. The delta provided by the library is also a change *from when - // resizing started*, meaning it is fairly useless for us. This is why we just use - // the client height and run with it. - - const heightBefore = this.layout.visibleTiles; - const heightInTiles = this.layout.pixelsToTiles(refToElement.clientHeight); - this.layout.setVisibleTilesWithin(heightInTiles, this.numTiles); - if (heightBefore === this.layout.visibleTiles) return; // no-op - this.forceUpdate(); // because the layout doesn't trigger a re-render + const newHeight = this.heightAtStart + delta.height; + this.applyHeightChange(newHeight); + this.setState({height: newHeight}); }; private onResizeStart = () => { + this.heightAtStart = this.state.height; this.setState({isResizing: true}); }; - private onResizeStop = () => { - this.setState({isResizing: false}); + private onResizeStop = (e, direction, ref, d) => { + const newHeight = this.heightAtStart + d.height; + this.applyHeightChange(newHeight); + this.setState({isResizing: false, height: newHeight}); }; private onShowAllClick = () => { - const numVisibleTiles = this.numVisibleTiles; - this.layout.visibleTiles = this.layout.tilesWithPadding(this.numTiles, MAX_PADDING_HEIGHT); - this.forceUpdate(); // because the layout doesn't trigger a re-render - setImmediate(this.focusRoomTile, numVisibleTiles); // focus the tile after the current bottom one + const newHeight = this.layout.tilesToPixelsWithPadding(this.numTiles, this.padding); + this.applyHeightChange(newHeight); + this.setState({height: newHeight}, () => { + this.focusRoomTile(this.numTiles - 1); + }); }; private onShowLessClick = () => { - this.layout.visibleTiles = this.layout.defaultVisibleTiles; - this.forceUpdate(); // because the layout doesn't trigger a re-render - // focus will flow to the show more button here + const newHeight = this.layout.tilesToPixelsWithPadding(this.layout.defaultVisibleTiles, this.padding); + this.applyHeightChange(newHeight); + this.setState({height: newHeight}); }; private focusRoomTile = (index: number) => { From 86817430c51cbad1234c200c61ff8de56f8ab415 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 10 Jul 2020 18:37:58 +0200 Subject: [PATCH 18/23] update initially shown amount of tiles on component update as rooms aren't all available at ctor time --- src/components/views/rooms/RoomSublist2.tsx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 713b70ce94..6acb5f929c 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -142,7 +142,11 @@ export default class RoomSublist2 extends React.Component { } private get numTiles(): number { - return (this.props.rooms || []).length + (this.props.extraBadTilesThatShouldntExist || []).length; + return RoomSublist2.calcNumTiles(this.props); + } + + private static calcNumTiles(props) { + return (props.rooms || []).length + (props.extraBadTilesThatShouldntExist || []).length; } private get numVisibleTiles(): number { @@ -150,8 +154,13 @@ export default class RoomSublist2 extends React.Component { return Math.min(nVisible, this.numTiles); } - public componentDidUpdate() { + public componentDidUpdate(prevProps) { this.state.notificationState.setRooms(this.props.rooms); + // as the rooms can come in one by one we need to reevaluate + // the amount of available rooms to cap the amount of requested visible rooms by the layout + if (RoomSublist2.calcNumTiles(prevProps) !== this.numTiles) { + this.setState({height: this.calculateInitialHeight()}); + } } public componentWillUnmount() { From e2aa6ecf6b6dd9a9f0290fef092d1de8cc2afee7 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 10 Jul 2020 18:38:32 +0200 Subject: [PATCH 19/23] fix show X more counter --- src/components/views/rooms/RoomSublist2.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 6acb5f929c..1a1a45b7cf 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -610,9 +610,11 @@ export default class RoomSublist2 extends React.Component { // floats above the resize handle, if we have one present. If the user has all // tiles visible, it becomes 'show less'. let showNButton = null; - // we have a cutoff condition - add the button to show all - const numMissing = this.numTiles - visibleTiles.length; + if (maxTilesPx > this.state.height) { + const nonPaddedHeight = this.state.height - RESIZE_HANDLE_HEIGHT - SHOW_N_BUTTON_HEIGHT; + const amountFullyShown = Math.floor(nonPaddedHeight / this.layout.tileHeight); + const numMissing = this.numTiles - amountFullyShown; let showMoreText = ( {_t("Show %(count)s more", {count: numMissing})} From 85ac256231e75dcef0d3f82d463768e90c39992a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 10 Jul 2020 18:38:53 +0200 Subject: [PATCH 20/23] cleanup --- res/css/views/rooms/_RoomSublist2.scss | 10 ---------- src/components/views/rooms/RoomSublist2.tsx | 2 -- 2 files changed, 12 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 3744641390..73dc7d58b8 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -267,16 +267,6 @@ limitations under the License. .mx_RoomSublist2_showLessButtonChevron { mask-image: url('$(res)/img/feather-customised/chevron-up.svg'); } - - &.mx_RoomSublist2_isCutting::before { - content: ''; - position: absolute; - top: 0; - left: 0; - right: 0; - height: 4px; - box-shadow: 0px -2px 3px rgba(46, 47, 50, 0.08); - } } &.mx_RoomSublist2_hasMenuOpen, diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 1a1a45b7cf..5a7bfa3990 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -585,7 +585,6 @@ export default class RoomSublist2 extends React.Component { // TODO: Error boundary: https://github.com/vector-im/riot-web/issues/14185 const visibleTiles = this.renderVisibleTiles(); - const classes = classNames({ 'mx_RoomSublist2': true, 'mx_RoomSublist2_hasMenuOpen': !!this.state.contextMenuPosition, @@ -603,7 +602,6 @@ export default class RoomSublist2 extends React.Component { const maxTilesPx = layout.tilesToPixelsWithPadding(this.numTiles, this.padding); const showMoreBtnClasses = classNames({ 'mx_RoomSublist2_showNButton': true, - 'mx_RoomSublist2_isCutting': this.state.isResizing && layout.visibleTiles < maxTilesFactored, }); // If we're hiding rooms, show a 'show more' button to the user. This button From 15ea3a528796d6e50271095d8754b7f6fcb25bc2 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 10 Jul 2020 18:42:51 +0200 Subject: [PATCH 21/23] add types --- src/components/views/rooms/RoomSublist2.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 5a7bfa3990..a059d146e8 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -214,8 +214,13 @@ export default class RoomSublist2 extends React.Component { this.setState({isResizing: true}); }; - private onResizeStop = (e, direction, ref, d) => { - const newHeight = this.heightAtStart + d.height; + private onResizeStop = ( + e: MouseEvent | TouchEvent, + travelDirection: Direction, + refToElement: HTMLDivElement, + delta: ResizeDelta, + ) => { + const newHeight = this.heightAtStart + delta.height; this.applyHeightChange(newHeight); this.setState({isResizing: false, height: newHeight}); }; From ae8d6f5523a3e65548175e6d10410ddce0b67006 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 10 Jul 2020 18:48:54 +0200 Subject: [PATCH 22/23] make tsc happy --- src/components/views/rooms/RoomSublist2.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index a059d146e8..73d53ccae7 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -88,8 +88,8 @@ interface IProps { // TODO: Use re-resizer's NumberSize when it is exposed as the type interface ResizeDelta { - width: number, - height: number, + width: number; + height: number; } type PartialDOMRect = Pick; @@ -98,6 +98,7 @@ interface IState { notificationState: ListNotificationState; contextMenuPosition: PartialDOMRect; isResizing: boolean; + height: number; } export default class RoomSublist2 extends React.Component { @@ -105,6 +106,7 @@ export default class RoomSublist2 extends React.Component { private sublistRef = createRef(); private dispatcherRef: string; private layout: ListLayout; + private heightAtStart: number; constructor(props: IProps) { super(props); @@ -684,7 +686,7 @@ export default class RoomSublist2 extends React.Component { content = ( Date: Fri, 10 Jul 2020 11:01:11 -0600 Subject: [PATCH 23/23] Fix bad merge --- src/components/views/rooms/RoomSublist2.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index b8023cc532..bfbdd3a161 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -120,7 +120,7 @@ export default class RoomSublist2 extends React.Component { notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId), contextMenuPosition: null, isResizing: false, - isExpanded: this.props.isFiltered ? this.props.isFiltered : !this.layout.isCollapsed + isExpanded: this.props.isFiltered ? this.props.isFiltered : !this.layout.isCollapsed, height, }; this.state.notificationState.setRooms(this.props.rooms);