From 2434749f659f926b3e2add55496b645b1fe06892 Mon Sep 17 00:00:00 2001 From: Kerry Date: Wed, 5 Apr 2023 14:46:17 +1200 Subject: [PATCH] Highlight event when any version triggered a highlight (#10502) * check previous events pushactions when deciding to highlight * test event highlight * strict fix * highlight edited text to red --- res/css/views/rooms/_EventTile.pcss | 3 +- src/components/views/rooms/EventTile.tsx | 23 +++- .../components/views/rooms/EventTile-test.tsx | 104 +++++++++++++++++- 3 files changed, 121 insertions(+), 9 deletions(-) diff --git a/res/css/views/rooms/_EventTile.pcss b/res/css/views/rooms/_EventTile.pcss index 979f3f5ba42..ef75fc154bd 100644 --- a/res/css/views/rooms/_EventTile.pcss +++ b/res/css/views/rooms/_EventTile.pcss @@ -132,7 +132,8 @@ $left-gutter: 64px; } &.mx_EventTile_highlight, - &.mx_EventTile_highlight .markdown-body { + &.mx_EventTile_highlight .markdown-body, + &.mx_EventTile_highlight .mx_EventTile_edited { color: $alert; } diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index f9e3f25b97e..6bbafd59582 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -653,15 +653,26 @@ export class UnwrappedEventTile extends React.Component return true; } + /** + * Determine whether an event should be highlighted + * For edited events, if a previous version of the event was highlighted + * the event should remain highlighted as the user may have been notified + * (Clearer explanation of why an event is highlighted is planned - + * https://github.com/vector-im/element-web/issues/24927) + * @returns boolean + */ private shouldHighlight(): boolean { if (this.props.forExport) return false; if (this.context.timelineRenderingType === TimelineRenderingType.Notification) return false; if (this.context.timelineRenderingType === TimelineRenderingType.ThreadsList) return false; - const actions = MatrixClientPeg.get().getPushActionsForEvent( - this.props.mxEvent.replacingEvent() || this.props.mxEvent, - ); - if (!actions || !actions.tweaks) { + const cli = MatrixClientPeg.get(); + const actions = cli.getPushActionsForEvent(this.props.mxEvent.replacingEvent() || this.props.mxEvent); + // get the actions for the previous version of the event too if it is an edit + const previousActions = this.props.mxEvent.replacingEvent() + ? cli.getPushActionsForEvent(this.props.mxEvent) + : undefined; + if (!actions?.tweaks && !previousActions?.tweaks) { return false; } @@ -670,13 +681,13 @@ export class UnwrappedEventTile extends React.Component return false; } - return actions.tweaks.highlight; + return !!(actions?.tweaks.highlight || previousActions?.tweaks.highlight); } private onSenderProfileClick = (): void => { dis.dispatch({ action: Action.ComposerInsert, - userId: this.props.mxEvent.getSender(), + userId: this.props.mxEvent.getSender()!, timelineRenderingType: this.context.timelineRenderingType, }); }; diff --git a/test/components/views/rooms/EventTile-test.tsx b/test/components/views/rooms/EventTile-test.tsx index 27884a333b8..abbb3f70b49 100644 --- a/test/components/views/rooms/EventTile-test.tsx +++ b/test/components/views/rooms/EventTile-test.tsx @@ -15,14 +15,16 @@ limitations under the License. */ import * as React from "react"; +import { render, waitFor, screen, act, fireEvent } from "@testing-library/react"; +import { mocked } from "jest-mock"; import { EventType } from "matrix-js-sdk/src/@types/event"; import { MatrixClient, PendingEventOrdering } from "matrix-js-sdk/src/client"; +import { TweakName } from "matrix-js-sdk/src/matrix"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { NotificationCountType, Room } from "matrix-js-sdk/src/models/room"; import { DeviceTrustLevel, UserTrustLevel } from "matrix-js-sdk/src/crypto/CrossSigning"; import { DeviceInfo } from "matrix-js-sdk/src/crypto/deviceinfo"; import { IEncryptedEventInfo } from "matrix-js-sdk/src/crypto/api"; -import { render, waitFor, screen, act, fireEvent } from "@testing-library/react"; import EventTile, { EventTileProps } from "../../../../src/components/views/rooms/EventTile"; import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; @@ -73,7 +75,7 @@ describe("EventTile", () => { stubClient(); client = MatrixClientPeg.get(); - room = new Room(ROOM_ID, client, client.getUserId()!, { + room = new Room(ROOM_ID, client, client.getSafeUserId(), { pendingEventOrdering: PendingEventOrdering.Detached, }); @@ -373,4 +375,102 @@ describe("EventTile", () => { ); }); }); + + describe("event highlighting", () => { + const isHighlighted = (container: HTMLElement): boolean => + !!container.getElementsByClassName("mx_EventTile_highlight").length; + + beforeEach(() => { + mocked(client.getPushActionsForEvent).mockReturnValue(null); + }); + + it("does not highlight message where message matches no push actions", () => { + const { container } = getComponent(); + + expect(client.getPushActionsForEvent).toHaveBeenCalledWith(mxEvent); + expect(isHighlighted(container)).toBeFalsy(); + }); + + it(`does not highlight when message's push actions does not have a highlight tweak`, () => { + mocked(client.getPushActionsForEvent).mockReturnValue({ notify: true, tweaks: {} }); + const { container } = getComponent(); + + expect(isHighlighted(container)).toBeFalsy(); + }); + + it(`highlights when message's push actions have a highlight tweak`, () => { + mocked(client.getPushActionsForEvent).mockReturnValue({ + notify: true, + tweaks: { [TweakName.Highlight]: true }, + }); + const { container } = getComponent(); + + expect(isHighlighted(container)).toBeTruthy(); + }); + + describe("when a message has been edited", () => { + let editingEvent: MatrixEvent; + + beforeEach(() => { + editingEvent = new MatrixEvent({ + type: "m.room.message", + room_id: ROOM_ID, + sender: "@alice:example.org", + content: { + "msgtype": "m.text", + "body": "* edited body", + "m.new_content": { + msgtype: "m.text", + body: "edited body", + }, + "m.relates_to": { + rel_type: "m.replace", + event_id: mxEvent.getId(), + }, + }, + }); + mxEvent.makeReplaced(editingEvent); + }); + + it("does not highlight message where no version of message matches any push actions", () => { + const { container } = getComponent(); + + // get push actions for both events + expect(client.getPushActionsForEvent).toHaveBeenCalledWith(mxEvent); + expect(client.getPushActionsForEvent).toHaveBeenCalledWith(editingEvent); + expect(isHighlighted(container)).toBeFalsy(); + }); + + it(`does not highlight when no version of message's push actions have a highlight tweak`, () => { + mocked(client.getPushActionsForEvent).mockReturnValue({ notify: true, tweaks: {} }); + const { container } = getComponent(); + + expect(isHighlighted(container)).toBeFalsy(); + }); + + it(`highlights when previous version of message's push actions have a highlight tweak`, () => { + mocked(client.getPushActionsForEvent).mockImplementation((event: MatrixEvent) => { + if (event === mxEvent) { + return { notify: true, tweaks: { [TweakName.Highlight]: true } }; + } + return { notify: false, tweaks: {} }; + }); + const { container } = getComponent(); + + expect(isHighlighted(container)).toBeTruthy(); + }); + + it(`highlights when new version of message's push actions have a highlight tweak`, () => { + mocked(client.getPushActionsForEvent).mockImplementation((event: MatrixEvent) => { + if (event === editingEvent) { + return { notify: true, tweaks: { [TweakName.Highlight]: true } }; + } + return { notify: false, tweaks: {} }; + }); + const { container } = getComponent(); + + expect(isHighlighted(container)).toBeTruthy(); + }); + }); + }); });