From 7b97c3032b8c25874c8bbe9d71e64a379b20a46e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 23 Jul 2020 21:23:45 -0600 Subject: [PATCH 1/9] Make the sublists aware of their own list changes This cuts the render time in half (from ~448ms to ~200ms on my account) per received event, as we're no longer re-mounting the entire room list and instead just the section(s) we care about. --- src/components/views/rooms/RoomList.tsx | 13 ++++-- src/components/views/rooms/RoomSublist.tsx | 47 ++++++++++++++-------- src/utils/arrays.ts | 20 ++++++++- 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index aff6099ab0..057b327aed 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -42,6 +42,7 @@ import { ViewRoomDeltaPayload } from "../../../dispatcher/payloads/ViewRoomDelta import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import SettingsStore from "../../../settings/SettingsStore"; import CustomRoomTagStore from "../../../stores/CustomRoomTagStore"; +import { arrayHasDiff } from "../../../utils/arrays"; interface IProps { onKeyDown: (ev: React.KeyboardEvent) => void; @@ -231,9 +232,14 @@ export default class RoomList extends React.Component { console.log("new lists", newLists); } - this.setState({sublists: newLists}, () => { - this.props.onResize(); - }); + const previousListIds = Object.keys(this.state.sublists); + const newListIds = Object.keys(newLists); + + if (arrayHasDiff(previousListIds, newListIds)) { + this.setState({sublists: newLists}, () => { + this.props.onResize(); + }); + } }; private renderCommunityInvites(): React.ReactElement[] { @@ -304,7 +310,6 @@ export default class RoomList extends React.Component { key={`sublist-${orderedTagId}`} tagId={orderedTagId} forRooms={true} - rooms={orderedRooms} startAsHidden={aesthetics.defaultHidden} label={aesthetics.sectionLabelRaw ? aesthetics.sectionLabelRaw : _t(aesthetics.sectionLabel)} onAddRoom={onAddRoomFn} diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 18bf56e9ba..1ca3a0006f 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -32,7 +32,7 @@ import { StyledMenuItemCheckbox, StyledMenuItemRadio, } from "../../structures/ContextMenu"; -import RoomListStore from "../../../stores/room-list/RoomListStore"; +import RoomListStore, { LISTS_UPDATE_EVENT } from "../../../stores/room-list/RoomListStore"; import { ListAlgorithm, SortAlgorithm } from "../../../stores/room-list/algorithms/models"; import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import dis from "../../../dispatcher/dispatcher"; @@ -47,6 +47,7 @@ import { Direction } from "re-resizable/lib/resizer"; import { polyfillTouchEvent } from "../../../@types/polyfill"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; +import { arrayHasDiff, arrayHasOrderChange } from "../../../utils/arrays"; const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS @@ -59,7 +60,6 @@ polyfillTouchEvent(); interface IProps { forRooms: boolean; - rooms?: Room[]; startAsHidden: boolean; label: string; onAddRoom?: () => void; @@ -90,6 +90,7 @@ interface IState { isResizing: boolean; isExpanded: boolean; // used for the for expand of the sublist when the room list is being filtered height: number; + rooms: Room[]; } export default class RoomSublist extends React.Component { @@ -104,16 +105,19 @@ export default class RoomSublist extends React.Component { this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId); this.heightAtStart = 0; - const height = this.calculateInitialHeight(); this.state = { notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId), contextMenuPosition: null, isResizing: false, isExpanded: this.props.isFiltered ? this.props.isFiltered : !this.layout.isCollapsed, - height, + height: 0, // to be fixed in a moment, we need `rooms` to calculate this. + rooms: RoomListStore.instance.orderedLists[this.props.tagId] || [], }; - this.state.notificationState.setRooms(this.props.rooms); + // Why Object.assign() and not this.state.height? Because TypeScript says no. + this.state = Object.assign(this.state, {height: this.calculateInitialHeight()}); + this.state.notificationState.setRooms(this.state.rooms); this.dispatcherRef = defaultDispatcher.register(this.onAction); + RoomListStore.instance.on(LISTS_UPDATE_EVENT, this.onListsUpdated); } private calculateInitialHeight() { @@ -142,11 +146,11 @@ export default class RoomSublist extends React.Component { } private get numTiles(): number { - return RoomSublist.calcNumTiles(this.props); + return RoomSublist.calcNumTiles(this.state.rooms, this.props.extraBadTilesThatShouldntExist); } - private static calcNumTiles(props) { - return (props.rooms || []).length + (props.extraBadTilesThatShouldntExist || []).length; + private static calcNumTiles(rooms: Room[], extraTiles: any[]) { + return (rooms || []).length + (extraTiles || []).length; } private get numVisibleTiles(): number { @@ -154,8 +158,8 @@ export default class RoomSublist extends React.Component { return Math.min(nVisible, this.numTiles); } - public componentDidUpdate(prevProps: Readonly) { - this.state.notificationState.setRooms(this.props.rooms); + public componentDidUpdate(prevProps: Readonly, prevState: Readonly) { + this.state.notificationState.setRooms(this.state.rooms); if (prevProps.isFiltered !== this.props.isFiltered) { if (this.props.isFiltered) { this.setState({isExpanded: true}); @@ -165,7 +169,7 @@ export default class RoomSublist extends React.Component { } // 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 (RoomSublist.calcNumTiles(prevProps) !== this.numTiles) { + if (RoomSublist.calcNumTiles(prevState.rooms, prevProps.extraBadTilesThatShouldntExist) !== this.numTiles) { this.setState({height: this.calculateInitialHeight()}); } } @@ -173,14 +177,23 @@ export default class RoomSublist extends React.Component { public componentWillUnmount() { this.state.notificationState.destroy(); defaultDispatcher.unregister(this.dispatcherRef); + RoomListStore.instance.off(LISTS_UPDATE_EVENT, this.onListsUpdated); } + private onListsUpdated = () => { + const currentRooms = this.state.rooms; + const newRooms = RoomListStore.instance.orderedLists[this.props.tagId] || []; + if (arrayHasOrderChange(currentRooms, newRooms)) { + this.setState({rooms: newRooms}); + } + }; + private onAction = (payload: ActionPayload) => { - if (payload.action === "view_room" && payload.show_room_tile && this.props.rooms) { + if (payload.action === "view_room" && payload.show_room_tile && this.state.rooms) { // XXX: we have to do this a tick later because we have incorrect intermediate props during a room change // where we lose the room we are changing from temporarily and then it comes back in an update right after. setImmediate(() => { - const roomIndex = this.props.rooms.findIndex((r) => r.roomId === payload.room_id); + const roomIndex = this.state.rooms.findIndex((r) => r.roomId === payload.room_id); if (!this.state.isExpanded && roomIndex > -1) { this.toggleCollapsed(); @@ -302,10 +315,10 @@ export default class RoomSublist extends React.Component { let room; if (this.props.tagId === DefaultTagID.Invite) { // switch to first room as that'll be the top of the list for the user - room = this.props.rooms && this.props.rooms[0]; + room = this.state.rooms && this.state.rooms[0]; } else { // find the first room with a count of the same colour as the badge count - room = this.props.rooms.find((r: Room) => { + room = this.state.rooms.find((r: Room) => { const notifState = this.state.notificationState.getForRoom(r); return notifState.count > 0 && notifState.color === this.state.notificationState.color; }); @@ -399,8 +412,8 @@ export default class RoomSublist extends React.Component { const tiles: React.ReactElement[] = []; - if (this.props.rooms) { - const visibleRooms = this.props.rooms.slice(0, this.numVisibleTiles); + if (this.state.rooms) { + const visibleRooms = this.state.rooms.slice(0, this.numVisibleTiles); for (const room of visibleRooms) { tiles.push( Date: Thu, 23 Jul 2020 22:12:10 -0600 Subject: [PATCH 2/9] Don't constantly re-mount the sublists with a new addRoomFn Any time we though that the room list had to re-render we were dynamically creating a new addRoomFn, which would signal to the sublist that it needed to re-render. The only reason we wrap the function from the aesthetics is to provide theoretical tiling/multiaccount support through use of different dispatchers, however considering that's not a reality yet we can just use a default dispatcher when none is supplied. --- src/components/views/rooms/RoomList.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 057b327aed..8d1879418a 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -81,7 +81,7 @@ interface ITagAesthetics { sectionLabel: string; sectionLabelRaw?: string; addRoomLabel?: string; - onAddRoom?: (dispatcher: Dispatcher) => void; + onAddRoom?: (dispatcher?: Dispatcher) => void; isInvite: boolean; defaultHidden: boolean; } @@ -105,14 +105,18 @@ const TAG_AESTHETICS: { isInvite: false, defaultHidden: false, addRoomLabel: _td("Start chat"), - onAddRoom: (dispatcher: Dispatcher) => dispatcher.dispatch({action: 'view_create_chat'}), + onAddRoom: (dispatcher?: Dispatcher) => { + (dispatcher || defaultDispatcher).dispatch({action: 'view_create_chat'}); + }, }, [DefaultTagID.Untagged]: { sectionLabel: _td("Rooms"), isInvite: false, defaultHidden: false, addRoomLabel: _td("Create room"), - onAddRoom: (dispatcher: Dispatcher) => dispatcher.dispatch({action: 'view_create_room'}), + onAddRoom: (dispatcher?: Dispatcher) => { + (dispatcher || defaultDispatcher).dispatch({action: 'view_create_room'}) + }, }, [DefaultTagID.LowPriority]: { sectionLabel: _td("Low priority"), @@ -304,7 +308,6 @@ export default class RoomList extends React.Component { : TAG_AESTHETICS[orderedTagId]; if (!aesthetics) throw new Error(`Tag ${orderedTagId} does not have aesthetics`); - const onAddRoomFn = aesthetics.onAddRoom ? () => aesthetics.onAddRoom(dis) : null; components.push( { forRooms={true} startAsHidden={aesthetics.defaultHidden} label={aesthetics.sectionLabelRaw ? aesthetics.sectionLabelRaw : _t(aesthetics.sectionLabel)} - onAddRoom={onAddRoomFn} + onAddRoom={aesthetics.onAddRoom} addRoomLabel={aesthetics.addRoomLabel ? _t(aesthetics.addRoomLabel) : aesthetics.addRoomLabel} isMinimized={this.props.isMinimized} onResize={this.props.onResize} From 9969b01c5f5270aec26eae32732c07b42d42ad9a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 23 Jul 2020 22:13:32 -0600 Subject: [PATCH 3/9] Only render sublists when they change significantly We can ignore off-screen updates, so do that. See diff for more details on what we're doing. --- src/components/views/rooms/RoomSublist.tsx | 50 ++++++++++++++++++++++ src/utils/objects.ts | 36 +++++++++++++++- 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 1ca3a0006f..a15a6c9cde 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -48,6 +48,7 @@ import { polyfillTouchEvent } from "../../../@types/polyfill"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; import { arrayHasDiff, arrayHasOrderChange } from "../../../utils/arrays"; +import { objectExcluding, objectHasValueChange } from "../../../utils/objects"; const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS @@ -174,6 +175,55 @@ export default class RoomSublist extends React.Component { } } + public shouldComponentUpdate(nextProps: Readonly, nextState: Readonly): boolean { + if (objectHasValueChange(this.props, nextProps)) { + // Something we don't care to optimize has updated, so update. + return true; + } + + // Do the same check used on props for state, without the rooms we're going to no-op + const prevStateNoRooms = objectExcluding(this.state, ['rooms']); + const nextStateNoRooms = objectExcluding(nextState, ['rooms']); + if (objectHasValueChange(prevStateNoRooms, nextStateNoRooms)) { + return true; + } + + // If we're supposed to handle extra tiles, take the performance hit and re-render all the + // time so we don't have to consider them as part of the visible room optimization. + const prevExtraTiles = this.props.extraBadTilesThatShouldntExist || []; + const nextExtraTiles = nextProps.extraBadTilesThatShouldntExist || []; + if (prevExtraTiles.length > 0 || nextExtraTiles.length > 0) { + return true; + } + + // If we're about to update the height of the list, we don't really care about which rooms + // are visible or not for no-op purposes, so ensure that the height calculation runs through. + if (RoomSublist.calcNumTiles(nextState.rooms, nextProps.extraBadTilesThatShouldntExist) !== this.numTiles) { + return true; + } + + // Finally, determine if the room update (as presumably that's all that's left) is within + // our visible range. If it is, then do a render. If the update is outside our visible range + // then we can skip the update. + // + // We also optimize for order changing here: if the update did happen in our visible range + // but doesn't result in the list re-sorting itself then there's no reason for us to update + // on our own. + const prevSlicedRooms = this.state.rooms.slice(0, this.numVisibleTiles); + const nextSlicedRooms = nextState.rooms.slice(0, this.numVisibleTiles); + if (arrayHasOrderChange(prevSlicedRooms, nextSlicedRooms)) { + return true; + } + + // Quickly double check we're not about to break something due to the number of rooms changing. + if (arrayHasDiff(this.state.rooms, nextState.rooms)) { + return true; + } + + // Finally, nothing happened so no-op the update + return false; + } + public componentWillUnmount() { this.state.notificationState.destroy(); defaultDispatcher.unregister(this.dispatcherRef); diff --git a/src/utils/objects.ts b/src/utils/objects.ts index 14fa928ce2..c3fbc64022 100644 --- a/src/utils/objects.ts +++ b/src/utils/objects.ts @@ -14,7 +14,41 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { arrayDiff, arrayMerge, arrayUnion } from "./arrays"; +import { arrayDiff, arrayHasDiff, arrayMerge, arrayUnion } from "./arrays"; + +/** + * Gets a new object which represents the provided object, excluding some properties. + * @param a The object to strip properties of. Must be defined. + * @param props The property names to remove. + * @returns The new object without the provided properties. + */ +export function objectExcluding(a: any, props: string[]): any { + // We use a Map to avoid hammering the `delete` keyword, which is slow and painful. + const tempMap = new Map(Object.entries(a)); + for (const prop of props) { + tempMap.delete(prop); + } + + // Convert the map to an object again + return Array.from(tempMap.entries()).reduce((c, [k, v]) => { + c[k] = v; + return c; + }, {}); +} + +/** + * Determines if the two objects, which are assumed to be of the same + * key shape, have a difference in their values. If a difference is + * determined, true is returned. + * @param a The first object. Must be defined. + * @param b The second object. Must be defined. + * @returns True if there's a perceptual difference in the object's values. + */ +export function objectHasValueChange(a: any, b: any): boolean { + const aValues = Object.values(a); + const bValues = Object.values(b); + return arrayHasDiff(aValues, bValues); +} /** * Determines the keys added, changed, and removed between two objects. From 7e50464eeb07a0a7857a346a400a75a2322b7f39 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 23 Jul 2020 22:19:16 -0600 Subject: [PATCH 4/9] Fix filtering causing sticky header artifacts In 7b97c3032b8c25874c8bbe9d71e64a379b20a46e we reduced the RoomList updates to just added/removed sublists, but didn't consider that we might also have to handle lengths of those sublists changing enough for us to fix the sticky headers. --- src/components/views/rooms/RoomList.tsx | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 8d1879418a..ef331fe1f5 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -239,7 +239,22 @@ export default class RoomList extends React.Component { const previousListIds = Object.keys(this.state.sublists); const newListIds = Object.keys(newLists); - if (arrayHasDiff(previousListIds, newListIds)) { + let doUpdate = arrayHasDiff(previousListIds, newListIds); + if (!doUpdate) { + // so we didn't have the visible sublists change, but did the contents of those + // sublists change significantly enough to break the sticky headers? Probably, so + // let's check the length of each. + for (const tagId of newListIds) { + const oldRooms = this.state.sublists[tagId]; + const newRooms = newLists[tagId]; + if (oldRooms.length !== newRooms.length) { + doUpdate = true; + break; + } + } + } + + if (doUpdate) { this.setState({sublists: newLists}, () => { this.props.onResize(); }); From fd15fc39849ccecb562f8488e987a1fbb95f52eb Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 23 Jul 2020 22:24:07 -0600 Subject: [PATCH 5/9] Ensure message previews update when needed In 9969b01c5f5270aec26eae32732c07b42d42ad9a we stopped updating the sublist whenever we felt like it, which indirectly froze message previews for room tiles (badges, unread state, etc were unaffected because that is managed by a different store). To fix this, we simply have to listen for changes and perform an update. --- src/components/views/rooms/RoomTile.tsx | 10 +++++++++- src/stores/room-list/MessagePreviewStore.ts | 6 ++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index 3265316c67..2c4d1d0163 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/src/components/views/rooms/RoomTile.tsx @@ -35,7 +35,7 @@ import { MenuItem, } from "../../structures/ContextMenu"; import { DefaultTagID, TagID } from "../../../stores/room-list/models"; -import { MessagePreviewStore } from "../../../stores/room-list/MessagePreviewStore"; +import { MessagePreviewStore, ROOM_PREVIEW_CHANGED } from "../../../stores/room-list/MessagePreviewStore"; import DecoratedRoomAvatar from "../avatars/DecoratedRoomAvatar"; import { getRoomNotifsState, @@ -128,6 +128,7 @@ export default class RoomTile extends React.Component { ActiveRoomObserver.addListener(this.props.room.roomId, this.onActiveRoomUpdate); this.dispatcherRef = defaultDispatcher.register(this.onAction); + MessagePreviewStore.instance.on(ROOM_PREVIEW_CHANGED, this.onRoomPreviewChanged); } private get showContextMenu(): boolean { @@ -150,6 +151,7 @@ export default class RoomTile extends React.Component { ActiveRoomObserver.removeListener(this.props.room.roomId, this.onActiveRoomUpdate); } defaultDispatcher.unregister(this.dispatcherRef); + MessagePreviewStore.instance.off(ROOM_PREVIEW_CHANGED, this.onRoomPreviewChanged); } private onAction = (payload: ActionPayload) => { @@ -160,6 +162,12 @@ export default class RoomTile extends React.Component { } }; + private onRoomPreviewChanged = (room: Room) => { + if (this.props.room && room.roomId === this.props.room.roomId) { + this.forceUpdate(); // we don't have any state to set, so just complain that we need an update + } + }; + private scrollIntoView = () => { if (!this.roomTileRef.current) return; this.roomTileRef.current.scrollIntoView({ diff --git a/src/stores/room-list/MessagePreviewStore.ts b/src/stores/room-list/MessagePreviewStore.ts index f61488c3bb..2803f0a23e 100644 --- a/src/stores/room-list/MessagePreviewStore.ts +++ b/src/stores/room-list/MessagePreviewStore.ts @@ -28,6 +28,10 @@ import { StickerEventPreview } from "./previews/StickerEventPreview"; import { ReactionEventPreview } from "./previews/ReactionEventPreview"; import { UPDATE_EVENT } from "../AsyncStore"; +// Emitted event for when a room's preview has changed. First argument will the room for which +// the change happened. +export const ROOM_PREVIEW_CHANGED = "room_preview_changed"; + const PREVIEWS = { 'm.room.message': { isState: false, @@ -146,6 +150,7 @@ export class MessagePreviewStore extends AsyncStoreWithClient { // We've muted the underlying Map, so just emit that we've changed. this.previews.set(room.roomId, map); this.emit(UPDATE_EVENT, this); + this.emit(ROOM_PREVIEW_CHANGED, room); } return; // we're done } @@ -153,6 +158,7 @@ export class MessagePreviewStore extends AsyncStoreWithClient { // At this point, we didn't generate a preview so clear it this.previews.set(room.roomId, new Map()); this.emit(UPDATE_EVENT, this); + this.emit(ROOM_PREVIEW_CHANGED, room); } protected async onAction(payload: ActionPayload) { From 82f90c473400c2ce643644b50c34dc87c8ab0914 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 23 Jul 2020 22:31:52 -0600 Subject: [PATCH 6/9] Do the faster length change check first ... because it's faster. Also we don't need to diff the array here. --- src/components/views/rooms/RoomSublist.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index a15a6c9cde..6438455a0a 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -47,7 +47,7 @@ import { Direction } from "re-resizable/lib/resizer"; import { polyfillTouchEvent } from "../../../@types/polyfill"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; -import { arrayHasDiff, arrayHasOrderChange } from "../../../utils/arrays"; +import { arrayHasOrderChange } from "../../../utils/arrays"; import { objectExcluding, objectHasValueChange } from "../../../utils/objects"; const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS @@ -202,6 +202,11 @@ export default class RoomSublist extends React.Component { return true; } + // Quickly double check we're not about to break something due to the number of rooms changing. + if (this.state.rooms.length !== nextState.rooms.length) { + return true; + } + // Finally, determine if the room update (as presumably that's all that's left) is within // our visible range. If it is, then do a render. If the update is outside our visible range // then we can skip the update. @@ -215,11 +220,6 @@ export default class RoomSublist extends React.Component { return true; } - // Quickly double check we're not about to break something due to the number of rooms changing. - if (arrayHasDiff(this.state.rooms, nextState.rooms)) { - return true; - } - // Finally, nothing happened so no-op the update return false; } From 0a31bd169c9ba6a31049e16aaccf1734db2a61fb Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 Jul 2020 10:20:00 -0600 Subject: [PATCH 7/9] Skip updates in collapsed lists too --- src/components/views/rooms/RoomSublist.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 6438455a0a..edbdfc0a2c 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -202,6 +202,13 @@ export default class RoomSublist extends React.Component { return true; } + // Before we go analyzing the rooms, we can see if we're collapsed. If we're collapsed, we don't need + // to render anything. We do this after the height check though to ensure that the height gets appropriately + // calculated for when/if we become uncollapsed. + if (!nextState.isExpanded) { + return false; + } + // Quickly double check we're not about to break something due to the number of rooms changing. if (this.state.rooms.length !== nextState.rooms.length) { return true; From 4d7980eb07aaa804cc5eeaeca844e398761ae9a8 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 Jul 2020 10:38:04 -0600 Subject: [PATCH 8/9] Ensure references to the room list store are broken for diffing See commit diff for details. --- src/components/views/rooms/RoomList.tsx | 9 +++++++-- src/utils/arrays.ts | 9 +++++++++ src/utils/objects.ts | 22 ++++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index ef331fe1f5..d8ea45eac2 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -42,7 +42,8 @@ import { ViewRoomDeltaPayload } from "../../../dispatcher/payloads/ViewRoomDelta import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import SettingsStore from "../../../settings/SettingsStore"; import CustomRoomTagStore from "../../../stores/CustomRoomTagStore"; -import { arrayHasDiff } from "../../../utils/arrays"; +import { arrayFastClone, arrayHasDiff } from "../../../utils/arrays"; +import { objectShallowClone } from "../../../utils/objects"; interface IProps { onKeyDown: (ev: React.KeyboardEvent) => void; @@ -255,7 +256,11 @@ export default class RoomList extends React.Component { } if (doUpdate) { - this.setState({sublists: newLists}, () => { + // We have to break our reference to the room list store if we want to be able to + // diff the object for changes, so do that. + const sublists = objectShallowClone(newLists, (k, v) => arrayFastClone(v)); + + this.setState({sublists: sublists}, () => { this.props.onResize(); }); } diff --git a/src/utils/arrays.ts b/src/utils/arrays.ts index ca82059b01..2a5b1b5f16 100644 --- a/src/utils/arrays.ts +++ b/src/utils/arrays.ts @@ -14,6 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ +/** + * Clones an array as fast as possible, retaining references of the array's values. + * @param a The array to clone. Must be defined. + * @returns A copy of the array. + */ +export function arrayFastClone(a: any[]): any[] { + return a.slice(0, a.length); +} + /** * Determines if the two arrays are different either in length, contents, * or order of those contents. diff --git a/src/utils/objects.ts b/src/utils/objects.ts index c3fbc64022..db8248759d 100644 --- a/src/utils/objects.ts +++ b/src/utils/objects.ts @@ -36,6 +36,28 @@ export function objectExcluding(a: any, props: string[]): any { }, {}); } +/** + * Clones an object to a caller-controlled depth. When a propertyCloner is supplied, the + * object's properties will be passed through it with the return value used as the new + * object's type. This is intended to be used to deep clone a reference, but without + * having to deep clone the entire object. This function is safe to call recursively within + * the propertyCloner. + * @param a The object to clone. Must be defined. + * @param propertyCloner The function to clone the properties of the object with, optionally. + * First argument is the property key with the second being the current value. + * @returns A cloned object. + */ +export function objectShallowClone(a: any, propertyCloner?: (k: string, v: any) => any): any { + const newObj = {}; + for (const [k, v] of Object.entries(a)) { + newObj[k] = v; + if (propertyCloner) { + newObj[k] = propertyCloner(k, v); + } + } + return newObj; +} + /** * Determines if the two objects, which are assumed to be of the same * key shape, have a difference in their values. If a difference is From d26fcb7f16f51fa6e301245c9797156ffb8ec7c1 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 Jul 2020 11:16:40 -0600 Subject: [PATCH 9/9] Update src/components/views/rooms/RoomList.tsx Co-authored-by: J. Ryan Stinnett --- src/components/views/rooms/RoomList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index d8ea45eac2..dd8b567c26 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -260,7 +260,7 @@ export default class RoomList extends React.Component { // diff the object for changes, so do that. const sublists = objectShallowClone(newLists, (k, v) => arrayFastClone(v)); - this.setState({sublists: sublists}, () => { + this.setState({sublists}, () => { this.props.onResize(); }); }