From 87120c6c26ae5afe8e48dc93085532c164ae01ae Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 10 Jul 2020 15:58:39 -0600 Subject: [PATCH 1/3] Ensure triggered updates get fired for filters in the new room list Fixes https://github.com/vector-im/riot-web/issues/14404 --- src/stores/room-list/RoomListStore2.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 4741e1a30f..38a9509e0c 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -481,8 +481,9 @@ export class RoomListStore2 extends AsyncStore { }; private onAlgorithmFilterUpdated = () => { - // The filter can happen off-cycle, so trigger an update if we need to. - this.updateFn.triggerIfWillMark(); + // The filter can happen off-cycle, so trigger an update. The filter will have + // already caused a mark. + this.updateFn.trigger(); }; /** From cfc39dc4a93a62638d0c4f28f13bbebb58f5ff43 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 10 Jul 2020 16:10:05 -0600 Subject: [PATCH 2/3] Remove now-dead code from sublist resizing --- src/components/views/rooms/RoomSublist2.tsx | 9 +++--- src/stores/room-list/ListLayout.ts | 33 ++------------------- 2 files changed, 6 insertions(+), 36 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index bfbdd3a161..c351384be4 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -20,7 +20,7 @@ import * as React from "react"; import { createRef } from "react"; import { Room } from "matrix-js-sdk/src/models/room"; import classNames from 'classnames'; -import {RovingAccessibleButton, RovingTabIndexWrapper} from "../../../accessibility/RovingTabIndex"; +import { RovingAccessibleButton, RovingTabIndexWrapper } from "../../../accessibility/RovingTabIndex"; import { _t } from "../../../languageHandler"; import AccessibleButton from "../../views/elements/AccessibleButton"; import RoomTile2 from "./RoomTile2"; @@ -36,12 +36,12 @@ import RoomListStore from "../../../stores/room-list/RoomListStore2"; import { ListAlgorithm, SortAlgorithm } from "../../../stores/room-list/algorithms/models"; import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import dis from "../../../dispatcher/dispatcher"; +import defaultDispatcher from "../../../dispatcher/dispatcher"; import NotificationBadge from "./NotificationBadge"; import { ListNotificationState } from "../../../stores/notifications/ListNotificationState"; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; import { Key } from "../../../Keyboard"; -import defaultDispatcher from "../../../dispatcher/dispatcher"; -import {ActionPayload} from "../../../dispatcher/payloads"; +import { ActionPayload } from "../../../dispatcher/payloads"; import { Enable, Resizable } from "re-resizable"; import { Direction } from "re-resizable/lib/resizer"; import { polyfillTouchEvent } from "../../../@types/polyfill"; @@ -130,8 +130,7 @@ export default class RoomSublist2 extends React.Component { 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; + return this.layout.tilesToPixelsWithPadding(tileCount, this.padding); } private get padding() { diff --git a/src/stores/room-list/ListLayout.ts b/src/stores/room-list/ListLayout.ts index f1900487bc..caf2e92bd1 100644 --- a/src/stores/room-list/ListLayout.ts +++ b/src/stores/room-list/ListLayout.ts @@ -18,11 +18,6 @@ import { TagID } from "./models"; 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; - interface ISerializedListLayout { numTiles: number; showPreviews: boolean; @@ -82,36 +77,12 @@ export class ListLayout { } public get minVisibleTiles(): number { - return 1 + RESIZER_BOX_FACTOR; + return 1; } public get defaultVisibleTiles(): number { // This number is what "feels right", and mostly subject to design's opinion. - return 5 + RESIZER_BOX_FACTOR; - } - - public setVisibleTilesWithin(newVal: number, maxPossible: number) { - maxPossible = maxPossible + RESIZER_BOX_FACTOR; - if (newVal > maxPossible) { - this.visibleTiles = maxPossible; - } else { - this.visibleTiles = newVal; - } - } - - public calculateTilesToPixelsMin(maxTiles: number, n: number, possiblePadding: number): number { - // Only apply the padding if we're about to use maxTiles as we need to - // plan for the padding. If we're using n, the padding is already accounted - // for by the resizing stuff. - let padding = 0; - if (maxTiles < n) { - padding = possiblePadding; - } - return this.tilesToPixels(Math.min(maxTiles, n)) + padding; - } - - public tilesWithResizerBoxFactor(n: number): number { - return n + RESIZER_BOX_FACTOR; + return 5; } public tilesWithPadding(n: number, paddingPx: number): number { From 0e49c4343c07875fa9220dce47e464db34789b44 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 10 Jul 2020 21:59:12 -0600 Subject: [PATCH 3/3] Internalize algorithm updates in the new room list store Fixes https://github.com/vector-im/riot-web/issues/14411 The act of setting/changing the algorithm was causing the update function to be marked, meaning we wouldn't trigger an update until something else happened later. To get around this, and still support internal functions spamming calls without multiple updates, we simply move the guts to an internalized function and make the public interface do a trigger. --- src/stores/room-list/RoomListStore2.ts | 16 ++++++++++++---- src/utils/MarkedExecution.ts | 11 ----------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 38a9509e0c..5ba448b93c 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -368,10 +368,14 @@ export class RoomListStore2 extends AsyncStore { } public async setTagSorting(tagId: TagID, sort: SortAlgorithm) { + await this.setAndPersistTagSorting(tagId, sort); + this.updateFn.trigger(); + } + + private async setAndPersistTagSorting(tagId: TagID, sort: SortAlgorithm) { 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 { @@ -407,10 +411,14 @@ export class RoomListStore2 extends AsyncStore { } public async setListOrder(tagId: TagID, order: ListAlgorithm) { + await this.setAndPersistListOrder(tagId, order); + this.updateFn.trigger(); + } + + private async setAndPersistListOrder(tagId: TagID, order: ListAlgorithm) { 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 { @@ -458,10 +466,10 @@ export class RoomListStore2 extends AsyncStore { const listOrder = this.calculateListOrder(tag); if (tagSort !== definedSort) { - await this.setTagSorting(tag, tagSort); + await this.setAndPersistTagSorting(tag, tagSort); } if (listOrder !== definedOrder) { - await this.setListOrder(tag, listOrder); + await this.setAndPersistListOrder(tag, listOrder); } } } diff --git a/src/utils/MarkedExecution.ts b/src/utils/MarkedExecution.ts index de6cf05953..b0b8fdf63d 100644 --- a/src/utils/MarkedExecution.ts +++ b/src/utils/MarkedExecution.ts @@ -53,15 +53,4 @@ export class MarkedExecution { this.reset(); // reset first just in case the fn() causes a trigger() this.fn(); } - - /** - * 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(); - } - } }