From 35413917476c6cffce0c4e4b4f639fc4af61a81a Mon Sep 17 00:00:00 2001 From: taffyko Date: Tue, 22 Oct 2024 13:44:30 -0700 Subject: [PATCH 1/4] Fix outstanding UX issues with replies/mentions/keyword notifs --- res/css/views/elements/_Pill.pcss | 6 +- res/css/views/rooms/_EventTile.pcss | 14 +++-- res/themes/dark/css/_dark.pcss | 1 + res/themes/legacy-dark/css/_legacy-dark.pcss | 1 + .../legacy-light/css/_legacy-light.pcss | 1 + res/themes/light-custom/css/_custom.pcss | 1 + res/themes/light/css/_light.pcss | 1 + src/components/views/elements/Pill.tsx | 26 +++++++- src/components/views/messages/TextualBody.tsx | 63 ++++++++++++++++++- .../views/messages/TextualBody-test.tsx | 19 +++++- 10 files changed, 121 insertions(+), 12 deletions(-) diff --git a/res/css/views/elements/_Pill.pcss b/res/css/views/elements/_Pill.pcss index ccf82ff6d62..9fae4b47826 100644 --- a/res/css/views/elements/_Pill.pcss +++ b/res/css/views/elements/_Pill.pcss @@ -26,7 +26,8 @@ Please see LICENSE files in the repository root for full details. } &.mx_UserPill_me, - &.mx_AtRoomPill { + &.mx_AtRoomPill, + &.mx_KeywordPill { background-color: var(--cpd-color-bg-critical-primary) !important; /* To override .markdown-body */ } @@ -45,7 +46,8 @@ Please see LICENSE files in the repository root for full details. } /* We don't want to indicate clickability */ - &.mx_AtRoomPill:hover { + &.mx_AtRoomPill:hover, + &.mx_KeywordPill:hover { background-color: var(--cpd-color-bg-critical-primary) !important; /* To override .markdown-body */ cursor: unset; } diff --git a/res/css/views/rooms/_EventTile.pcss b/res/css/views/rooms/_EventTile.pcss index 311e0591667..09b9fe4f427 100644 --- a/res/css/views/rooms/_EventTile.pcss +++ b/res/css/views/rooms/_EventTile.pcss @@ -135,12 +135,6 @@ $left-gutter: 64px; } } - &.mx_EventTile_highlight, - &.mx_EventTile_highlight .markdown-body, - &.mx_EventTile_highlight .mx_EventTile_edited { - color: $alert; - } - &.mx_EventTile_bubbleContainer { display: grid; grid-template-columns: 1fr 100px; @@ -584,6 +578,14 @@ $left-gutter: 64px; padding-inline-start: calc(var(--EventTile_group_line-spacing-inline-start) + 20px); } } + + &.mx_EventTile_highlight, + &.mx_EventTile_highlight .markdown-body { + > .mx_EventTile_line { + box-shadow: inset var(--EventTile-box-shadow-offset-x) 0 0 var(--EventTile-box-shadow-spread-radius) + $event-highlight-edge-color; + } + } } &[data-layout="bubble"] { diff --git a/res/themes/dark/css/_dark.pcss b/res/themes/dark/css/_dark.pcss index 56630763fef..c83d9419b9b 100644 --- a/res/themes/dark/css/_dark.pcss +++ b/res/themes/dark/css/_dark.pcss @@ -83,6 +83,7 @@ $spacePanel-bg-color: rgba(38, 39, 43, 0.82); $panel-gradient: rgba(34, 38, 46, 0), rgba(34, 38, 46, 1); $h3-color: $primary-content; $event-highlight-bg-color: #25271f; +$event-highlight-edge-color: #f0b132; $header-panel-text-primary-color: $text-secondary-color; /* ******************** */ diff --git a/res/themes/legacy-dark/css/_legacy-dark.pcss b/res/themes/legacy-dark/css/_legacy-dark.pcss index c6840f5b907..41482a7353f 100644 --- a/res/themes/legacy-dark/css/_legacy-dark.pcss +++ b/res/themes/legacy-dark/css/_legacy-dark.pcss @@ -164,6 +164,7 @@ $widget-menu-bar-bg-color: $header-panel-bg-color; $widget-body-bg-color: #1a1d23; $event-highlight-bg-color: #25271f; +$event-highlight-edge-color: #f0b132; /* event timestamp */ $event-timestamp-color: $text-secondary-color; diff --git a/res/themes/legacy-light/css/_legacy-light.pcss b/res/themes/legacy-light/css/_legacy-light.pcss index 398cf0e1f10..e150aca0d55 100644 --- a/res/themes/legacy-light/css/_legacy-light.pcss +++ b/res/themes/legacy-light/css/_legacy-light.pcss @@ -235,6 +235,7 @@ $widget-body-bg-color: #fff; $yellow-background: #fff8e3; $event-highlight-bg-color: $yellow-background; +$event-highlight-edge-color: var(--cpd-color-yellow-500); /* event timestamp */ $event-timestamp-color: #acacac; diff --git a/res/themes/light-custom/css/_custom.pcss b/res/themes/light-custom/css/_custom.pcss index 0dfb38da98b..10b23c9b8da 100644 --- a/res/themes/light-custom/css/_custom.pcss +++ b/res/themes/light-custom/css/_custom.pcss @@ -100,6 +100,7 @@ $button-danger-disabled-bg-color: var(--warning-color-50pct); /* still needs alp /* --timeline-highlights-color */ $event-selected-color: var(--timeline-highlights-color); $event-highlight-bg-color: var(--timeline-highlights-color); +$event-highlight-edge-color: var(--timeline-highlights-color); /* redirect some variables away from their hardcoded values in the light theme */ $settings-grey-fg-color: $primary-content; diff --git a/res/themes/light/css/_light.pcss b/res/themes/light/css/_light.pcss index d649b6b38da..026ef2cc742 100644 --- a/res/themes/light/css/_light.pcss +++ b/res/themes/light/css/_light.pcss @@ -112,6 +112,7 @@ $spacePanel-bg-color: rgba(232, 232, 232, 0.77); $panel-gradient: rgba(242, 245, 248, 0), rgba(242, 245, 248, 1); $h3-color: #3d3b39; $event-highlight-bg-color: $yellow-background; +$event-highlight-edge-color: var(--cpd-color-yellow-500); $header-panel-text-primary-color: #91a1c0; /* ******************** */ diff --git a/src/components/views/elements/Pill.tsx b/src/components/views/elements/Pill.tsx index 14093587ad3..f1bef7c700e 100644 --- a/src/components/views/elements/Pill.tsx +++ b/src/components/views/elements/Pill.tsx @@ -25,6 +25,7 @@ export enum PillType { AtRoomMention = "TYPE_AT_ROOM_MENTION", // '@room' mention EventInSameRoom = "TYPE_EVENT_IN_SAME_ROOM", EventInOtherRoom = "TYPE_EVENT_IN_OTHER_ROOM", + Keyword = "TYPE_KEYWORD", // Used to highlight keywords that triggered a notification rule } export const pillRoomNotifPos = (text: string | null): number => { @@ -76,14 +77,32 @@ export interface PillProps { room?: Room; // Whether to include an avatar in the pill shouldShowPillAvatar?: boolean; + // Explicitly-provided text to display in the pill + text?: string; } -export const Pill: React.FC = ({ type: propType, url, inMessage, room, shouldShowPillAvatar = true }) => { - const { event, member, onClick, resourceId, targetRoom, text, type } = usePermalink({ +export const Pill: React.FC = ({ + type: propType, + url, + inMessage, + room, + shouldShowPillAvatar = true, + text: customPillText, +}) => { + const { + event, + member, + onClick, + resourceId, + targetRoom, + text: linkText, + type, + } = usePermalink({ room, type: propType, url, }); + const text = customPillText ?? linkText; if (!type || !text) { return null; @@ -96,6 +115,7 @@ export const Pill: React.FC = ({ type: propType, url, inMessage, room mx_UserPill: type === PillType.UserMention, mx_UserPill_me: resourceId === MatrixClientPeg.safeGet().getUserId(), mx_EventPill: type === PillType.EventInOtherRoom || type === PillType.EventInSameRoom, + mx_KeywordPill: type === PillType.Keyword, }); let avatar: ReactElement | null = null; @@ -131,6 +151,8 @@ export const Pill: React.FC = ({ type: propType, url, inMessage, room case PillType.UserMention: avatar = ; break; + case PillType.Keyword: + break; default: return null; } diff --git a/src/components/views/messages/TextualBody.tsx b/src/components/views/messages/TextualBody.tsx index 0e0c29747f9..23e3fa89da8 100644 --- a/src/components/views/messages/TextualBody.tsx +++ b/src/components/views/messages/TextualBody.tsx @@ -8,8 +8,9 @@ Please see LICENSE files in the repository root for full details. import React, { createRef, SyntheticEvent, MouseEvent } from "react"; import ReactDOM from "react-dom"; -import { MsgType } from "matrix-js-sdk/src/matrix"; +import { MsgType, PushRuleKind } from "matrix-js-sdk/src/matrix"; import { TooltipProvider } from "@vector-im/compound-web"; +import { globToRegexp } from "matrix-js-sdk/src/utils"; import * as HtmlUtils from "../../../HtmlUtils"; import { formatDate } from "../../../DateUtils"; @@ -36,6 +37,7 @@ import { EditWysiwygComposer } from "../rooms/wysiwyg_composer"; import { IEventTileOps } from "../rooms/EventTile"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; import CodeBlock from "./CodeBlock"; +import { Pill, PillType } from "../elements/Pill"; interface IState { // the URLs (if any) to be previewed with a LinkPreviewWidget inside this TextualBody. @@ -102,6 +104,16 @@ export default class TextualBody extends React.Component { } } } + + // Highlight notification keywords using pills + const pushDetails = this.props.mxEvent.getPushDetails(); + if ( + pushDetails.rule?.enabled && + pushDetails.rule.kind === PushRuleKind.ContentSpecific && + pushDetails.rule.pattern + ) { + this.pillifyNotificationKeywords([content], this.regExpForKeywordPattern(pushDetails.rule.pattern)); + } } private addCodeElement(pre: HTMLPreElement): void { @@ -211,6 +223,55 @@ export default class TextualBody extends React.Component { } } + /** + * Marks the text that activated a push-notification keyword pattern. + */ + private pillifyNotificationKeywords(nodes: ArrayLike, exp: RegExp): void { + let node: Node | null = nodes[0]; + while (node) { + if (node.nodeType === Node.TEXT_NODE) { + const text = node.nodeValue; + if (!text) { + node = node.nextSibling; + continue; + } + const match = text.match(exp); + if (!match || match.length < 3) { + node = node.nextSibling; + continue; + } + const keywordText = match[2]; + const idx = match.index! + match[1].length; + const before = text.substring(0, idx); + const after = text.substring(idx + keywordText.length); + + const container = document.createElement("span"); + const newContent = ( + <> + {before} + + + + {after} + + ); + ReactDOM.render(newContent, container); + + node.parentNode?.replaceChild(container, node); + } else if (node.childNodes && node.childNodes.length) { + this.pillifyNotificationKeywords(node.childNodes as NodeListOf, exp); + } + + node = node.nextSibling; + } + } + + private regExpForKeywordPattern(pattern: string): RegExp { + // Reflects the push notification pattern-matching implementation at + // https://github.com/matrix-org/matrix-js-sdk/blob/dbd7d26968b94700827bac525c39afff2c198e61/src/pushprocessor.ts#L570 + return new RegExp("(^|\\W)(" + globToRegexp(pattern) + ")(\\W|$)", "i"); + } + private findLinks(nodes: ArrayLike): string[] { let links: string[] = []; diff --git a/test/unit-tests/components/views/messages/TextualBody-test.tsx b/test/unit-tests/components/views/messages/TextualBody-test.tsx index c7ffc4ed933..ce1a53565ed 100644 --- a/test/unit-tests/components/views/messages/TextualBody-test.tsx +++ b/test/unit-tests/components/views/messages/TextualBody-test.tsx @@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details. */ import React from "react"; -import { MatrixClient, MatrixEvent } from "matrix-js-sdk/src/matrix"; +import { MatrixClient, MatrixEvent, PushRuleKind } from "matrix-js-sdk/src/matrix"; import { mocked, MockedObject } from "jest-mock"; import { render, waitFor } from "jest-matrix-react"; @@ -228,6 +228,23 @@ describe("", () => { const content = container.querySelector(".mx_EventTile_body"); expect(content.innerHTML.replace(defaultEvent.getId(), "%event_id%")).toMatchSnapshot(); }); + + it("should pillify a keyword responsible for triggering a notification", () => { + const ev = mkRoomTextMessage("foo bar baz"); + ev.setPushDetails(undefined, { + actions: [], + pattern: "bar", + rule_id: "bar", + default: false, + enabled: true, + kind: PushRuleKind.ContentSpecific, + }); + const { container } = getComponent({ mxEvent: ev }); + const content = container.querySelector(".mx_EventTile_body"); + expect(content.innerHTML).toMatchInlineSnapshot( + `"foo bar baz"`, + ); + }); }); describe("renders formatted m.text correctly", () => { From 16ad8146c5853231644a0252a6a6330c92c527a6 Mon Sep 17 00:00:00 2001 From: taffyko Date: Tue, 22 Oct 2024 13:48:10 -0700 Subject: [PATCH 2/4] Use createRoot instead of deprecated ReactDOM.render I foresee this change being made across the codebase shortly and want to proactively prevent my PR from falling behind --- src/components/views/messages/TextualBody.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/views/messages/TextualBody.tsx b/src/components/views/messages/TextualBody.tsx index 23e3fa89da8..a5d52abde17 100644 --- a/src/components/views/messages/TextualBody.tsx +++ b/src/components/views/messages/TextualBody.tsx @@ -8,6 +8,7 @@ Please see LICENSE files in the repository root for full details. import React, { createRef, SyntheticEvent, MouseEvent } from "react"; import ReactDOM from "react-dom"; +import { createRoot } from "react-dom/client"; import { MsgType, PushRuleKind } from "matrix-js-sdk/src/matrix"; import { TooltipProvider } from "@vector-im/compound-web"; import { globToRegexp } from "matrix-js-sdk/src/utils"; @@ -255,7 +256,7 @@ export default class TextualBody extends React.Component { {after} ); - ReactDOM.render(newContent, container); + createRoot(container).render(newContent); node.parentNode?.replaceChild(container, node); } else if (node.childNodes && node.childNodes.length) { From 05bd8a244036ba830d980624c26df9e50057f5d9 Mon Sep 17 00:00:00 2001 From: taffyko Date: Wed, 23 Oct 2024 10:11:27 -0700 Subject: [PATCH 3/4] Clean up react root on unmount --- src/components/views/messages/TextualBody.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/components/views/messages/TextualBody.tsx b/src/components/views/messages/TextualBody.tsx index a5d52abde17..79f5a506156 100644 --- a/src/components/views/messages/TextualBody.tsx +++ b/src/components/views/messages/TextualBody.tsx @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. import React, { createRef, SyntheticEvent, MouseEvent } from "react"; import ReactDOM from "react-dom"; -import { createRoot } from "react-dom/client"; +import { createRoot, Root } from "react-dom/client"; import { MsgType, PushRuleKind } from "matrix-js-sdk/src/matrix"; import { TooltipProvider } from "@vector-im/compound-web"; import { globToRegexp } from "matrix-js-sdk/src/utils"; @@ -53,7 +53,7 @@ export default class TextualBody extends React.Component { private pills: Element[] = []; private tooltips: Element[] = []; - private reactRoots: Element[] = []; + private reactRoots: Array = []; private ref = createRef(); @@ -149,7 +149,11 @@ export default class TextualBody extends React.Component { unmountTooltips(this.tooltips); for (const root of this.reactRoots) { - ReactDOM.unmountComponentAtNode(root); + if (root instanceof Element) { + ReactDOM.unmountComponentAtNode(root); + } else { + setTimeout(() => root.unmount()); + } } this.pills = []; @@ -256,7 +260,9 @@ export default class TextualBody extends React.Component { {after} ); - createRoot(container).render(newContent); + const containerRoot = createRoot(container); + containerRoot.render(newContent); + this.reactRoots.push(containerRoot); node.parentNode?.replaceChild(container, node); } else if (node.childNodes && node.childNodes.length) { From 6aa1b2e7ef2283dabb96b21a38a21b34b3fda31c Mon Sep 17 00:00:00 2001 From: taffyko Date: Wed, 23 Oct 2024 11:14:03 -0700 Subject: [PATCH 4/4] Remove addition of left-edge highlight on message mentions It is clear that it would be best for me to address this piece in a separate PR. --- res/css/views/rooms/_EventTile.pcss | 8 -------- res/themes/dark/css/_dark.pcss | 1 - res/themes/legacy-dark/css/_legacy-dark.pcss | 1 - res/themes/legacy-light/css/_legacy-light.pcss | 1 - res/themes/light-custom/css/_custom.pcss | 1 - res/themes/light/css/_light.pcss | 1 - 6 files changed, 13 deletions(-) diff --git a/res/css/views/rooms/_EventTile.pcss b/res/css/views/rooms/_EventTile.pcss index 09b9fe4f427..56a2c57f340 100644 --- a/res/css/views/rooms/_EventTile.pcss +++ b/res/css/views/rooms/_EventTile.pcss @@ -578,14 +578,6 @@ $left-gutter: 64px; padding-inline-start: calc(var(--EventTile_group_line-spacing-inline-start) + 20px); } } - - &.mx_EventTile_highlight, - &.mx_EventTile_highlight .markdown-body { - > .mx_EventTile_line { - box-shadow: inset var(--EventTile-box-shadow-offset-x) 0 0 var(--EventTile-box-shadow-spread-radius) - $event-highlight-edge-color; - } - } } &[data-layout="bubble"] { diff --git a/res/themes/dark/css/_dark.pcss b/res/themes/dark/css/_dark.pcss index c83d9419b9b..56630763fef 100644 --- a/res/themes/dark/css/_dark.pcss +++ b/res/themes/dark/css/_dark.pcss @@ -83,7 +83,6 @@ $spacePanel-bg-color: rgba(38, 39, 43, 0.82); $panel-gradient: rgba(34, 38, 46, 0), rgba(34, 38, 46, 1); $h3-color: $primary-content; $event-highlight-bg-color: #25271f; -$event-highlight-edge-color: #f0b132; $header-panel-text-primary-color: $text-secondary-color; /* ******************** */ diff --git a/res/themes/legacy-dark/css/_legacy-dark.pcss b/res/themes/legacy-dark/css/_legacy-dark.pcss index 41482a7353f..c6840f5b907 100644 --- a/res/themes/legacy-dark/css/_legacy-dark.pcss +++ b/res/themes/legacy-dark/css/_legacy-dark.pcss @@ -164,7 +164,6 @@ $widget-menu-bar-bg-color: $header-panel-bg-color; $widget-body-bg-color: #1a1d23; $event-highlight-bg-color: #25271f; -$event-highlight-edge-color: #f0b132; /* event timestamp */ $event-timestamp-color: $text-secondary-color; diff --git a/res/themes/legacy-light/css/_legacy-light.pcss b/res/themes/legacy-light/css/_legacy-light.pcss index e150aca0d55..398cf0e1f10 100644 --- a/res/themes/legacy-light/css/_legacy-light.pcss +++ b/res/themes/legacy-light/css/_legacy-light.pcss @@ -235,7 +235,6 @@ $widget-body-bg-color: #fff; $yellow-background: #fff8e3; $event-highlight-bg-color: $yellow-background; -$event-highlight-edge-color: var(--cpd-color-yellow-500); /* event timestamp */ $event-timestamp-color: #acacac; diff --git a/res/themes/light-custom/css/_custom.pcss b/res/themes/light-custom/css/_custom.pcss index 10b23c9b8da..0dfb38da98b 100644 --- a/res/themes/light-custom/css/_custom.pcss +++ b/res/themes/light-custom/css/_custom.pcss @@ -100,7 +100,6 @@ $button-danger-disabled-bg-color: var(--warning-color-50pct); /* still needs alp /* --timeline-highlights-color */ $event-selected-color: var(--timeline-highlights-color); $event-highlight-bg-color: var(--timeline-highlights-color); -$event-highlight-edge-color: var(--timeline-highlights-color); /* redirect some variables away from their hardcoded values in the light theme */ $settings-grey-fg-color: $primary-content; diff --git a/res/themes/light/css/_light.pcss b/res/themes/light/css/_light.pcss index 026ef2cc742..d649b6b38da 100644 --- a/res/themes/light/css/_light.pcss +++ b/res/themes/light/css/_light.pcss @@ -112,7 +112,6 @@ $spacePanel-bg-color: rgba(232, 232, 232, 0.77); $panel-gradient: rgba(242, 245, 248, 0), rgba(242, 245, 248, 1); $h3-color: #3d3b39; $event-highlight-bg-color: $yellow-background; -$event-highlight-edge-color: var(--cpd-color-yellow-500); $header-panel-text-primary-color: #91a1c0; /* ******************** */