From f935303eeb65f96d0822a7ac9ed3d316c6ee16f6 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 30 Jun 2020 18:51:59 -0600 Subject: [PATCH 1/5] Change default number of rooms visible to 10 Fixes https://github.com/vector-im/riot-web/issues/14266 --- src/stores/room-list/ListLayout.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/stores/room-list/ListLayout.ts b/src/stores/room-list/ListLayout.ts index 56f94ccd9a7..2cc8eda510d 100644 --- a/src/stores/room-list/ListLayout.ts +++ b/src/stores/room-list/ListLayout.ts @@ -85,10 +85,8 @@ export class ListLayout { } public get defaultVisibleTiles(): number { - // TODO: Remove dogfood flag: https://github.com/vector-im/riot-web/issues/14231 - // TODO: Resolve dogfooding: https://github.com/vector-im/riot-web/issues/14137 - const val = Number(localStorage.getItem("mx_dogfood_rl_defTiles") || 4); - return val + RESIZER_BOX_FACTOR; + // 10 is what "feels right", and mostly subject to design's opinion. + return 10 + RESIZER_BOX_FACTOR; } public calculateTilesToPixelsMin(maxTiles: number, n: number, possiblePadding: number): number { From 8cfe12b817995be90ad68083a49a5263fc6fe0fd Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 30 Jun 2020 18:52:13 -0600 Subject: [PATCH 2/5] Add a layout reset function For https://github.com/vector-im/riot-web/issues/14265 Intended to be accessed via `mx_RoomListStore2.resetLayout()` --- src/stores/room-list/ListLayout.ts | 4 ++++ src/stores/room-list/RoomListStore2.ts | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/stores/room-list/ListLayout.ts b/src/stores/room-list/ListLayout.ts index 2cc8eda510d..d8564bf947b 100644 --- a/src/stores/room-list/ListLayout.ts +++ b/src/stores/room-list/ListLayout.ts @@ -120,6 +120,10 @@ export class ListLayout { return px / this.tileHeight; } + public reset() { + localStorage.removeItem(this.key); + } + private save() { localStorage.setItem(this.key, JSON.stringify(this.serialize())); } diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 497b8e55306..ac2324295ef 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -30,6 +30,7 @@ import { TagWatcher } from "./TagWatcher"; import RoomViewStore from "../RoomViewStore"; import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/Algorithm"; import { EffectiveMembership, getEffectiveMembership } from "./membership"; +import { ListLayout } from "./ListLayout"; interface IState { tagsEnabled?: boolean; @@ -401,6 +402,15 @@ export class RoomListStore2 extends AsyncStore { this.emit(LISTS_UPDATE_EVENT, this); } + // Note: this primarily exists for debugging, and isn't really intended to be used by anything. + public async resetLayouts() { + console.warn("Resetting layouts for room list"); + for (const tagId of Object.keys(this.orderedLists)) { + new ListLayout(tagId).reset(); + } + await this.regenerateAllLists(); + } + public addFilter(filter: IFilterCondition): void { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log("Adding filter condition:", filter); From 7674030c6eb32d7d9ede6f92e64be697f4b37bb1 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 30 Jun 2020 19:14:36 -0600 Subject: [PATCH 3/5] Show 'show more' when there are less tiles than the default For example, if you only have 3/10 rooms required for the default then resize smaller, we should have a 'show more' button. This works by changing the rendering to be slightly more efficient and only looping over what is seen (renderVisibleTiles(), using this.numTiles in place of tiles.length) and using a new setVisibleTilesWithin() function on the layout. Previously resizing the 3/10 case would be setting visibleTiles to ~8 instead of ~1 like it should (because the getter returns a default). --- src/components/views/rooms/RoomSublist2.tsx | 39 ++++++++++++--------- src/stores/room-list/ListLayout.ts | 8 +++++ 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 58ebf54bf79..66825272540 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -91,6 +91,12 @@ export default class RoomSublist2 extends React.Component { return (this.props.rooms || []).length; } + private get numVisibleTiles(): number { + if (!this.props.layout) return 0; + const nVisible = Math.floor(this.props.layout.visibleTiles); + return Math.min(nVisible, this.numTiles); + } + public componentDidUpdate() { this.state.notificationState.setRooms(this.props.rooms); } @@ -107,7 +113,7 @@ export default class RoomSublist2 extends React.Component { private onResize = (e: React.MouseEvent, data: ResizeCallbackData) => { const direction = e.movementY < 0 ? -1 : +1; const tileDiff = this.props.layout.pixelsToTiles(Math.abs(e.movementY)) * direction; - this.props.layout.visibleTiles += tileDiff; + this.props.layout.setVisibleTilesWithin(tileDiff, this.numTiles); this.forceUpdate(); // because the layout doesn't trigger a re-render }; @@ -173,13 +179,17 @@ export default class RoomSublist2 extends React.Component { } }; - private renderTiles(): React.ReactElement[] { - if (this.props.layout && this.props.layout.isCollapsed) return []; // don't waste time on rendering + private renderVisibleTiles(): React.ReactElement[] { + if (this.props.layout && this.props.layout.isCollapsed) { + // don't waste time on rendering + return []; + } const tiles: React.ReactElement[] = []; if (this.props.rooms) { - for (const room of this.props.rooms) { + const visibleRooms = this.props.rooms.slice(0, this.numVisibleTiles); + for (const room of visibleRooms) { tiles.push( { public render(): React.ReactElement { // TODO: Error boundary: https://github.com/vector-im/riot-web/issues/14185 - const tiles = this.renderTiles(); + const visibleTiles = this.renderVisibleTiles(); const classes = classNames({ 'mx_RoomSublist2': true, @@ -347,13 +357,10 @@ export default class RoomSublist2 extends React.Component { }); let content = null; - if (tiles.length > 0) { + if (visibleTiles.length > 0) { const layout = this.props.layout; // to shorten calls - const nVisible = Math.floor(layout.visibleTiles); - const visibleTiles = tiles.slice(0, nVisible); - - const maxTilesFactored = layout.tilesWithResizerBoxFactor(tiles.length); + const maxTilesFactored = layout.tilesWithResizerBoxFactor(this.numTiles); const showMoreBtnClasses = classNames({ 'mx_RoomSublist2_showNButton': true, 'mx_RoomSublist2_isCutting': this.state.isResizing && layout.visibleTiles < maxTilesFactored, @@ -363,9 +370,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 (tiles.length > nVisible) { + if (this.numTiles > this.numVisibleTiles) { // we have a cutoff condition - add the button to show all - const numMissing = tiles.length - visibleTiles.length; + const numMissing = this.numTiles - visibleTiles.length; let showMoreText = ( {_t("Show %(count)s more", {count: numMissing})} @@ -380,7 +387,7 @@ export default class RoomSublist2 extends React.Component { {showMoreText} ); - } else if (tiles.length <= nVisible && tiles.length > this.props.layout.defaultVisibleTiles) { + } else if (this.numTiles <= this.numVisibleTiles && this.numTiles > this.props.layout.defaultVisibleTiles) { // we have all tiles visible - add a button to show less let showLessText = ( @@ -400,7 +407,7 @@ export default class RoomSublist2 extends React.Component { // Figure out if we need a handle let handles = ['s']; - if (layout.visibleTiles >= tiles.length && tiles.length <= layout.minVisibleTiles) { + if (layout.visibleTiles >= this.numTiles && this.numTiles <= layout.minVisibleTiles) { handles = []; // no handles, we're at a minimum } @@ -419,9 +426,9 @@ export default class RoomSublist2 extends React.Component { if (showNButton) padding += SHOW_N_BUTTON_HEIGHT; padding += RESIZE_HANDLE_HEIGHT; // always append the handle height - const relativeTiles = layout.tilesWithPadding(tiles.length, padding); + const relativeTiles = layout.tilesWithPadding(this.numTiles, padding); const minTilesPx = layout.calculateTilesToPixelsMin(relativeTiles, layout.minVisibleTiles, padding); - const maxTilesPx = layout.tilesToPixelsWithPadding(tiles.length, padding); + const maxTilesPx = layout.tilesToPixelsWithPadding(this.numTiles, padding); const tilesWithoutPadding = Math.min(relativeTiles, layout.visibleTiles); const tilesPx = layout.calculateTilesToPixelsMin(relativeTiles, tilesWithoutPadding, padding); diff --git a/src/stores/room-list/ListLayout.ts b/src/stores/room-list/ListLayout.ts index d8564bf947b..528276e801e 100644 --- a/src/stores/room-list/ListLayout.ts +++ b/src/stores/room-list/ListLayout.ts @@ -89,6 +89,14 @@ export class ListLayout { return 10 + RESIZER_BOX_FACTOR; } + public setVisibleTilesWithin(diff: number, maxPossible: number) { + if (this.visibleTiles > maxPossible) { + this.visibleTiles = maxPossible + diff; + } else { + this.visibleTiles += diff; + } + } + 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 From 8cfbfd4221f962f6ae36a154b9a5210b43c2e31e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 30 Jun 2020 19:20:11 -0600 Subject: [PATCH 4/5] Increase RESIZER_BOX_FACTOR to account for overlap from handle Fixes https://github.com/vector-im/riot-web/issues/14136 The resizer handle wasn't being considered in this. 78% is both verified through mathematics and playing with it manually. --- src/stores/room-list/ListLayout.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/stores/room-list/ListLayout.ts b/src/stores/room-list/ListLayout.ts index 528276e801e..efb0c4bdfba 100644 --- a/src/stores/room-list/ListLayout.ts +++ b/src/stores/room-list/ListLayout.ts @@ -18,9 +18,9 @@ import { TagID } from "./models"; const TILE_HEIGHT_PX = 44; -// the .65 comes from the CSS where the show more button is -// mathematically 65% of a tile when floating. -const RESIZER_BOX_FACTOR = 0.65; +// 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; interface ISerializedListLayout { numTiles: number; From 946fde5cc5fd24daa1ea7b66385112b17fad36a5 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 1 Jul 2020 11:59:32 -0600 Subject: [PATCH 5/5] Be consistent in visible tiles usage --- src/components/views/rooms/RoomSublist2.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 66825272540..fb955fcd574 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -370,7 +370,7 @@ 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 > this.numVisibleTiles) { + if (this.numTiles > visibleTiles.length) { // we have a cutoff condition - add the button to show all const numMissing = this.numTiles - visibleTiles.length; let showMoreText = ( @@ -387,7 +387,7 @@ export default class RoomSublist2 extends React.Component { {showMoreText} ); - } else if (this.numTiles <= this.numVisibleTiles && this.numTiles > this.props.layout.defaultVisibleTiles) { + } else if (this.numTiles <= visibleTiles.length && this.numTiles > this.props.layout.defaultVisibleTiles) { // we have all tiles visible - add a button to show less let showLessText = (