From 1d45921d14d71e74fa8becb187029bcf390cf9e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 19 Jan 2022 01:58:31 +0100 Subject: [PATCH] Improve/add notifications for location and poll events (#7552) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add getSenderName() Signed-off-by: Šimon Brandner * Handle location and poll event notifications Signed-off-by: Šimon Brandner * i18n Signed-off-by: Šimon Brandner * pollQuestions -> pollQuestion Signed-off-by: Šimon Brandner * Make lookup safe and remove poll end event lookup as it wouldn't work Signed-off-by: Šimon Brandner * i18n Signed-off-by: Šimon Brandner --- src/Notifier.ts | 9 ++++- src/TextForEvent.tsx | 67 +++++++++++++++++++++++++++++++------ src/i18n/strings/en_EN.json | 3 ++ test/TextForEvent-test.ts | 14 +++++++- 4 files changed, 81 insertions(+), 12 deletions(-) diff --git a/src/Notifier.ts b/src/Notifier.ts index 35b3aee165..89153ef92c 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -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'; @@ -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 = { diff --git a/src/TextForEvent.tsx b/src/TextForEvent.tsx index e83c9efab6..83ecb58f4b 100644 --- a/src/TextForEvent.tsx +++ b/src/TextForEvent.tsx @@ -20,7 +20,16 @@ import { logger } from "matrix-js-sdk/src/logger"; 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 { M_EMOTE, M_NOTICE, M_MESSAGE, MessageEvent } from "matrix-events-sdk"; +import { + M_EMOTE, + M_NOTICE, + M_MESSAGE, + MessageEvent, + M_POLL_START, + M_POLL_END, + PollStartEvent, +} from "matrix-events-sdk"; +import { LOCATION_EVENT_TYPE } from "matrix-js-sdk/src/@types/location"; import { _t } from './languageHandler'; import * as Roles from './Roles'; @@ -36,12 +45,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 && @@ -55,19 +68,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, }); } } @@ -325,6 +338,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; @@ -418,7 +442,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.', { @@ -434,7 +458,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, ' @@ -456,7 +480,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; @@ -523,7 +547,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 ?? []; @@ -729,6 +753,25 @@ 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 - %(pollQuestion)s", { + senderName: getSenderName(event), + pollQuestion: (event.unstableExtensibleEvent as PollStartEvent)?.question?.text, + }); +} + +function textForPollEndEvent(event: MatrixEvent): () => string | null { + return () => _t("%(senderName)s has ended a poll", { + senderName: getSenderName(event), + }); +} + interface IHandlers { [type: string]: (ev: MatrixEvent, allowJSX: boolean, showHiddenEvents?: boolean) => @@ -739,6 +782,10 @@ const handlers: IHandlers = { [EventType.RoomMessage]: textForMessageEvent, [EventType.Sticker]: textForMessageEvent, [EventType.CallInvite]: textForCallInviteEvent, + [M_POLL_START.name]: textForPollStartEvent, + [M_POLL_END.name]: textForPollEndEvent, + [M_POLL_START.altName]: textForPollStartEvent, + [M_POLL_END.altName]: textForPollEndEvent, }; const stateHandlers: IHandlers = { diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index aa4f13dda6..f81ef8c1a7 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -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 - %(pollQuestion)s": "%(senderName)s has started a poll - %(pollQuestion)s", + "%(senderName)s has ended a poll": "%(senderName)s has ended a poll", "Light": "Light", "Light high contrast": "Light high contrast", "Dark": "Dark", diff --git a/test/TextForEvent-test.ts b/test/TextForEvent-test.ts index 90702395a6..4659de7eea 100644 --- a/test/TextForEvent-test.ts +++ b/test/TextForEvent-test.ts @@ -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"; @@ -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);