From 90419bdffd6d284059e5d9e6c1ab1c1588c56cec Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 7 Nov 2023 15:44:30 +0530 Subject: [PATCH] Implement new unreachable state and fix broken string ref (#11748) * Fix string ref issue * Implement unreachable state * Fix eslint failure * Fix i18n * Fix i18n again * Write cypress test * Write jest test * Write more jest tests * Update method name * Use unstable prefix * Always use prefix This is never to going to be in the spec so always use the io.element prefix * Update tests * Remove redundant code from cypress test * Use unstable prefix * Refactor code * Remove supressOnHover prop * Remove sub-text label * Join lines * Remove blank line * Add jsdoc --- cypress/e2e/presence/presence.spec.ts | 64 +++++++ res/css/views/rooms/_EntityTile.pcss | 8 +- .../views/dialogs/ForwardDialog.tsx | 3 +- src/components/views/rooms/EntityTile.tsx | 74 +++---- src/components/views/rooms/MemberList.tsx | 20 +- src/components/views/rooms/PresenceLabel.tsx | 2 + src/i18n/strings/en_EN.json | 3 +- .../views/rooms/MemberList-test.tsx | 180 ++++++++++-------- .../views/rooms/PresenceLabel-test.tsx | 13 ++ 9 files changed, 234 insertions(+), 133 deletions(-) create mode 100644 cypress/e2e/presence/presence.spec.ts diff --git a/cypress/e2e/presence/presence.spec.ts b/cypress/e2e/presence/presence.spec.ts new file mode 100644 index 0000000000..12a3228b4c --- /dev/null +++ b/cypress/e2e/presence/presence.spec.ts @@ -0,0 +1,64 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/// +import { HomeserverInstance } from "../../plugins/utils/homeserver"; + +describe("Presence tests", () => { + let homeserver: HomeserverInstance; + + beforeEach(() => { + cy.startHomeserver("default").then((data) => { + homeserver = data; + }); + }); + + afterEach(() => { + cy.stopHomeserver(homeserver); + }); + + it("renders unreachable presence state correctly", () => { + cy.initTestUser(homeserver, "Janet"); + cy.getBot(homeserver, { displayName: "Bob" }).then((bob) => { + cy.intercept("GET", "**/sync*", (req) => { + req.continue((res) => { + res.body.presence = { + events: [ + { + type: "m.presence", + sender: bob.getUserId(), + content: { + presence: "io.element.unreachable", + currently_active: false, + }, + }, + ], + }; + }); + }); + cy.createRoom({ name: "My Room", invite: [bob.getUserId()] }).then((roomId) => { + cy.viewRoomById(roomId); + }); + cy.findByRole("button", { name: "Room info" }).click(); + cy.get(".mx_RightPanel").within(() => { + cy.contains("People").click(); + }); + cy.get(".mx_EntityTile_unreachable") + .should("contain.text", "Bob") + .should("contain.text", "User's server unreachable"); + }); + }); +}); diff --git a/res/css/views/rooms/_EntityTile.pcss b/res/css/views/rooms/_EntityTile.pcss index 9632946bd5..1df4ff3dcc 100644 --- a/res/css/views/rooms/_EntityTile.pcss +++ b/res/css/views/rooms/_EntityTile.pcss @@ -46,11 +46,11 @@ limitations under the License. background-color: $header-panel-text-primary-color; } -.mx_EntityTile .mx_PresenceLabel { +.mx_EntityTile:not(.mx_EntityTile_unreachable) .mx_PresenceLabel { display: none; } -.mx_EntityTile:not(.mx_EntityTile_noHover):hover .mx_PresenceLabel { +.mx_EntityTile:hover .mx_PresenceLabel { display: block; } @@ -106,7 +106,9 @@ limitations under the License. } .mx_EntityTile_unknown .mx_EntityTile_avatar, -.mx_EntityTile_unknown .mx_EntityTile_name { +.mx_EntityTile_unknown .mx_EntityTile_name, +.mx_EntityTile_unreachable .mx_EntityTile_avatar, +.mx_EntityTile_unreachable .mx_EntityTile_name { opacity: 0.25; } diff --git a/src/components/views/dialogs/ForwardDialog.tsx b/src/components/views/dialogs/ForwardDialog.tsx index 2ed75714ec..a21acd7b71 100644 --- a/src/components/views/dialogs/ForwardDialog.tsx +++ b/src/components/views/dialogs/ForwardDialog.tsx @@ -264,8 +264,7 @@ const ForwardDialog: React.FC = ({ matrixClient: cli, event, permalinkCr } name={text} - presenceState="online" - suppressOnHover={true} + showPresence={false} onClick={() => setTruncateAt(totalCount)} /> ); diff --git a/src/components/views/rooms/EntityTile.tsx b/src/components/views/rooms/EntityTile.tsx index f20df88c32..cfb579b11c 100644 --- a/src/components/views/rooms/EntityTile.tsx +++ b/src/components/views/rooms/EntityTile.tsx @@ -35,12 +35,13 @@ const PowerLabel: Record = { [PowerStatus.Moderator]: _td("power_level|mod"), }; -export type PresenceState = "offline" | "online" | "unavailable"; +export type PresenceState = "offline" | "online" | "unavailable" | "io.element.unreachable"; const PRESENCE_CLASS: Record = { - offline: "mx_EntityTile_offline", - online: "mx_EntityTile_online", - unavailable: "mx_EntityTile_unavailable", + "offline": "mx_EntityTile_offline", + "online": "mx_EntityTile_online", + "unavailable": "mx_EntityTile_unavailable", + "io.element.unreachable": "mx_EntityTile_unreachable", }; function presenceClassForMember(presenceState?: PresenceState, lastActiveAgo?: number, showPresence?: boolean): string { @@ -75,7 +76,6 @@ interface IProps { presenceCurrentlyActive?: boolean; showInviteButton: boolean; onClick(): void; - suppressOnHover: boolean; showPresence: boolean; subtextLabel?: string; e2eStatus?: E2EState; @@ -93,7 +93,6 @@ export default class EntityTile extends React.PureComponent { presenceLastActiveAgo: 0, presenceLastTs: 0, showInviteButton: false, - suppressOnHover: false, showPresence: true, }; @@ -105,10 +104,27 @@ export default class EntityTile extends React.PureComponent { }; } + /** + * Creates the PresenceLabel component if needed + * @returns The PresenceLabel component if we need to render it, undefined otherwise + */ + private getPresenceLabel(): JSX.Element | undefined { + if (!this.props.showPresence) return; + const activeAgo = this.props.presenceLastActiveAgo + ? Date.now() - (this.props.presenceLastTs - this.props.presenceLastActiveAgo) + : -1; + return ( + + ); + } + public render(): React.ReactNode { const mainClassNames: Record = { mx_EntityTile: true, - mx_EntityTile_noHover: !!this.props.suppressOnHover, }; if (this.props.className) mainClassNames[this.props.className] = true; @@ -119,43 +135,13 @@ export default class EntityTile extends React.PureComponent { ); mainClassNames[presenceClass] = true; - let nameEl; const name = this.props.nameJSX || this.props.name; - - if (!this.props.suppressOnHover) { - const activeAgo = this.props.presenceLastActiveAgo - ? Date.now() - (this.props.presenceLastTs - this.props.presenceLastActiveAgo) - : -1; - - let presenceLabel: JSX.Element | undefined; - if (this.props.showPresence) { - presenceLabel = ( - - ); - } - if (this.props.subtextLabel) { - presenceLabel = {this.props.subtextLabel}; - } - nameEl = ( -
-
{name}
- {presenceLabel} -
- ); - } else if (this.props.subtextLabel) { - nameEl = ( -
-
{name}
- {this.props.subtextLabel} -
- ); - } else { - nameEl =
{name}
; - } + const nameAndPresence = ( +
+
{name}
+ {this.getPresenceLabel()} +
+ ); let inviteButton; if (this.props.showInviteButton) { @@ -198,7 +184,7 @@ export default class EntityTile extends React.PureComponent { {av} {e2eIcon} - {nameEl} + {nameAndPresence} {powerLabel} {inviteButton} diff --git a/src/components/views/rooms/MemberList.tsx b/src/components/views/rooms/MemberList.tsx index 0eb6f49c06..a0b9cc0f70 100644 --- a/src/components/views/rooms/MemberList.tsx +++ b/src/components/views/rooms/MemberList.tsx @@ -81,6 +81,7 @@ export default class MemberList extends React.Component { public static contextType = SDKContext; public context!: React.ContextType; + private tiles: Map = new Map(); public constructor(props: IProps, context: React.ContextType) { super(props); @@ -154,7 +155,7 @@ export default class MemberList extends React.Component { // Attach a SINGLE listener for global presence changes then locate the // member tile and re-render it. This is more efficient than every tile // ever attaching their own listener. - const tile = this.refs[user.userId]; + const tile = this.tiles.get(user.userId); if (tile) { this.updateList(); // reorder the membership list } @@ -245,8 +246,7 @@ export default class MemberList extends React.Component { } name={text} - presenceState="online" - suppressOnHover={true} + showPresence={false} onClick={onClick} /> ); @@ -307,14 +307,24 @@ export default class MemberList extends React.Component { return members.map((m) => { if (m instanceof RoomMember) { // Is a Matrix invite - return ; + return ( + { + if (tile) this.tiles.set(m.userId, tile); + else this.tiles.delete(m.userId); + }} + showPresence={this.showPresence} + /> + ); } else { // Is a 3pid invite return ( this.onPending3pidInviteClick(m)} /> ); diff --git a/src/components/views/rooms/PresenceLabel.tsx b/src/components/views/rooms/PresenceLabel.tsx index db5d0519ab..24e144c8ef 100644 --- a/src/components/views/rooms/PresenceLabel.tsx +++ b/src/components/views/rooms/PresenceLabel.tsx @@ -44,6 +44,8 @@ export default class PresenceLabel extends React.Component { // the 'active ago' ends up being 0. if (presence && BUSY_PRESENCE_NAME.matches(presence)) return _t("presence|busy"); + if (presence === "io.element.unreachable") return _t("presence|unreachable"); + if (!currentlyActive && activeAgo !== undefined && activeAgo > 0) { const duration = formatDuration(activeAgo); if (presence === "online") return _t("presence|online_for", { duration: duration }); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 2ff6dd3bd6..7ae2f7a70f 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1740,7 +1740,8 @@ "online": "Online", "online_for": "Online for %(duration)s", "unknown": "Unknown", - "unknown_for": "Unknown for %(duration)s" + "unknown_for": "Unknown for %(duration)s", + "unreachable": "User's server unreachable" }, "quick_settings": { "all_settings": "All settings", diff --git a/test/components/views/rooms/MemberList-test.tsx b/test/components/views/rooms/MemberList-test.tsx index 7b9ae19f4e..f9f591b373 100644 --- a/test/components/views/rooms/MemberList-test.tsx +++ b/test/components/views/rooms/MemberList-test.tsx @@ -16,8 +16,8 @@ limitations under the License. */ import React from "react"; -import { act, render, RenderResult } from "@testing-library/react"; -import { Room, MatrixClient, RoomState, RoomMember, User } from "matrix-js-sdk/src/matrix"; +import { act, render, RenderResult, screen } from "@testing-library/react"; +import { Room, MatrixClient, RoomState, RoomMember, User, MatrixEvent } from "matrix-js-sdk/src/matrix"; import { compare } from "matrix-js-sdk/src/utils"; import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; @@ -137,84 +137,88 @@ describe("MemberList", () => { } } + function renderMemberList(enablePresence: boolean): void { + TestUtils.stubClient(); + client = MatrixClientPeg.safeGet(); + client.hasLazyLoadMembersEnabled = () => false; + + // Make room + memberListRoom = createRoom(); + expect(memberListRoom.roomId).toBeTruthy(); + + // Make users + adminUsers = []; + moderatorUsers = []; + defaultUsers = []; + const usersPerLevel = 2; + for (let i = 0; i < usersPerLevel; i++) { + const adminUser = new RoomMember(memberListRoom.roomId, `@admin${i}:localhost`); + adminUser.membership = "join"; + adminUser.powerLevel = 100; + adminUser.user = User.createUser(adminUser.userId, client); + adminUser.user.currentlyActive = true; + adminUser.user.presence = "online"; + adminUser.user.lastPresenceTs = 1000; + adminUser.user.lastActiveAgo = 10; + adminUsers.push(adminUser); + + const moderatorUser = new RoomMember(memberListRoom.roomId, `@moderator${i}:localhost`); + moderatorUser.membership = "join"; + moderatorUser.powerLevel = 50; + moderatorUser.user = User.createUser(moderatorUser.userId, client); + moderatorUser.user.currentlyActive = true; + moderatorUser.user.presence = "online"; + moderatorUser.user.lastPresenceTs = 1000; + moderatorUser.user.lastActiveAgo = 10; + moderatorUsers.push(moderatorUser); + + const defaultUser = new RoomMember(memberListRoom.roomId, `@default${i}:localhost`); + defaultUser.membership = "join"; + defaultUser.powerLevel = 0; + defaultUser.user = User.createUser(defaultUser.userId, client); + defaultUser.user.currentlyActive = true; + defaultUser.user.presence = "online"; + defaultUser.user.lastPresenceTs = 1000; + defaultUser.user.lastActiveAgo = 10; + defaultUsers.push(defaultUser); + } + + client.getRoom = (roomId) => { + if (roomId === memberListRoom.roomId) return memberListRoom; + else return null; + }; + memberListRoom.currentState = { + members: {}, + getMember: jest.fn(), + getStateEvents: ((eventType, stateKey) => + stateKey === undefined ? [] : null) as RoomState["getStateEvents"], // ignore 3pid invites + } as unknown as RoomState; + for (const member of [...adminUsers, ...moderatorUsers, ...defaultUsers]) { + memberListRoom.currentState.members[member.userId] = member; + } + + const gatherWrappedRef = (r: MemberList) => { + memberList = r; + }; + const context = new TestSdkContext(); + context.client = client; + context.memberListStore.isPresenceEnabled = jest.fn().mockReturnValue(enablePresence); + root = render( + + + , + ); + } + describe.each([false, true])("does order members correctly (presence %s)", (enablePresence) => { beforeEach(function () { - TestUtils.stubClient(); - client = MatrixClientPeg.safeGet(); - client.hasLazyLoadMembersEnabled = () => false; - - // Make room - memberListRoom = createRoom(); - expect(memberListRoom.roomId).toBeTruthy(); - - // Make users - adminUsers = []; - moderatorUsers = []; - defaultUsers = []; - const usersPerLevel = 2; - for (let i = 0; i < usersPerLevel; i++) { - const adminUser = new RoomMember(memberListRoom.roomId, `@admin${i}:localhost`); - adminUser.membership = "join"; - adminUser.powerLevel = 100; - adminUser.user = new User(adminUser.userId); - adminUser.user.currentlyActive = true; - adminUser.user.presence = "online"; - adminUser.user.lastPresenceTs = 1000; - adminUser.user.lastActiveAgo = 10; - adminUsers.push(adminUser); - - const moderatorUser = new RoomMember(memberListRoom.roomId, `@moderator${i}:localhost`); - moderatorUser.membership = "join"; - moderatorUser.powerLevel = 50; - moderatorUser.user = new User(moderatorUser.userId); - moderatorUser.user.currentlyActive = true; - moderatorUser.user.presence = "online"; - moderatorUser.user.lastPresenceTs = 1000; - moderatorUser.user.lastActiveAgo = 10; - moderatorUsers.push(moderatorUser); - - const defaultUser = new RoomMember(memberListRoom.roomId, `@default${i}:localhost`); - defaultUser.membership = "join"; - defaultUser.powerLevel = 0; - defaultUser.user = new User(defaultUser.userId); - defaultUser.user.currentlyActive = true; - defaultUser.user.presence = "online"; - defaultUser.user.lastPresenceTs = 1000; - defaultUser.user.lastActiveAgo = 10; - defaultUsers.push(defaultUser); - } - - client.getRoom = (roomId) => { - if (roomId === memberListRoom.roomId) return memberListRoom; - else return null; - }; - memberListRoom.currentState = { - members: {}, - getMember: jest.fn(), - getStateEvents: ((eventType, stateKey) => - stateKey === undefined ? [] : null) as RoomState["getStateEvents"], // ignore 3pid invites - } as unknown as RoomState; - for (const member of [...adminUsers, ...moderatorUsers, ...defaultUsers]) { - memberListRoom.currentState.members[member.userId] = member; - } - - const gatherWrappedRef = (r: MemberList) => { - memberList = r; - }; - const context = new TestSdkContext(); - context.client = client; - context.memberListStore.isPresenceEnabled = jest.fn().mockReturnValue(enablePresence); - root = render( - - - , - ); + renderMemberList(enablePresence); }); describe("does order members correctly", () => { @@ -308,4 +312,24 @@ describe("MemberList", () => { }); }); }); + + describe("memberlist is rendered correctly", () => { + beforeEach(function () { + renderMemberList(true); + }); + + it("memberlist is re-rendered on unreachable presence event", async () => { + defaultUsers[0].user?.setPresenceEvent( + new MatrixEvent({ + type: "m.presence", + sender: defaultUsers[0].userId, + content: { + presence: "io.element.unreachable", + currently_active: false, + }, + }), + ); + expect(await screen.findByText(/User's server unreachable/)).toBeInTheDocument(); + }); + }); }); diff --git a/test/components/views/rooms/PresenceLabel-test.tsx b/test/components/views/rooms/PresenceLabel-test.tsx index 78ccb234c0..f080c6e05d 100644 --- a/test/components/views/rooms/PresenceLabel-test.tsx +++ b/test/components/views/rooms/PresenceLabel-test.tsx @@ -32,4 +32,17 @@ describe("", () => { `); }); + + it("should render 'Unreachable' for presence=unreachable", () => { + const { asFragment } = render(); + expect(asFragment()).toMatchInlineSnapshot(` + +
+ User's server unreachable +
+ + `); + }); });