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

Improve/add notifications for location and poll events #7552

Merged
merged 7 commits into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
9 changes: 8 additions & 1 deletion src/Notifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event";
import { Room } from "matrix-js-sdk/src/models/room";
import { logger } from "matrix-js-sdk/src/logger";
import { MsgType } from "matrix-js-sdk/src/@types/event";
import { LOCATION_EVENT_TYPE } from "matrix-js-sdk/src/@types/location";

import { MatrixClientPeg } from './MatrixClientPeg';
import SdkConfig from './SdkConfig';
Expand Down Expand Up @@ -56,10 +57,16 @@ This is useful when the content body contains fallback text that would explain t
type of tile.
*/
const msgTypeHandlers = {
[MsgType.KeyVerificationRequest]: (event) => {
[MsgType.KeyVerificationRequest]: (event: MatrixEvent) => {
const name = (event.sender || {}).name;
return _t("%(name)s is requesting verification", { name });
},
[LOCATION_EVENT_TYPE.name]: (event: MatrixEvent) => {
return TextForEvent.textForLocationEvent(event)();
},
[LOCATION_EVENT_TYPE.altName]: (event: MatrixEvent) => {
return TextForEvent.textForLocationEvent(event)();
},
};

export const Notifier = {
Expand Down
60 changes: 51 additions & 9 deletions src/TextForEvent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import { removeDirectionOverrideChars } from 'matrix-js-sdk/src/utils';
import { GuestAccess, HistoryVisibility, JoinRule } from "matrix-js-sdk/src/@types/partials";
import { EventType, MsgType } from "matrix-js-sdk/src/@types/event";
import { EmoteEvent, NoticeEvent, MessageEvent } from "matrix-events-sdk";
import { POLL_END_EVENT_TYPE, POLL_START_EVENT_TYPE } from "matrix-js-sdk/src/@types/polls";
import { LOCATION_EVENT_TYPE } from "matrix-js-sdk/src/@types/location";
import { TEXT_NODE_TYPE } from "matrix-js-sdk/src/@types/extensible_events";

import { _t } from './languageHandler';
import * as Roles from './Roles';
Expand All @@ -36,12 +39,16 @@ import { ROOM_SECURITY_TAB } from "./components/views/dialogs/RoomSettingsDialog
import AccessibleButton from './components/views/elements/AccessibleButton';
import RightPanelStore from './stores/right-panel/RightPanelStore';

export function getSenderName(event: MatrixEvent): string {
return event.sender?.name ?? event.getSender() ?? _t("Someone");
}

// These functions are frequently used just to check whether an event has
// 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 textForCallInviteEvent(event: MatrixEvent): () => string | null {
const getSenderName = () => event.sender ? event.sender.name : _t('Someone');
const senderName = getSenderName(event);
// FIXME: Find a better way to determine this from the event?
let isVoice = true;
if (event.getContent().offer && event.getContent().offer.sdp &&
Expand All @@ -55,19 +62,19 @@ function textForCallInviteEvent(event: MatrixEvent): () => string | null {
// and more accurate, we break out the string-based variables to a couple booleans.
if (isVoice && isSupported) {
return () => _t("%(senderName)s placed a voice call.", {
senderName: getSenderName(),
senderName: senderName,
});
} else if (isVoice && !isSupported) {
return () => _t("%(senderName)s placed a voice call. (not supported by this browser)", {
senderName: getSenderName(),
senderName: senderName,
});
} else if (!isVoice && isSupported) {
return () => _t("%(senderName)s placed a video call.", {
senderName: getSenderName(),
senderName: senderName,
});
} else if (!isVoice && !isSupported) {
return () => _t("%(senderName)s placed a video call. (not supported by this browser)", {
senderName: getSenderName(),
senderName: senderName,
});
}
}
Expand Down Expand Up @@ -325,6 +332,17 @@ function textForServerACLEvent(ev: MatrixEvent): () => string | null {
}

function textForMessageEvent(ev: MatrixEvent): () => string | null {
const type = ev.getType();
const content = ev.getContent();
const msgtype = content.msgtype;

if (
(LOCATION_EVENT_TYPE.matches(type) || LOCATION_EVENT_TYPE.matches(msgtype)) &&
SettingsStore.getValue("feature_location_share")
) {
return textForLocationEvent(ev);
}

return () => {
const senderDisplayName = ev.sender && ev.sender.name ? ev.sender.name : ev.getSender();
let message = ev.getContent().body;
Expand Down Expand Up @@ -418,7 +436,7 @@ function textForCanonicalAliasEvent(ev: MatrixEvent): () => string | null {
}

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

if (!isValid3pidInvite(event)) {
return () => _t('%(senderName)s revoked the invitation for %(targetDisplayName)s to join the room.', {
Expand All @@ -434,7 +452,7 @@ function textForThreePidInviteEvent(event: MatrixEvent): () => string | null {
}

function textForHistoryVisibilityEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender ? event.sender.name : event.getSender();
const senderName = getSenderName(event);
switch (event.getContent().history_visibility) {
case HistoryVisibility.Invited:
return () => _t('%(senderName)s made future room history visible to all room members, '
Expand All @@ -456,7 +474,7 @@ function textForHistoryVisibilityEvent(event: MatrixEvent): () => string | null

// Currently will only display a change if a user's power level is changed
function textForPowerEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender ? event.sender.name : event.getSender();
const senderName = getSenderName(event);
if (!event.getPrevContent() || !event.getPrevContent().users ||
!event.getContent() || !event.getContent().users) {
return null;
Expand Down Expand Up @@ -523,7 +541,7 @@ const onPinnedMessagesClick = (): void => {

function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string | JSX.Element | null {
if (!SettingsStore.getValue("feature_pinning")) return null;
const senderName = event.sender ? event.sender.name : event.getSender();
const senderName = getSenderName(event);
const roomId = event.getRoomId();

const pinned = event.getContent().pinned ?? [];
Expand Down Expand Up @@ -729,6 +747,26 @@ function textForMjolnirEvent(event: MatrixEvent): () => string | null {
"for %(reason)s", { senderName, oldGlob: prevEntity, newGlob: entity, reason });
}

export function textForLocationEvent(event: MatrixEvent): () => string | null {
return () => _t("%(senderName)s has shared their location", {
senderName: getSenderName(event),
});
}

function textForPollStartEvent(event: MatrixEvent): () => string | null {
return () => _t("%(senderName)s has started a poll - %(pollQuestions)s", {
senderName: getSenderName(event),
pollQuestions: event.getContent()[POLL_START_EVENT_TYPE.name].question[TEXT_NODE_TYPE.name],
});
}

function textForPollEndEvent(event: MatrixEvent): () => string | null {
return () => _t("%(senderName)s has ended a poll - %(pollQuestions)s", {
senderName: getSenderName(event),
pollQuestions: event.getContent()[POLL_START_EVENT_TYPE.name].question[TEXT_NODE_TYPE.name],
});
}

interface IHandlers {
[type: string]:
(ev: MatrixEvent, allowJSX: boolean, showHiddenEvents?: boolean) =>
Expand All @@ -739,6 +777,10 @@ const handlers: IHandlers = {
[EventType.RoomMessage]: textForMessageEvent,
[EventType.Sticker]: textForMessageEvent,
[EventType.CallInvite]: textForCallInviteEvent,
[POLL_START_EVENT_TYPE.name]: textForPollStartEvent,
[POLL_END_EVENT_TYPE.name]: textForPollEndEvent,
[POLL_START_EVENT_TYPE.altName]: textForPollStartEvent,
[POLL_END_EVENT_TYPE.altName]: textForPollEndEvent,
SimonBrandner marked this conversation as resolved.
Show resolved Hide resolved
};

const stateHandlers: IHandlers = {
Expand Down
3 changes: 3 additions & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,9 @@
"%(senderName)s changed a rule that was banning rooms matching %(oldGlob)s to matching %(newGlob)s for %(reason)s": "%(senderName)s changed a rule that was banning rooms matching %(oldGlob)s to matching %(newGlob)s for %(reason)s",
"%(senderName)s changed a rule that was banning servers matching %(oldGlob)s to matching %(newGlob)s for %(reason)s": "%(senderName)s changed a rule that was banning servers matching %(oldGlob)s to matching %(newGlob)s for %(reason)s",
"%(senderName)s updated a ban rule that was matching %(oldGlob)s to matching %(newGlob)s for %(reason)s": "%(senderName)s updated a ban rule that was matching %(oldGlob)s to matching %(newGlob)s for %(reason)s",
"%(senderName)s has shared their location": "%(senderName)s has shared their location",
"%(senderName)s has started a poll - %(pollQuestions)s": "%(senderName)s has started a poll - %(pollQuestions)s",
"%(senderName)s has ended a poll - %(pollQuestions)s": "%(senderName)s has ended a poll - %(pollQuestions)s",
"Light": "Light",
"Light high contrast": "Light high contrast",
"Dark": "Dark",
Expand Down
14 changes: 13 additions & 1 deletion test/TextForEvent-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import './skinned-sdk';
import { MatrixEvent } from "matrix-js-sdk";
import renderer from 'react-test-renderer';

import { textForEvent } from "../src/TextForEvent";
import { getSenderName, textForEvent } from "../src/TextForEvent";
import SettingsStore from "../src/settings/SettingsStore";
import { SettingLevel } from "../src/settings/SettingLevel";

Expand Down Expand Up @@ -54,6 +54,18 @@ function renderComponent(component): string {
}

describe('TextForEvent', () => {
describe("getSenderName()", () => {
it("Prefers sender.name", () => {
expect(getSenderName({ sender: { name: "Alice" } } as MatrixEvent)).toBe("Alice");
});
it("Handles missing sender", () => {
expect(getSenderName({ getSender: () => "Alice" } as MatrixEvent)).toBe("Alice");
});
it("Handles missing sender and get sender", () => {
expect(getSenderName({ getSender: () => undefined } as MatrixEvent)).toBe("Someone");
});
});

describe("TextForPinnedEvent", () => {
SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true);

Expand Down