Fix outstanding UX issues with replies/mentions/keyword notifs (#28270)

* Fix outstanding UX issues with replies/mentions/keyword notifs * 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 * Clean up react root on unmount * 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. * Update call to ReactRootManager.render --------- Co-authored-by: Michael Telatynski <[email protected]>
element-hq · Jan 22, 2025 · 68c03db · 68c03db
1 parent 9a109cd
commit 68c03db
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 12 deletions.
diff --git 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
@@ -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;

diff --git 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<PillProps> = ({ type: propType, url, inMessage, room, shouldShowPillAvatar = true }) => {
-    const { event, member, onClick, resourceId, targetRoom, text, type } = usePermalink({
+export const Pill: React.FC<PillProps> = ({
+    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<PillProps> = ({ 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<PillProps> = ({ type: propType, url, inMessage, room
         case PillType.UserMention:
             avatar = <PillMemberAvatar shouldShowPillAvatar={shouldShowPillAvatar} member={member} />;
             break;
+        case PillType.Keyword:
+            break;
         default:
             return null;
     }

diff --git a/src/components/views/messages/TextualBody.tsx b/src/components/views/messages/TextualBody.tsx
@@ -7,8 +7,9 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import React, { createRef, SyntheticEvent, MouseEvent, StrictMode } from "react";
-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";
@@ -35,6 +36,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";
 import { ReactRootManager } from "../../../utils/react";
 
 interface IState {
@@ -100,6 +102,16 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {
                 }
             }
         }
+
+        // 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 {
@@ -210,6 +222,55 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {
         }
     }
 
+    /**
+     * Marks the text that activated a push-notification keyword pattern.
+     */
+    private pillifyNotificationKeywords(nodes: ArrayLike<Element>, 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}
+                        <TooltipProvider>
+                            <Pill text={keywordText} type={PillType.Keyword} />
+                        </TooltipProvider>
+                        {after}
+                    </>
+                );
+                this.reactRoots.render(newContent, container, node);
+
+                node.parentNode?.replaceChild(container, node);
+            } else if (node.childNodes && node.childNodes.length) {
+                this.pillifyNotificationKeywords(node.childNodes as NodeListOf<Element>, 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<Element>): 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
@@ -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("<TextualBody />", () => {
             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(
+                `"<span>foo <bdi><span tabindex="0"><span class="mx_Pill mx_KeywordPill"><span class="mx_Pill_text">bar</span></span></span></bdi> baz</span>"`,
+            );
+        });
     });
 
     describe("renders formatted m.text correctly", () => {