From 09808fa7be02175bbc8fea8c1ae10b5027e7c4f6 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 Jul 2020 12:58:10 -0600 Subject: [PATCH 1/5] Only update RoomTiles when they change significantly --- src/components/views/rooms/RoomTile.tsx | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index 2c4d1d0163..990510ac7a 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/src/components/views/rooms/RoomTile.tsx @@ -55,14 +55,13 @@ import {ActionPayload} from "../../../dispatcher/payloads"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import { NotificationState } from "../../../stores/notifications/NotificationState"; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; +import { objectDiff, objectHasValueChange } from "../../../utils/objects"; interface IProps { room: Room; showMessagePreview: boolean; isMinimized: boolean; tag: TagID; - - // TODO: Incoming call boxes: https://github.com/vector-im/riot-web/issues/14177 } type PartialDOMRect = Pick; @@ -154,6 +153,24 @@ export default class RoomTile extends React.Component { MessagePreviewStore.instance.off(ROOM_PREVIEW_CHANGED, this.onRoomPreviewChanged); } + public shouldComponentUpdate(nextProps: Readonly, nextState: Readonly): boolean { + // Whenever a prop change happens (or our parent updates) we can get told to update too. Try + // to minimize that by seeing if anything actually changed. + if (objectHasValueChange(this.props, nextProps)) { + console.log(`DIFF_PROPS@${this.props.room.roomId}`, objectDiff(this.props, nextProps)); + return true; + } + + // Do the same for state + if (objectHasValueChange(this.state, nextState)) { + console.log(`DIFF_STATE@${this.props.room.roomId}`, objectDiff(this.state, nextState)); + return true; + } + + // Finally, nothing changed so say so. + return false; + } + private onAction = (payload: ActionPayload) => { if (payload.action === "view_room" && payload.room_id === this.props.room.roomId && payload.show_room_tile) { setImmediate(() => { From 37035f945b2a1507eb60d24d2bdc437dda1d1b33 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 Jul 2020 13:09:26 -0600 Subject: [PATCH 2/5] Move message previews into RoomTile's state Now that it doesn't re-render without state updates, we should just wedge it into state. --- src/components/views/rooms/RoomTile.tsx | 37 +++++++++++++++---------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index 990510ac7a..1edbe83d31 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/src/components/views/rooms/RoomTile.tsx @@ -72,6 +72,7 @@ interface IState { selected: boolean; notificationsMenuPosition: PartialDOMRect; generalMenuPosition: PartialDOMRect; + messagePreview?: string; } const messagePreviewId = (roomId: string) => `mx_RoomTile_messagePreview_${roomId}`; @@ -123,6 +124,9 @@ export default class RoomTile extends React.Component { selected: ActiveRoomObserver.activeRoomId === this.props.room.roomId, notificationsMenuPosition: null, generalMenuPosition: null, + + // generatePreview() will return nothing if the user has previews disabled + messagePreview: this.generatePreview(), }; ActiveRoomObserver.addListener(this.props.room.roomId, this.onActiveRoomUpdate); @@ -157,13 +161,13 @@ export default class RoomTile extends React.Component { // Whenever a prop change happens (or our parent updates) we can get told to update too. Try // to minimize that by seeing if anything actually changed. if (objectHasValueChange(this.props, nextProps)) { - console.log(`DIFF_PROPS@${this.props.room.roomId}`, objectDiff(this.props, nextProps)); + console.log(`DIFF_PROPS@${this.props.room.roomId}`, JSON.stringify(objectDiff(this.props, nextProps))); return true; } // Do the same for state if (objectHasValueChange(this.state, nextState)) { - console.log(`DIFF_STATE@${this.props.room.roomId}`, objectDiff(this.state, nextState)); + console.log(`DIFF_STATE@${this.props.room.roomId}`, JSON.stringify(objectDiff(this.state, nextState))); return true; } @@ -181,10 +185,19 @@ 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 + // generatePreview() will return nothing if the user has previews disabled + this.setState({messagePreview: this.generatePreview()}); } }; + private generatePreview(): string | null { + if (!this.showMessagePreview) { + return null; + } + + return MessagePreviewStore.instance.getPreviewForRoom(this.props.room, this.props.tag); + } + private scrollIntoView = () => { if (!this.roomTileRef.current) return; this.roomTileRef.current.scrollIntoView({ @@ -520,18 +533,12 @@ export default class RoomTile extends React.Component { name = name.replace(":", ":\u200b"); // add a zero-width space to allow linewrapping after the colon let messagePreview = null; - if (this.showMessagePreview) { - // The preview store heavily caches this info, so should be safe to hammer. - const text = MessagePreviewStore.instance.getPreviewForRoom(this.props.room, this.props.tag); - - // Only show the preview if there is one to show. - if (text) { - messagePreview = ( -
- {text} -
- ); - } + if (this.showMessagePreview && this.state.messagePreview) { + messagePreview = ( +
+ {this.state.messagePreview} +
+ ); } const nameClasses = classNames({ From c3623f439cd87edb8ac6abfc61c0063c74c73bbe Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 Jul 2020 13:10:39 -0600 Subject: [PATCH 3/5] Rip hover support out of the RoomTile component It's all handled by CSS and this literally does nothing but spam renders. --- src/components/views/rooms/RoomTile.tsx | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index 1edbe83d31..b8406dc045 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/src/components/views/rooms/RoomTile.tsx @@ -67,7 +67,6 @@ interface IProps { type PartialDOMRect = Pick; interface IState { - hover: boolean; notificationState: NotificationState; selected: boolean; notificationsMenuPosition: PartialDOMRect; @@ -119,7 +118,6 @@ export default class RoomTile extends React.Component { super(props); this.state = { - hover: false, notificationState: RoomNotificationStateStore.instance.getRoomState(this.props.room), selected: ActiveRoomObserver.activeRoomId === this.props.room.roomId, notificationsMenuPosition: null, @@ -206,14 +204,6 @@ export default class RoomTile extends React.Component { }); }; - private onTileMouseEnter = () => { - this.setState({hover: true}); - }; - - private onTileMouseLeave = () => { - this.setState({hover: false}); - }; - private onTileClick = (ev: React.KeyboardEvent) => { ev.preventDefault(); ev.stopPropagation(); @@ -592,8 +582,6 @@ export default class RoomTile extends React.Component { tabIndex={isActive ? 0 : -1} inputRef={ref} className={classes} - onMouseEnter={this.onTileMouseEnter} - onMouseLeave={this.onTileMouseLeave} onClick={this.onTileClick} onContextMenu={this.onContextMenu} role="treeitem" From e3800dba0e1bbd4fc277cd5dd2a9b156dae69f4a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 Jul 2020 13:23:51 -0600 Subject: [PATCH 4/5] Remove debug logging --- src/components/views/rooms/RoomTile.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index b8406dc045..b7275e5074 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/src/components/views/rooms/RoomTile.tsx @@ -159,13 +159,11 @@ export default class RoomTile extends React.Component { // Whenever a prop change happens (or our parent updates) we can get told to update too. Try // to minimize that by seeing if anything actually changed. if (objectHasValueChange(this.props, nextProps)) { - console.log(`DIFF_PROPS@${this.props.room.roomId}`, JSON.stringify(objectDiff(this.props, nextProps))); return true; } // Do the same for state if (objectHasValueChange(this.state, nextState)) { - console.log(`DIFF_STATE@${this.props.room.roomId}`, JSON.stringify(objectDiff(this.state, nextState))); return true; } From 46f9d44e640c0a8ff83e528e163aa15bce84f0e7 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 Jul 2020 13:58:21 -0600 Subject: [PATCH 5/5] Use PureComponent instead It ranges wildly between 10 and 50ms while ours is stable at 30-40ms, but the effort doesn't need to be duplicated. --- src/components/views/rooms/RoomTile.tsx | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index b7275e5074..259d2aa5b8 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/src/components/views/rooms/RoomTile.tsx @@ -55,7 +55,6 @@ import {ActionPayload} from "../../../dispatcher/payloads"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import { NotificationState } from "../../../stores/notifications/NotificationState"; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; -import { objectDiff, objectHasValueChange } from "../../../utils/objects"; interface IProps { room: Room; @@ -110,7 +109,7 @@ const NotifOption: React.FC = ({active, onClick, iconClassNam ); }; -export default class RoomTile extends React.Component { +export default class RoomTile extends React.PureComponent { private dispatcherRef: string; private roomTileRef = createRef(); @@ -155,22 +154,6 @@ export default class RoomTile extends React.Component { MessagePreviewStore.instance.off(ROOM_PREVIEW_CHANGED, this.onRoomPreviewChanged); } - public shouldComponentUpdate(nextProps: Readonly, nextState: Readonly): boolean { - // Whenever a prop change happens (or our parent updates) we can get told to update too. Try - // to minimize that by seeing if anything actually changed. - if (objectHasValueChange(this.props, nextProps)) { - return true; - } - - // Do the same for state - if (objectHasValueChange(this.state, nextState)) { - return true; - } - - // Finally, nothing changed so say so. - return false; - } - private onAction = (payload: ActionPayload) => { if (payload.action === "view_room" && payload.room_id === this.props.room.roomId && payload.show_room_tile) { setImmediate(() => {