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

Commit

Permalink
Merge pull request #6205 from robintown/text-for-event-perf
Browse files Browse the repository at this point in the history
  • Loading branch information
@t3chguy
t3chguy authored Jul 16, 2021
2 parents aaa9040 + 092fdf5 commit 0f15cee
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 54 deletions.
52 changes: 33 additions & 19 deletions src/TextForEvent.tsx
View file
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
View file
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");

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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 0f15cee

Please sign in to comment.