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

Avoid hitting the settings store from TextForEvent #6205

Merged
merged 17 commits into from
Jul 16, 2021
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
52 changes: 33 additions & 19 deletions src/TextForEvent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import React from 'react';
import { MatrixClientPeg } from './MatrixClientPeg';
import { _t } from './languageHandler';
Expand All @@ -32,7 +31,7 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event";
// any text to display at all. For this reason they return deferred values
// to avoid the expense of looking up translations when they're not needed.

function textForMemberEvent(ev: MatrixEvent): () => string | null {
function textForMemberEvent(ev: MatrixEvent, allowJSX: boolean, showHiddenEvents?: boolean): () => string | null {
// XXX: SYJS-16 "sender is sometimes null for join messages"
const senderName = ev.sender ? ev.sender.name : ev.getSender();
const targetName = ev.target ? ev.target.name : ev.getStateKey();
Expand Down Expand Up @@ -84,7 +83,7 @@ function textForMemberEvent(ev: MatrixEvent): () => string | null {
return () => _t('%(senderName)s changed their profile picture', { senderName });
} else if (!prevContent.avatar_url && content.avatar_url) {
return () => _t('%(senderName)s set a profile picture', { senderName });
} else if (SettingsStore.getValue("showHiddenEventsInTimeline")) {
} else if (showHiddenEvents ?? SettingsStore.getValue("showHiddenEventsInTimeline")) {
// This is a null rejoin, it will only be visible if using 'show hidden events' (labs)
return () => _t("%(senderName)s made no change", { senderName });
} else {
Expand Down Expand Up @@ -319,15 +318,15 @@ function textForCanonicalAliasEvent(ev: MatrixEvent): () => string | null {
});
}

function textForCallAnswerEvent(event): () => string | null {
function textForCallAnswerEvent(event: MatrixEvent): () => string | null {
return () => {
const senderName = event.sender ? event.sender.name : _t('Someone');
const supported = MatrixClientPeg.get().supportsVoip() ? '' : _t('(not supported by this browser)');
return _t('%(senderName)s answered the call.', { senderName }) + ' ' + supported;
};
}

function textForCallHangupEvent(event): () => string | null {
function textForCallHangupEvent(event: MatrixEvent): () => string | null {
const getSenderName = () => event.sender ? event.sender.name : _t('Someone');
const eventContent = event.getContent();
let getReason = () => "";
Expand Down Expand Up @@ -364,14 +363,14 @@ function textForCallHangupEvent(event): () => string | null {
return () => _t('%(senderName)s ended the call.', { senderName: getSenderName() }) + ' ' + getReason();
}

function textForCallRejectEvent(event): () => string | null {
function textForCallRejectEvent(event: MatrixEvent): () => string | null {
return () => {
const senderName = event.sender ? event.sender.name : _t('Someone');
return _t('%(senderName)s declined the call.', { senderName });
};
}

function textForCallInviteEvent(event): () => string | null {
function textForCallInviteEvent(event: MatrixEvent): () => string | null {
const getSenderName = () => event.sender ? event.sender.name : _t('Someone');
// FIXME: Find a better way to determine this from the event?
let isVoice = true;
Expand Down Expand Up @@ -403,7 +402,7 @@ function textForCallInviteEvent(event): () => string | null {
}
}

function textForThreePidInviteEvent(event): () => string | null {
function textForThreePidInviteEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender ? event.sender.name : event.getSender();

if (!isValid3pidInvite(event)) {
Expand All @@ -419,7 +418,7 @@ function textForThreePidInviteEvent(event): () => string | null {
});
}

function textForHistoryVisibilityEvent(event): () => string | null {
function textForHistoryVisibilityEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender ? event.sender.name : event.getSender();
switch (event.getContent().history_visibility) {
case 'invited':
Expand All @@ -441,7 +440,7 @@ function textForHistoryVisibilityEvent(event): () => string | null {
}

// Currently will only display a change if a user's power level is changed
function textForPowerEvent(event): () => string | null {
function textForPowerEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender ? event.sender.name : event.getSender();
if (!event.getPrevContent() || !event.getPrevContent().users ||
!event.getContent() || !event.getContent().users) {
Expand Down Expand Up @@ -523,7 +522,7 @@ function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string
return () => _t("%(senderName)s changed the pinned messages for the room.", { senderName });
}

function textForWidgetEvent(event): () => string | null {
function textForWidgetEvent(event: MatrixEvent): () => string | null {
const senderName = event.getSender();
const { name: prevName, type: prevType, url: prevUrl } = event.getPrevContent();
const { name, type, url } = event.getContent() || {};
Expand Down Expand Up @@ -553,12 +552,12 @@ function textForWidgetEvent(event): () => string | null {
}
}

function textForWidgetLayoutEvent(event): () => string | null {
function textForWidgetLayoutEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender?.name || event.getSender();
return () => _t("%(senderName)s has updated the widget layout", { senderName });
}

function textForMjolnirEvent(event): () => string | null {
function textForMjolnirEvent(event: MatrixEvent): () => string | null {
const senderName = event.getSender();
const { entity: prevEntity } = event.getPrevContent();
const { entity, recommendation, reason } = event.getContent();
Expand Down Expand Up @@ -646,7 +645,9 @@ function textForMjolnirEvent(event): () => string | null {
}

interface IHandlers {
[type: string]: (ev: MatrixEvent, allowJSX?: boolean) => (() => string | JSX.Element | null);
[type: string]:
(ev: MatrixEvent, allowJSX: boolean, showHiddenEvents?: boolean) =>
(() => string | JSX.Element | null);
}

const handlers: IHandlers = {
Expand Down Expand Up @@ -682,14 +683,27 @@ for (const evType of ALL_RULE_TYPES) {
stateHandlers[evType] = textForMjolnirEvent;
}

export function hasText(ev: MatrixEvent): boolean {
/**
* Determines whether the given event has text to display.
* @param ev The event
* @param showHiddenEvents An optional cached setting value for showHiddenEventsInTimeline
* to avoid hitting the settings store
*/
export function hasText(ev: MatrixEvent, showHiddenEvents?: boolean): boolean {
const handler = (ev.isState() ? stateHandlers : handlers)[ev.getType()];
return Boolean(handler?.(ev));
return Boolean(handler?.(ev, false, showHiddenEvents));
}

/**
* Gets the textual content of the given event.
* @param ev The event
* @param allowJSX Whether to output rich JSX content
* @param showHiddenEvents An optional cached setting value for showHiddenEventsInTimeline
* to avoid hitting the settings store
*/
export function textForEvent(ev: MatrixEvent): string;
export function textForEvent(ev: MatrixEvent, allowJSX: true): string | JSX.Element;
export function textForEvent(ev: MatrixEvent, allowJSX = false): string | JSX.Element {
export function textForEvent(ev: MatrixEvent, allowJSX: true, showHiddenEvents?: boolean): string | JSX.Element;
export function textForEvent(ev: MatrixEvent, allowJSX = false, showHiddenEvents?: boolean): string | JSX.Element {
const handler = (ev.isState() ? stateHandlers : handlers)[ev.getType()];
return handler?.(ev, allowJSX)?.() || '';
return handler?.(ev, allowJSX, showHiddenEvents)?.() || '';
}
30 changes: 20 additions & 10 deletions src/components/structures/MessagePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ const membershipTypes = [EventType.RoomMember, EventType.RoomThirdPartyInvite, E

// check if there is a previous event and it has the same sender as this event
// and the types are the same/is in continuedTypes and the time between them is <= CONTINUATION_MAX_INTERVAL
function shouldFormContinuation(prevEvent: MatrixEvent, mxEvent: MatrixEvent): boolean {
function shouldFormContinuation(
prevEvent: MatrixEvent,
mxEvent: MatrixEvent,
showHiddenEvents: boolean,
): boolean {
// sanity check inputs
if (!prevEvent || !prevEvent.sender || !mxEvent.sender) return false;
// check if within the max continuation period
Expand All @@ -74,7 +78,7 @@ function shouldFormContinuation(prevEvent: MatrixEvent, mxEvent: MatrixEvent): b
mxEvent.sender.getMxcAvatarUrl() !== prevEvent.sender.getMxcAvatarUrl()) return false;

// if we don't have tile for previous event then it was shown by showHiddenEvents and has no SenderProfile
if (!haveTileForEvent(prevEvent)) return false;
if (!haveTileForEvent(prevEvent, showHiddenEvents)) return false;

return true;
}
Expand Down Expand Up @@ -239,7 +243,8 @@ export default class MessagePanel extends React.Component<IProps, IState> {
};

// Cache hidden events setting on mount since Settings is expensive to
// query, and we check this in a hot code path.
// 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");
robintown marked this conversation as resolved.
Show resolved Hide resolved

this.showTypingNotificationsWatcherRef =
Expand Down Expand Up @@ -399,17 +404,21 @@ export default class MessagePanel extends React.Component<IProps, IState> {
return !this.isMounted;
};

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

// TODO: Implement granular (per-room) hide options
public shouldShowEvent(mxEv: MatrixEvent): boolean {
if (MatrixClientPeg.get().isUserIgnored(mxEv.getSender())) {
return false; // ignored = no show (only happens if the ignore happens after an event was received)
}

if (this.showHiddenEventsInTimeline) {
if (this.showHiddenEvents) {
return true;
}

if (!haveTileForEvent(mxEv)) {
if (!haveTileForEvent(mxEv, this.showHiddenEvents)) {
return false; // no tile = no show
}

Expand Down Expand Up @@ -569,7 +578,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {

if (grouper) {
if (grouper.shouldGroup(mxEv)) {
grouper.add(mxEv);
grouper.add(mxEv, this.showHiddenEvents);
continue;
} else {
// not part of group, so get the group tiles, close the
Expand Down Expand Up @@ -649,7 +658,8 @@ export default class MessagePanel extends React.Component<IProps, IState> {
}

// is this a continuation of the previous message?
const continuation = !wantsDateSeparator && shouldFormContinuation(prevEvent, mxEv);
const continuation = !wantsDateSeparator &&
shouldFormContinuation(prevEvent, mxEv, this.showHiddenEvents);

const eventId = mxEv.getId();
const highlight = (eventId === this.props.highlightedEventId);
Expand Down Expand Up @@ -946,7 +956,7 @@ abstract class BaseGrouper {
}

public abstract shouldGroup(ev: MatrixEvent): boolean;
public abstract add(ev: MatrixEvent): void;
public abstract add(ev: MatrixEvent, showHiddenEvents?: boolean): void;
public abstract getTiles(): ReactNode[];
public abstract getNewPrevEvent(): MatrixEvent;
}
Expand Down Expand Up @@ -1200,10 +1210,10 @@ class MemberGrouper extends BaseGrouper {
return membershipTypes.includes(ev.getType() as EventType);
}

public add(ev: MatrixEvent): void {
public add(ev: MatrixEvent, showHiddenEvents?: boolean): void {
if (ev.getType() === EventType.RoomMember) {
// We can ignore any events that don't actually have a message to display
if (!hasText(ev)) return;
if (!hasText(ev, showHiddenEvents)) return;
}
this.readMarker = this.readMarker || this.panel.readMarkerForEvent(
ev.getId(),
Expand Down
7 changes: 6 additions & 1 deletion src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ export interface IState {
canReply: boolean;
layout: Layout;
lowBandwidth: boolean;
showHiddenEventsInTimeline: boolean;
showReadReceipts: boolean;
showRedactions: boolean;
showJoinLeaves: boolean;
Expand Down Expand Up @@ -230,6 +231,7 @@ export default class RoomView extends React.Component<IProps, IState> {
canReply: false,
layout: SettingsStore.getValue("layout"),
lowBandwidth: SettingsStore.getValue("lowBandwidth"),
showHiddenEventsInTimeline: SettingsStore.getValue("showHiddenEventsInTimeline"),
showReadReceipts: true,
showRedactions: true,
showJoinLeaves: true,
Expand Down Expand Up @@ -267,6 +269,9 @@ export default class RoomView extends React.Component<IProps, IState> {
SettingsStore.watchSetting("lowBandwidth", null, () =>
this.setState({ lowBandwidth: SettingsStore.getValue("lowBandwidth") }),
),
SettingsStore.watchSetting("showHiddenEventsInTimeline", null, () =>
this.setState({ showHiddenEventsInTimeline: SettingsStore.getValue("showHiddenEventsInTimeline") }),
),
];
}

Expand Down Expand Up @@ -1388,7 +1393,7 @@ export default class RoomView extends React.Component<IProps, IState> {
continue;
}

if (!haveTileForEvent(mxEv)) {
if (!haveTileForEvent(mxEv, this.state.showHiddenEventsInTimeline)) {
// XXX: can this ever happen? It will make the result count
// not match the displayed count.
continue;
Expand Down
3 changes: 2 additions & 1 deletion src/components/structures/TimelinePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1337,7 +1337,8 @@ class TimelinePanel extends React.Component<IProps, IState> {

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

if (isWithoutTile || !node) {
// don't start counting if the event should be ignored,
Expand Down
17 changes: 9 additions & 8 deletions src/components/views/messages/TextualEvent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React from 'react';
import { MatrixEvent } from 'matrix-js-sdk/src/models/event';
import React from "react";
import { MatrixEvent } from "matrix-js-sdk/src/models/event";

import RoomContext from "../../../contexts/RoomContext";
import * as TextForEvent from "../../../TextForEvent";
import { replaceableComponent } from "../../../utils/replaceableComponent";

Expand All @@ -26,11 +27,11 @@ interface IProps {

@replaceableComponent("views.messages.TextualEvent")
export default class TextualEvent extends React.Component<IProps> {
render() {
const text = TextForEvent.textForEvent(this.props.mxEvent, true);
if (!text || (text as string).length === 0) return null;
return (
<div className="mx_TextualEvent">{ text }</div>
);
static contextType = RoomContext;

public render() {
const text = TextForEvent.textForEvent(this.props.mxEvent, true, this.context?.showHiddenEventsInTimeline);
if (!text) return null;
return <div className="mx_TextualEvent">{ text }</div>;
}
}
4 changes: 2 additions & 2 deletions src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,7 @@ function isMessageEvent(ev) {
return (messageTypes.includes(ev.getType()));
}

export function haveTileForEvent(e) {
export function haveTileForEvent(e: MatrixEvent, showHiddenEvents?: boolean) {
// Only messages have a tile (black-rectangle) if redacted
if (e.isRedacted() && !isMessageEvent(e)) return false;

Expand All @@ -1170,7 +1170,7 @@ export function haveTileForEvent(e) {
const handler = getHandlerTile(e);
if (handler === undefined) return false;
if (handler === 'messages.TextualEvent') {
return hasText(e);
return hasText(e, showHiddenEvents);
} else if (handler === 'messages.RoomCreate') {
return Boolean(e.getContent()['predecessor']);
} else {
Expand Down
Loading