Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 36 additions & 14 deletions apps/web/playwright/e2e/spaces/threads-activity-centre/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
* - Invite the bot to both rooms and ensure that it has joined
*/
export const test = base.extend<{
room1Name: string;
room1Name?: string;
room1: { name: string; roomId: string };
room2Name: string;
room2Name?: string;
room2: { name: string; roomId: string };
msg: MessageBuilder;
util: Helpers;
Expand All @@ -39,20 +39,20 @@
room1: async ({ room1Name: name, app, user, bot }, use) => {
const roomId = await app.client.createRoom({
name,
invite: [bot.credentials!.userId],
invite: [bot.credentials.userId],
preset: "public_chat" as Preset,
});
await bot.awaitRoomMembership(roomId);
await use({ name, roomId });
},
room2Name: "Room 2",
room2: async ({ room2Name: name, app, user, bot }, use) => {
const roomId = await app.client.createRoom({ name, invite: [bot.credentials!.userId] });
const roomId = await app.client.createRoom({ name, invite: [bot.credentials.userId] });
await bot.awaitRoomMembership(roomId);
await use({ name, roomId });
},
msg: async ({ page, app, util }, use) => {
await use(new MessageBuilder());
await use(new MessageBuilder(page, app, util));
},
util: async ({ room1, room2, page, app, bot }, use) => {
await use(new Helpers(page, app, bot));
Expand All @@ -69,7 +69,13 @@
* Provides utilities that build on the ability to find messages, e.g. replyTo,
* which finds a message and then constructs a reply to it.
*/
export class MessageBuilder {

Check warning on line 72 in apps/web/playwright/e2e/spaces/threads-activity-centre/index.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Mark these members as `readonly`.

See more on https://sonarcloud.io/project/issues?id=element-web&issues=AaBJWDMoZC2NEC2ODGYU&open=AaBJWDMoZC2NEC2ODGYU&pullRequest=32851
constructor(
private page: Page,
private app: ElementAppPage,
private helpers: Helpers,
) {}

/**
* Map of message content -> event.
*/
Expand Down Expand Up @@ -158,7 +164,11 @@
* MessageBuilder.replyTo} which creates a reply based on a previous message.
*/
export abstract class MessageContentSpec {
constructor(public readonly messageFinder: MessageBuilder) {}
messageFinder: MessageBuilder | null;

constructor(messageFinder: MessageBuilder = null) {
this.messageFinder = messageFinder;
}

public abstract getContent(room: JSHandle<Room>): Promise<Record<string, unknown>>;
}
Expand Down Expand Up @@ -220,9 +230,9 @@
await expect(this.page.locator(".mx_ThreadView_timelinePanelWrapper")).toBeVisible();
}

async findRoomById(roomId: string): Promise<JSHandle<Room>> {
async findRoomById(roomId: string): Promise<JSHandle<Room | undefined>> {
return this.app.client.evaluateHandle((cli, roomId) => {
return cli.getRooms().find((r) => r.roomId === roomId)!;
return cli.getRooms().find((r) => r.roomId === roomId);
}, roomId);
}

Expand Down Expand Up @@ -265,13 +275,20 @@
}

/**
* Click on a room in the Threads Activity Centre
* @param name - room name
* Click on a thread row in the Threads Activity Centre
* @param name - room or thread name
*/
clickRoomInTac(name: string) {
clickThreadInTac(name: string) {
return this.getTacPanel().getByRole("menuitem", { name }).click();
}

/**
* Switch to the "Other threads" tab in the TAC
*/
switchToOtherThreadsTab() {
return this.getTacPanel().getByRole("tab", { name: "Other threads" }).click();
}

/**
* Assert that the threads activity centre button has no indicator
*/
Expand All @@ -298,10 +315,15 @@
}

/**
* Assert that the threads activity centre panel has the expected rooms
* Assert that the threads activity centre panel has the expected thread rows
* @param content - the expected rooms and their notification levels
*/
async assertRoomsInTac(content: Array<{ room: string; notificationLevel: "highlight" | "notification" }>) {
async assertThreadsInTac(content: Array<{ room: string; notificationLevel: "highlight" | "notification" }>) {
const getBadgeClass = (notificationLevel: "highlight" | "notification") =>
notificationLevel === "highlight"
? "mx_NotificationBadge_level_highlight"
: "mx_NotificationBadge_level_notification";

// Ensure that we have the right number of rooms
await expect(this.getTacPanel().getByRole("menuitem")).toHaveCount(content.length);

Expand All @@ -312,7 +334,7 @@
// Ensure that the room name are correct
await expect(roomLocator).toHaveText(new RegExp(room));
// There is no accessibility marker for the StatelessNotificationBadge
await expect(roomLocator.locator(`[data-notification-level="${notificationLevel}"]`)).toBeVisible();
await expect(roomLocator.locator(`.${getBadgeClass(notificationLevel)}`)).toBeVisible();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,17 +95,18 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
});

test(
"should show the rooms with unread threads",
"should show threads from other users in the Other threads tab",
{ tag: "@screenshot" },
async ({ room1, room2, util, msg, user, app, page }) => {
await util.goTo(room2);
await util.populateThreads(room1, room2, msg, user);
// The indicator should be shown
await util.assertHighlightIndicator();

// Verify that we have the expected rooms in the TAC
// Bot-sent threads appear in "Other threads" tab
await util.openTac();
await util.assertRoomsInTac([
await util.switchToOtherThreadsTab();
await util.assertThreadsInTac([
{ room: room2.name, notificationLevel: "highlight" },
{ room: room1.name, notificationLevel: "notification" },
]);
Expand All @@ -122,19 +123,21 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
await util.goTo(room2);
await util.populateThreads(room1, room2, msg, user);

// Click on the first room in TAC
// Click on the first thread in TAC (switch to Other threads tab first)
await util.openTac();
await util.clickRoomInTac(room2.name);
await util.switchToOtherThreadsTab();
await util.clickThreadInTac(room2.name);

// Verify that the thread panel is opened after a click on the room in the TAC
// Verify that the thread panel is opened after a click on a thread in the TAC
await util.assertThreadPanelIsOpened();

// Open a thread and mark it as read
// The room 2 doesn't have a mention anymore in its unread, so the highest notification level is notification
await util.openThread("Msg1");
await util.assertNotificationTac();
await util.openTac();
await util.assertRoomsInTac([
await util.switchToOtherThreadsTab();
await util.assertThreadsInTac([
{ room: room1.name, notificationLevel: "notification" },
{ room: room2.name, notificationLevel: "notification" },
]);
Expand All @@ -147,7 +150,8 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
await util.populateThreads(room1, room2, msg, user, false);

await util.openTac();
await util.assertRoomsInTac([
await util.switchToOtherThreadsTab();
await util.assertThreadsInTac([
{ room: room1.name, notificationLevel: "notification" },
{ room: room2.name, notificationLevel: "notification" },
]);
Expand Down Expand Up @@ -183,8 +187,10 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {

await util.assertNotificationTac();

// Bot-sent thread appears in "Other threads" tab
await util.openTac();
await util.clickRoomInTac(room1.name);
await util.switchToOtherThreadsTab();
await util.clickThreadInTac(room1.name);

await util.clickMarkAllThreadsRead();

Expand All @@ -201,8 +207,10 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
}) => {
await util.receiveMessages(room1, ["Msg1", msg.threadedOff("Msg1", "Resp1")]);

// Bot-sent thread appears in "Other threads" tab
await util.openTac();
await util.clickRoomInTac(room1.name);
await util.switchToOtherThreadsTab();
await util.clickThreadInTac(room1.name);

await util.assertThreadPanelIsOpened();
});
Expand Down
108 changes: 85 additions & 23 deletions apps/web/res/css/structures/_ThreadsActivityCentre.pcss

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using !important to override compound styles, we should update the NavBar component to match the figma designs

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!important removed, I'm not confident to update NavBar in compound. Maybe with guidance.

Original file line number Diff line number Diff line change
Expand Up @@ -70,33 +70,95 @@
}
}

.mx_ThreadsActivityCentre_rows {
overflow-y: scroll;
/* Let some space at the top and the bottom of the pop-up */
max-height: calc(100vh - 200px);

.mx_ThreadsActivityCentreRow {
height: 48px;

/* Make the label of the MenuItem stay on one line and truncate with ellipsis if needed */
& > span {
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
/* Arbitrary size, keep the TAC as the wanted width */
width: 202px;
}
/* Compound styles its Menu/NavBar/MenuItem internals with single-class selectors. The
`[role="menu"]` qualifier gives our rules a higher specificity so they win without
`!important`. */
.mx_ThreadsActivityCentre_menu[role="menu"] {
/* Override compound's default 340px cap */
max-inline-size: 640px;
inline-size: 640px;
/* Override compound FloatingMenu's padding-block; we handle our own spacing via the
tabs + rows containers. */
padding-block: 0;

.mx_ThreadsActivityCentre_tabs {
/* Override NavBar's default block margin to match the Figma design */
margin: 0;
padding: var(--cpd-space-2x) var(--cpd-space-4x) 0;
}

/* Hide the notification badge on hover — compound's `nav-hint ~ *` rule would normally
* do this, but the app-web CSS layer overrides compound-web regardless of specificity. */
&:hover .mx_ThreadsActivityCentre_notificationBadge {
display: none;
.mx_ThreadsActivityCentre_rows {
overflow-y: auto;
/* Fixed panel height from the design (~6 rows), so the menu doesn't resize as the
list changes. Row heights vary with the preview, and 432px isn't on the spacing
scale, so this stays in px. */
height: 432px;
min-height: 432px;

.mx_ThreadsActivityCentreThreadRow {
/* Floor for rows without a message preview, which would otherwise be ~48px */
min-height: var(--cpd-space-14x);

/* Override compound's display: grid with a simple left-to-right flex row:
[avatar] [gap] [text content] [indicator] */
display: flex;
align-items: center;
gap: var(--cpd-space-3x);
padding: var(--cpd-space-3x) var(--cpd-space-4x);

/* Compound's .icon class adds margin-inline-end; zero it since gap handles spacing */
& > .mx_DecoratedRoomAvatar {
margin-inline-end: 0;
flex-shrink: 0;
}

/* Notification dot at the trailing edge */
& > .mx_NotificationBadge {
flex-shrink: 0;
align-self: center;
}

.mx_ThreadsActivityCentreThreadRow_content {
flex: 1;
min-width: 0;
display: flex;
flex-direction: column;
gap: var(--cpd-space-0-5x);
text-align: start;
}

.mx_ThreadsActivityCentreThreadRow_header {
display: flex;
align-items: center;
gap: var(--cpd-space-2x);
}

/* Font/letter-spacing come from the Compound Text components; only layout + colour here. */
.mx_ThreadsActivityCentreThreadRow_roomName {
flex: 1;
min-width: 0;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}

.mx_ThreadsActivityCentreThreadRow_preview {
display: -webkit-box;
-webkit-line-clamp: 2;
-webkit-box-orient: vertical;
overflow: hidden;
color: var(--cpd-color-text-secondary);
}

.mx_ThreadsActivityCentreThreadRow_sender {
color: var(--cpd-color-text-primary);
}
}
}
}

.mx_ThreadsActivityCentre_emptyCaption {
padding-left: 16px;
padding-right: 16px;
font-size: 13px;
/* Typography comes from the Compound Text component. */
padding: var(--cpd-space-3x) var(--cpd-space-4x);
color: var(--cpd-color-text-secondary);
}
2 changes: 1 addition & 1 deletion apps/web/src/Unread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export function doesRoomHaveUnreadMessages(room: Room, includeThreads: boolean):
return false;
}

function doesTimelineHaveUnreadMessages(room: Room, timeline: Array<MatrixEvent>): boolean {
export function doesTimelineHaveUnreadMessages(room: Room, timeline: Array<MatrixEvent>): boolean {
// The room is a space, let's ignore it
if (room.isSpaceRoom()) return false;

Expand Down
Loading
Loading