Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Avoid looking up settings during timeline rendering #8313

Merged
merged 5 commits into from
Apr 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Unread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function eventTriggersUnreadCount(ev: MatrixEvent): boolean {
}

if (ev.isRedacted()) return false;
return haveRendererForEvent(ev);
return haveRendererForEvent(ev, false /* hidden messages should never trigger unread counts anyways */);
turt2live marked this conversation as resolved.
Show resolved Hide resolved
}

export function doesRoomHaveUnreadMessages(room: Room): boolean {
Expand Down
8 changes: 4 additions & 4 deletions src/components/structures/MessagePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
// displayed event in the current render cycle.
private readReceiptsByUserId: Record<string, IReadReceiptForUser> = {};

private readonly showHiddenEventsInTimeline: boolean;
private readonly _showHiddenEvents: boolean;
private readonly threadsEnabled: boolean;
private isMounted = false;

Expand Down Expand Up @@ -270,7 +270,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
// Cache these settings on mount since Settings is expensive to query,
// and we check this in a hot code path. This is also cached in our
// RoomContext, however we still need a fallback for roomless MessagePanels.
this.showHiddenEventsInTimeline = SettingsStore.getValue("showHiddenEventsInTimeline");
this._showHiddenEvents = SettingsStore.getValue("showHiddenEventsInTimeline");
this.threadsEnabled = SettingsStore.getValue("feature_thread");

this.showTypingNotificationsWatcherRef =
Expand Down Expand Up @@ -465,7 +465,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
};

public get showHiddenEvents(): boolean {
return this.context?.showHiddenEventsInTimeline ?? this.showHiddenEventsInTimeline;
return this.context?.showHiddenEvents ?? this._showHiddenEvents;
}

// TODO: Implement granular (per-room) hide options
Expand Down Expand Up @@ -748,7 +748,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
const willWantDateSeparator = this.wantsDateSeparator(mxEv, nextEv.getDate() || new Date());
lastInSection = willWantDateSeparator ||
mxEv.getSender() !== nextEv.getSender() ||
getEventDisplayInfo(nextEv).isInfoMessage ||
getEventDisplayInfo(nextEv, this.showHiddenEvents).isInfoMessage ||
!shouldFormContinuation(
mxEv, nextEv, this.showHiddenEvents, this.threadsEnabled, this.context.timelineRenderingType,
);
Expand Down
8 changes: 4 additions & 4 deletions src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export interface IRoomState {
showTwelveHourTimestamps: boolean;
readMarkerInViewThresholdMs: number;
readMarkerOutOfViewThresholdMs: number;
showHiddenEventsInTimeline: boolean;
showHiddenEvents: boolean;
showReadReceipts: boolean;
showRedactions: boolean;
showJoinLeaves: boolean;
Expand Down Expand Up @@ -271,7 +271,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
showTwelveHourTimestamps: SettingsStore.getValue("showTwelveHourTimestamps"),
readMarkerInViewThresholdMs: SettingsStore.getValue("readMarkerInViewThresholdMs"),
readMarkerOutOfViewThresholdMs: SettingsStore.getValue("readMarkerOutOfViewThresholdMs"),
showHiddenEventsInTimeline: SettingsStore.getValue("showHiddenEventsInTimeline"),
showHiddenEvents: SettingsStore.getValue("showHiddenEventsInTimeline"),
showReadReceipts: true,
showRedactions: true,
showJoinLeaves: true,
Expand Down Expand Up @@ -328,7 +328,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
this.setState({ readMarkerOutOfViewThresholdMs: value as number }),
),
SettingsStore.watchSetting("showHiddenEventsInTimeline", null, (...[,,, value]) =>
this.setState({ showHiddenEventsInTimeline: value as boolean }),
this.setState({ showHiddenEvents: value as boolean }),
),
];
}
Expand Down Expand Up @@ -1480,7 +1480,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
continue;
}

if (!haveRendererForEvent(mxEv, this.state.showHiddenEventsInTimeline)) {
if (!haveRendererForEvent(mxEv, this.state.showHiddenEvents)) {
// XXX: can this ever happen? It will make the result count
// not match the displayed count.
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/ThreadPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ const ThreadPanel: React.FC<IProps> = ({
<RoomContext.Provider value={{
...roomContext,
timelineRenderingType: TimelineRenderingType.ThreadsList,
showHiddenEventsInTimeline: true,
showHiddenEvents: true,
narrow,
}}>
<BaseCard
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/TimelinePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1511,7 +1511,7 @@ class TimelinePanel extends React.Component<IProps, IState> {

const shouldIgnore = !!ev.status || // local echo
(ignoreOwn && ev.getSender() === myUserId); // own message
const isWithoutTile = !haveRendererForEvent(ev, this.context?.showHiddenEventsInTimeline) ||
const isWithoutTile = !haveRendererForEvent(ev, this.context?.showHiddenEvents) ||
shouldHideEvent(ev, this.context);

if (isWithoutTile || !node) {
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/messages/TextualEvent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default class TextualEvent extends React.Component<IProps> {
static contextType = RoomContext;

public render() {
const text = TextForEvent.textForEvent(this.props.mxEvent, true, this.context?.showHiddenEventsInTimeline);
const text = TextForEvent.textForEvent(this.props.mxEvent, true, this.context?.showHiddenEvents);
if (!text) return null;
return <div className="mx_TextualEvent">{ text }</div>;
}
Expand Down
41 changes: 22 additions & 19 deletions src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1214,21 +1214,24 @@ export class UnwrappedEventTile extends React.Component<IProps, IState> {
msgOption = readAvatars;
}

const replyChain =
(haveRendererForEvent(this.props.mxEvent) && shouldDisplayReply(this.props.mxEvent))
? <ReplyChain
parentEv={this.props.mxEvent}
onHeightChanged={this.props.onHeightChanged}
ref={this.replyChain}
forExport={this.props.forExport}
permalinkCreator={this.props.permalinkCreator}
layout={this.props.layout}
alwaysShowTimestamps={this.props.alwaysShowTimestamps || this.state.hover}
isQuoteExpanded={isQuoteExpanded}
setQuoteExpanded={this.setQuoteExpanded}
getRelationsForEvent={this.props.getRelationsForEvent}
/>
: null;
let replyChain;
if (
haveRendererForEvent(this.props.mxEvent, this.context.showHiddenEvents) &&
shouldDisplayReply(this.props.mxEvent)
) {
replyChain = <ReplyChain
parentEv={this.props.mxEvent}
onHeightChanged={this.props.onHeightChanged}
ref={this.replyChain}
forExport={this.props.forExport}
permalinkCreator={this.props.permalinkCreator}
layout={this.props.layout}
alwaysShowTimestamps={this.props.alwaysShowTimestamps || this.state.hover}
isQuoteExpanded={isQuoteExpanded}
setQuoteExpanded={this.setQuoteExpanded}
getRelationsForEvent={this.props.getRelationsForEvent}
/>;
}

const isOwnEvent = this.props.mxEvent?.sender?.userId === MatrixClientPeg.get().getUserId();

Expand Down Expand Up @@ -1267,7 +1270,7 @@ export class UnwrappedEventTile extends React.Component<IProps, IState> {
highlightLink: this.props.highlightLink,
onHeightChanged: this.props.onHeightChanged,
permalinkCreator: this.props.permalinkCreator,
}) }
}, this.context.showHiddenEvents) }
</div>,
]);
}
Expand Down Expand Up @@ -1309,7 +1312,7 @@ export class UnwrappedEventTile extends React.Component<IProps, IState> {
highlightLink: this.props.highlightLink,
onHeightChanged: this.props.onHeightChanged,
permalinkCreator: this.props.permalinkCreator,
}) }
}, this.context.showHiddenEvents) }
{ actionBar }
<a href={permalink} onClick={this.onPermalinkClicked}>
{ timestamp }
Expand Down Expand Up @@ -1395,7 +1398,7 @@ export class UnwrappedEventTile extends React.Component<IProps, IState> {
highlightLink: this.props.highlightLink,
onHeightChanged: this.props.onHeightChanged,
permalinkCreator: this.props.permalinkCreator,
}) }
}, this.context.showHiddenEvents) }
</div>,
<a
className="mx_EventTile_senderDetailsLink"
Expand Down Expand Up @@ -1448,7 +1451,7 @@ export class UnwrappedEventTile extends React.Component<IProps, IState> {
highlightLink: this.props.highlightLink,
onHeightChanged: this.props.onHeightChanged,
permalinkCreator: this.props.permalinkCreator,
}) }
}, this.context.showHiddenEvents) }
{ keyRequestInfo }
{ actionBar }
{ this.props.layout === Layout.IRC && <>
Expand Down
6 changes: 4 additions & 2 deletions src/components/views/rooms/ReplyTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@ export default class ReplyTile extends React.PureComponent<IProps> {
const msgType = mxEvent.getContent().msgtype;
const evType = mxEvent.getType() as EventType;

const { hasRenderer, isInfoMessage, isSeeingThroughMessageHiddenForModeration } = getEventDisplayInfo(mxEvent);
const {
hasRenderer, isInfoMessage, isSeeingThroughMessageHiddenForModeration,
} = getEventDisplayInfo(mxEvent, false /* Replies are never hidden, so this should be fine */);
// This shouldn't happen: the caller should check we support this type
// before trying to instantiate us
if (!hasRenderer) {
Expand Down Expand Up @@ -177,7 +179,7 @@ export default class ReplyTile extends React.PureComponent<IProps> {
highlightLink: this.props.highlightLink,
onHeightChanged: this.props.onHeightChanged,
permalinkCreator: this.props.permalinkCreator,
}) }
}, false /* showHiddenEvents shouldn't be relevant */) }
</a>
</div>
);
Expand Down
6 changes: 3 additions & 3 deletions src/components/views/rooms/SearchResultTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export default class SearchResultTile extends React.Component<IProps> {
highlights = this.props.searchHighlights;
}

if (haveRendererForEvent(mxEv, this.context?.showHiddenEventsInTimeline)) {
if (haveRendererForEvent(mxEv, this.context?.showHiddenEvents)) {
// do we need a date separator since the last event?
const prevEv = timeline[j - 1];
// is this a continuation of the previous message?
Expand All @@ -87,7 +87,7 @@ export default class SearchResultTile extends React.Component<IProps> {
shouldFormContinuation(
prevEv,
mxEv,
this.context?.showHiddenEventsInTimeline,
this.context?.showHiddenEvents,
threadsEnabled,
TimelineRenderingType.Search,
);
Expand All @@ -102,7 +102,7 @@ export default class SearchResultTile extends React.Component<IProps> {
!shouldFormContinuation(
mxEv,
nextEv,
this.context?.showHiddenEventsInTimeline,
this.context?.showHiddenEvents,
threadsEnabled,
TimelineRenderingType.Search,
)
Expand Down
2 changes: 1 addition & 1 deletion src/contexts/RoomContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const RoomContext = createContext<IRoomState>({
showTwelveHourTimestamps: false,
readMarkerInViewThresholdMs: 3000,
readMarkerOutOfViewThresholdMs: 30000,
showHiddenEventsInTimeline: false,
showHiddenEvents: false,
showReadReceipts: true,
showRedactions: true,
showJoinLeaves: true,
Expand Down
24 changes: 17 additions & 7 deletions src/events/EventTileFactory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -141,19 +141,25 @@ const SINGULAR_STATE_EVENTS = new Set([
* Find an event tile factory for the given conditions.
* @param mxEvent The event.
* @param cli The matrix client to reference when needed.
* @param showHiddenEvents Whether hidden events should be shown.
* @param asHiddenEv When true, treat the event as always hidden.
* @returns The factory, or falsy if not possible.
*/
export function pickFactory(mxEvent: MatrixEvent, cli: MatrixClient, asHiddenEv?: boolean): Optional<Factory> {
export function pickFactory(
mxEvent: MatrixEvent,
cli: MatrixClient,
showHiddenEvents: boolean,
asHiddenEv?: boolean,
): Optional<Factory> {
const evType = mxEvent.getType(); // cache this to reduce call stack execution hits

// Note: we avoid calling SettingsStore unless absolutely necessary - this code is on the critical path.

if (asHiddenEv && SettingsStore.getValue("showHiddenEventsInTimeline")) {
if (asHiddenEv && showHiddenEvents) {
return JSONEventFactory;
}

const noEventFactoryFactory: (() => Optional<Factory>) = () => SettingsStore.getValue("showHiddenEventsInTimeline")
const noEventFactoryFactory: (() => Optional<Factory>) = () => showHiddenEvents
? JSONEventFactory
: undefined; // just don't render things that we shouldn't render

Expand Down Expand Up @@ -242,17 +248,19 @@ export function pickFactory(mxEvent: MatrixEvent, cli: MatrixClient, asHiddenEv?
* Render an event as a tile
* @param renderType The render type. Used to inform properties given to the eventual component.
* @param props The properties to provide to the eventual component.
* @param showHiddenEvents Whether hidden events should be shown.
* @param cli Optional client instance to use, otherwise the default MatrixClientPeg will be used.
* @returns The tile as JSX, or falsy if unable to render.
*/
export function renderTile(
renderType: TimelineRenderingType,
props: EventTileTypeProps,
showHiddenEvents: boolean,
cli?: MatrixClient,
): Optional<JSX.Element> {
cli = cli ?? MatrixClientPeg.get(); // because param defaults don't do the correct thing

const factory = pickFactory(props.mxEvent, cli);
const factory = pickFactory(props.mxEvent, cli, showHiddenEvents);
if (!factory) return undefined;

// Note that we split off the ones we actually care about here just to be sure that we're
Expand Down Expand Up @@ -316,16 +324,18 @@ export function renderTile(
/**
* A version of renderTile() specifically for replies.
* @param props The properties to specify on the eventual object.
* @param showHiddenEvents Whether hidden events should be shown.
* @param cli Optional client instance to use, otherwise the default MatrixClientPeg will be used.
* @returns The tile as JSX, or falsy if unable to render.
*/
export function renderReplyTile(
props: EventTileTypeProps,
showHiddenEvents: boolean,
cli?: MatrixClient,
): Optional<JSX.Element> {
cli = cli ?? MatrixClientPeg.get(); // because param defaults don't do the correct thing

const factory = pickFactory(props.mxEvent, cli);
const factory = pickFactory(props.mxEvent, cli, showHiddenEvents);
if (!factory) return undefined;

// See renderTile() for why we split off so much
Expand Down Expand Up @@ -367,7 +377,7 @@ export function isMessageEvent(ev: MatrixEvent): boolean {
return (messageTypes.includes(ev.getType() as EventType)) || M_POLL_START.matches(ev.getType());
}

export function haveRendererForEvent(mxEvent: MatrixEvent, showHiddenEvents?: boolean): boolean {
export function haveRendererForEvent(mxEvent: MatrixEvent, showHiddenEvents: boolean): boolean {
// Only show "Message deleted" tile for plain message events, encrypted events,
// and state events as they'll likely still contain enough keys to be relevant.
if (mxEvent.isRedacted() && !mxEvent.isEncrypted() && !isMessageEvent(mxEvent) && !mxEvent.isState()) {
Expand All @@ -377,7 +387,7 @@ export function haveRendererForEvent(mxEvent: MatrixEvent, showHiddenEvents?: bo
// No tile for replacement events since they update the original tile
if (mxEvent.isRelation(RelationType.Replace)) return false;

const handler = pickFactory(mxEvent, MatrixClientPeg.get());
const handler = pickFactory(mxEvent, MatrixClientPeg.get(), showHiddenEvents);
if (!handler) return false;
if (handler === TextualEventFactory) {
return hasText(mxEvent, showHiddenEvents);
Expand Down
2 changes: 2 additions & 0 deletions src/settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ export const SETTINGS: {[setting: string]: ISetting} = {
"feature_msc3531_hide_messages_pending_moderation": {
isFeature: true,
labsGroup: LabGroup.Moderation,
// Requires a reload since this setting is cached in EventUtils
controller: new ReloadOnChangeController(),
displayName: _td("Let moderators hide messages pending moderation."),
supportedLevels: LEVELS_FEATURE,
default: false,
Expand Down
8 changes: 4 additions & 4 deletions src/utils/EventRenderingUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { haveRendererForEvent, JitsiEventFactory, JSONEventFactory, pickFactory
import { MatrixClientPeg } from "../MatrixClientPeg";
import { getMessageModerationState, MessageModerationState } from "./EventUtils";

export function getEventDisplayInfo(mxEvent: MatrixEvent, hideEvent?: boolean): {
export function getEventDisplayInfo(mxEvent: MatrixEvent, showHiddenEvents: boolean, hideEvent?: boolean): {
isInfoMessage: boolean;
hasRenderer: boolean;
isBubbleMessage: boolean;
Expand All @@ -52,7 +52,7 @@ export function getEventDisplayInfo(mxEvent: MatrixEvent, hideEvent?: boolean):
}

// TODO: Thread a MatrixClient through to here
let factory = pickFactory(mxEvent, MatrixClientPeg.get());
let factory = pickFactory(mxEvent, MatrixClientPeg.get(), showHiddenEvents);

// Info messages are basically information about commands processed on a room
let isBubbleMessage = (
Expand Down Expand Up @@ -92,11 +92,11 @@ export function getEventDisplayInfo(mxEvent: MatrixEvent, hideEvent?: boolean):
// source tile when there's no regular tile for an event and also for
// replace relations (which otherwise would display as a confusing
// duplicate of the thing they are replacing).
if (hideEvent || !haveRendererForEvent(mxEvent)) {
if (hideEvent || !haveRendererForEvent(mxEvent, showHiddenEvents)) {
// forcefully ask for a factory for a hidden event (hidden event
// setting is checked internally)
// TODO: Thread a MatrixClient through to here
factory = pickFactory(mxEvent, MatrixClientPeg.get(), true);
factory = pickFactory(mxEvent, MatrixClientPeg.get(), showHiddenEvents, true);
if (factory === JSONEventFactory) {
isBubbleMessage = false;
// Reuse info message avatar and sender profile styling
Expand Down
Loading