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.
pull/21833/head
Travis Ralston 2020-07-23 21:23:45 -06:00
parent 60a6b13f4b
commit 7b97c3032b
3 changed files with 58 additions and 22 deletions

View File

@ -42,6 +42,7 @@ import { ViewRoomDeltaPayload } from "../../../dispatcher/payloads/ViewRoomDelta
import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore";
import SettingsStore from "../../../settings/SettingsStore"; import SettingsStore from "../../../settings/SettingsStore";
import CustomRoomTagStore from "../../../stores/CustomRoomTagStore"; import CustomRoomTagStore from "../../../stores/CustomRoomTagStore";
import { arrayHasDiff } from "../../../utils/arrays";
interface IProps { interface IProps {
onKeyDown: (ev: React.KeyboardEvent) => void; onKeyDown: (ev: React.KeyboardEvent) => void;
@ -231,9 +232,14 @@ export default class RoomList extends React.Component<IProps, IState> {
console.log("new lists", newLists); console.log("new lists", newLists);
} }
this.setState({sublists: newLists}, () => { const previousListIds = Object.keys(this.state.sublists);
this.props.onResize(); const newListIds = Object.keys(newLists);
});
if (arrayHasDiff(previousListIds, newListIds)) {
this.setState({sublists: newLists}, () => {
this.props.onResize();
});
}
}; };
private renderCommunityInvites(): React.ReactElement[] { private renderCommunityInvites(): React.ReactElement[] {
@ -304,7 +310,6 @@ export default class RoomList extends React.Component<IProps, IState> {
key={`sublist-${orderedTagId}`} key={`sublist-${orderedTagId}`}
tagId={orderedTagId} tagId={orderedTagId}
forRooms={true} forRooms={true}
rooms={orderedRooms}
startAsHidden={aesthetics.defaultHidden} startAsHidden={aesthetics.defaultHidden}
label={aesthetics.sectionLabelRaw ? aesthetics.sectionLabelRaw : _t(aesthetics.sectionLabel)} label={aesthetics.sectionLabelRaw ? aesthetics.sectionLabelRaw : _t(aesthetics.sectionLabel)}
onAddRoom={onAddRoomFn} onAddRoom={onAddRoomFn}

View File

@ -32,7 +32,7 @@ import {
StyledMenuItemCheckbox, StyledMenuItemCheckbox,
StyledMenuItemRadio, StyledMenuItemRadio,
} from "../../structures/ContextMenu"; } 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 { ListAlgorithm, SortAlgorithm } from "../../../stores/room-list/algorithms/models";
import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import { DefaultTagID, TagID } from "../../../stores/room-list/models";
import dis from "../../../dispatcher/dispatcher"; import dis from "../../../dispatcher/dispatcher";
@ -47,6 +47,7 @@ import { Direction } from "re-resizable/lib/resizer";
import { polyfillTouchEvent } from "../../../@types/polyfill"; import { polyfillTouchEvent } from "../../../@types/polyfill";
import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore";
import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore";
import { arrayHasDiff, arrayHasOrderChange } from "../../../utils/arrays";
const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS
const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS
@ -59,7 +60,6 @@ polyfillTouchEvent();
interface IProps { interface IProps {
forRooms: boolean; forRooms: boolean;
rooms?: Room[];
startAsHidden: boolean; startAsHidden: boolean;
label: string; label: string;
onAddRoom?: () => void; onAddRoom?: () => void;
@ -90,6 +90,7 @@ interface IState {
isResizing: boolean; isResizing: boolean;
isExpanded: boolean; // used for the for expand of the sublist when the room list is being filtered isExpanded: boolean; // used for the for expand of the sublist when the room list is being filtered
height: number; height: number;
rooms: Room[];
} }
export default class RoomSublist extends React.Component<IProps, IState> { export default class RoomSublist extends React.Component<IProps, IState> {
@ -104,16 +105,19 @@ export default class RoomSublist extends React.Component<IProps, IState> {
this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId); this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId);
this.heightAtStart = 0; this.heightAtStart = 0;
const height = this.calculateInitialHeight();
this.state = { this.state = {
notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId), notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId),
contextMenuPosition: null, contextMenuPosition: null,
isResizing: false, isResizing: false,
isExpanded: this.props.isFiltered ? this.props.isFiltered : !this.layout.isCollapsed, 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); this.dispatcherRef = defaultDispatcher.register(this.onAction);
RoomListStore.instance.on(LISTS_UPDATE_EVENT, this.onListsUpdated);
} }
private calculateInitialHeight() { private calculateInitialHeight() {
@ -142,11 +146,11 @@ export default class RoomSublist extends React.Component<IProps, IState> {
} }
private get numTiles(): number { private get numTiles(): number {
return RoomSublist.calcNumTiles(this.props); return RoomSublist.calcNumTiles(this.state.rooms, this.props.extraBadTilesThatShouldntExist);
} }
private static calcNumTiles(props) { private static calcNumTiles(rooms: Room[], extraTiles: any[]) {
return (props.rooms || []).length + (props.extraBadTilesThatShouldntExist || []).length; return (rooms || []).length + (extraTiles || []).length;
} }
private get numVisibleTiles(): number { private get numVisibleTiles(): number {
@ -154,8 +158,8 @@ export default class RoomSublist extends React.Component<IProps, IState> {
return Math.min(nVisible, this.numTiles); return Math.min(nVisible, this.numTiles);
} }
public componentDidUpdate(prevProps: Readonly<IProps>) { public componentDidUpdate(prevProps: Readonly<IProps>, prevState: Readonly<IState>) {
this.state.notificationState.setRooms(this.props.rooms); this.state.notificationState.setRooms(this.state.rooms);
if (prevProps.isFiltered !== this.props.isFiltered) { if (prevProps.isFiltered !== this.props.isFiltered) {
if (this.props.isFiltered) { if (this.props.isFiltered) {
this.setState({isExpanded: true}); this.setState({isExpanded: true});
@ -165,7 +169,7 @@ export default class RoomSublist extends React.Component<IProps, IState> {
} }
// as the rooms can come in one by one we need to reevaluate // 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 // 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()}); this.setState({height: this.calculateInitialHeight()});
} }
} }
@ -173,14 +177,23 @@ export default class RoomSublist extends React.Component<IProps, IState> {
public componentWillUnmount() { public componentWillUnmount() {
this.state.notificationState.destroy(); this.state.notificationState.destroy();
defaultDispatcher.unregister(this.dispatcherRef); 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) => { 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 // 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. // where we lose the room we are changing from temporarily and then it comes back in an update right after.
setImmediate(() => { 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) { if (!this.state.isExpanded && roomIndex > -1) {
this.toggleCollapsed(); this.toggleCollapsed();
@ -302,10 +315,10 @@ export default class RoomSublist extends React.Component<IProps, IState> {
let room; let room;
if (this.props.tagId === DefaultTagID.Invite) { if (this.props.tagId === DefaultTagID.Invite) {
// switch to first room as that'll be the top of the list for the user // 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 { } else {
// find the first room with a count of the same colour as the badge count // 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); const notifState = this.state.notificationState.getForRoom(r);
return notifState.count > 0 && notifState.color === this.state.notificationState.color; return notifState.count > 0 && notifState.color === this.state.notificationState.color;
}); });
@ -399,8 +412,8 @@ export default class RoomSublist extends React.Component<IProps, IState> {
const tiles: React.ReactElement[] = []; const tiles: React.ReactElement[] = [];
if (this.props.rooms) { if (this.state.rooms) {
const visibleRooms = this.props.rooms.slice(0, this.numVisibleTiles); const visibleRooms = this.state.rooms.slice(0, this.numVisibleTiles);
for (const room of visibleRooms) { for (const room of visibleRooms) {
tiles.push( tiles.push(
<RoomTile <RoomTile

View File

@ -14,11 +14,29 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
/**
* Determines if the two arrays are different either in length, contents,
* or order of those contents.
* @param a The first array. Must be defined.
* @param b The second array. Must be defined.
* @returns True if they are different, false otherwise.
*/
export function arrayHasOrderChange(a: any[], b: any[]): boolean {
if (a.length === b.length) {
for (let i = 0; i < a.length; i++) {
if (a[i] !== b[i]) return true;
}
return false;
} else {
return true; // like arrayHasDiff, a difference in length is a natural change
}
}
/** /**
* Determines if two arrays are different through a shallow comparison. * Determines if two arrays are different through a shallow comparison.
* @param a The first array. Must be defined. * @param a The first array. Must be defined.
* @param b The second array. Must be defined. * @param b The second array. Must be defined.
* @returns True if they are the same, false otherwise. * @returns True if they are different, false otherwise.
*/ */
export function arrayHasDiff(a: any[], b: any[]): boolean { export function arrayHasDiff(a: any[], b: any[]): boolean {
if (a.length === b.length) { if (a.length === b.length) {