Skip to content

Commit

Permalink
Get upstream changes via verji#develop (#13)
Browse files Browse the repository at this point in the history
* Update `@vector-im/compound-design-tokens` in package.json (matrix-org#12339)

* Change 'type' prop on badges to 'forceDot' (matrix-org#12327)

* Change 'type' prop on badges tio 'forceDot'

Which, hopefully, better represents what it actually does. Tidies
up some of the logic.

Split out from matrix-org#12254

* Missed a file

* More comments

* Oops, there is no count here.

* Back out the logic refactor of StatelessNotificationBadge

because it was also updating the logic for mark as unread badges and
rewriting the ternary to the previous logic would be quite complex.

* Fix doc comment

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Clarify doc on displaying the count

* Update doc for the forceDot param here too.

---------

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* [Backport staging] Update `@vector-im/compound-design-tokens` in package.json (matrix-org#12340)

(cherry picked from commit e3ba643)

Co-authored-by: Florian Duros <florianduros@element.io>

* Fix the image view (matrix-org#12341)

* TAC: Fix hover state when expanded (matrix-org#12337)

* Fix TAC hover state

* Add playwright test

* Update playwright snapshot after last compound style changes

* v3.95.0-rc.0

* v3.95.0

* Reset matrix-js-sdk back to develop branch

* TAC: Order rooms by most recent after notification level (matrix-org#12329)

* Order room by thread timestamp

* Fix key errors in test

* Update jest snapshots

* Update snapshots

* Rename alpha/beta to numbers

* Add playwright test

* Replace forceCount prop with hideIfDot (matrix-org#12344)

This replaces the `forceCount` prop on room badge components with
`hideIfDot` which hopefully gives a better idea of what it does,
since forceCount did not really force a count. Also remove the
prop where it was just passing the default value anyway.

---------

Co-authored-by: Florian Duros <florianduros@element.io>
Co-authored-by: David Baker <dbkr@users.noreply.github.com>
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Co-authored-by: ElementRobot <releases@riot.im>
Co-authored-by: Robin <robin@robin.town>
  • Loading branch information
6 people authored Mar 15, 2024
1 parent 9f39f10 commit de3a9bc
Show file tree
Hide file tree
Showing 26 changed files with 404 additions and 93 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
Changes in [3.95.0](https://github.com/matrix-org/matrix-react-sdk/releases/tag/v3.95.0) (2024-03-14)
=====================================================================================================
## 🐛 Bug Fixes

* Update `@vector-im/compound-design-tokens` in package.json ([#12340](https://github.com/matrix-org/matrix-react-sdk/pull/12340)).

Changes in [3.94.0](https://github.com/matrix-org/matrix-react-sdk/releases/tag/v3.94.0) (2024-03-12)
=====================================================================================================
## ✨ Features
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "matrix-react-sdk",
"version": "3.94.0",
"version": "3.95.0",
"description": "SDK for matrix.org using React",
"author": "matrix.org",
"repository": {
Expand Down Expand Up @@ -75,7 +75,7 @@
"@matrix-org/spec": "^1.7.0",
"@sentry/browser": "^7.0.0",
"@testing-library/react-hooks": "^8.0.1",
"@vector-im/compound-design-tokens": "^1.0.0",
"@vector-im/compound-design-tokens": "^1.2.0",
"@vector-im/compound-web": "^3.1.1",
"@zxcvbn-ts/core": "^3.0.4",
"@zxcvbn-ts/language-common": "^3.0.4",
Expand Down
51 changes: 31 additions & 20 deletions playwright/e2e/spaces/threads-activity-centre/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,30 @@ import { ElementAppPage } from "../../../pages/ElementAppPage";
* - Invite the bot to both rooms and ensure that it has joined
*/
export const test = base.extend<{
roomAlphaName?: string;
roomAlpha: { name: string; roomId: string };
roomBetaName?: string;
roomBeta: { name: string; roomId: string };
room1Name?: string;
room1: { name: string; roomId: string };
room2Name?: string;
room2: { name: string; roomId: string };
msg: MessageBuilder;
util: Helpers;
}>({
displayName: "Mae",
botCreateOpts: { displayName: "Other User" },

roomAlphaName: "Room Alpha",
roomAlpha: async ({ roomAlphaName: name, app, user, bot }, use) => {
room1Name: "Room 1",
room1: async ({ room1Name: name, app, user, bot }, use) => {
const roomId = await app.client.createRoom({ name, invite: [bot.credentials.userId] });
await use({ name, roomId });
},
roomBetaName: "Room Beta",
roomBeta: async ({ roomBetaName: name, app, user, bot }, use) => {
room2Name: "Room 2",
room2: async ({ room2Name: name, app, user, bot }, use) => {
const roomId = await app.client.createRoom({ name, invite: [bot.credentials.userId] });
await use({ name, roomId });
},
msg: async ({ page, app, util }, use) => {
await use(new MessageBuilder(page, app, util));
},
util: async ({ roomAlpha, roomBeta, page, app, bot }, use) => {
util: async ({ room1, room2, page, app, bot }, use) => {
await use(new Helpers(page, app, bot));
},
});
Expand Down Expand Up @@ -265,6 +265,13 @@ export class Helpers {
return this.getTacButton().click();
}

/**
* Hover over the Threads Activity Centre button
*/
hoverTacButton() {
return this.getTacButton().hover();
}

/**
* Click on a room in the Threads Activity Centre
* @param name - room name
Expand Down Expand Up @@ -330,23 +337,27 @@ export class Helpers {
* @param room1
* @param room2
* @param msg - MessageBuilder
* @param hasMention - whether to include a mention in the first message
*/
async populateThreads(
room1: { name: string; roomId: string },
room2: { name: string; roomId: string },
msg: MessageBuilder,
hasMention = true,
) {
await this.receiveMessages(room2, [
"Msg1",
msg.threadedOff("Msg1", {
"body": "User",
"format": "org.matrix.custom.html",
"formatted_body": "<a href='https://matrix.to/#/@user:localhost'>User</a>",
"m.mentions": {
user_ids: ["@user:localhost"],
},
}),
]);
if (hasMention) {
await this.receiveMessages(room2, [
"Msg1",
msg.threadedOff("Msg1", {
"body": "User",
"format": "org.matrix.custom.html",
"formatted_body": "<a href='https://matrix.to/#/@user:localhost'>User</a>",
"m.mentions": {
user_ids: ["@user:localhost"],
},
}),
]);
}
await this.receiveMessages(room2, ["Msg2", msg.threadedOff("Msg2", "Resp2")]);
await this.receiveMessages(room1, ["Msg3", msg.threadedOff("Msg3", "Resp3")]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ test.describe("Threads Activity Centre", () => {
await expect(util.getSpacePanel()).toMatchScreenshot("tac-button-expanded.png");
});

test("should not show indicator when there is no thread", async ({ roomAlpha: room1, util }) => {
test("should not show indicator when there is no thread", async ({ room1, util }) => {
// No indicator should be shown
await util.assertNoTacIndicator();

Expand All @@ -46,23 +46,15 @@ test.describe("Threads Activity Centre", () => {
await util.assertNoTacIndicator();
});

test("should show a notification indicator when there is a message in a thread", async ({
roomAlpha: room1,
util,
msg,
}) => {
test("should show a notification indicator when there is a message in a thread", async ({ room1, util, msg }) => {
await util.goTo(room1);
await util.receiveMessages(room1, ["Msg1", msg.threadedOff("Msg1", "Resp1")]);

// The indicator should be shown
await util.assertNotificationTac();
});

test("should show a highlight indicator when there is a mention in a thread", async ({
roomAlpha: room1,
util,
msg,
}) => {
test("should show a highlight indicator when there is a mention in a thread", async ({ room1, util, msg }) => {
await util.goTo(room1);
await util.receiveMessages(room1, [
"Msg1",
Expand All @@ -80,7 +72,7 @@ test.describe("Threads Activity Centre", () => {
await util.assertHighlightIndicator();
});

test("should show the rooms with unread threads", async ({ roomAlpha: room1, roomBeta: room2, util, msg }) => {
test("should show the rooms with unread threads", async ({ room1, room2, util, msg }) => {
await util.goTo(room2);
await util.populateThreads(room1, room2, msg);
// The indicator should be shown
Expand All @@ -97,7 +89,7 @@ test.describe("Threads Activity Centre", () => {
await expect(util.getTacPanel()).toMatchScreenshot("tac-panel-mix-unread.png");
});

test("should update with a thread is read", async ({ roomAlpha: room1, roomBeta: room2, util, msg }) => {
test("should update with a thread is read", async ({ room1, room2, util, msg }) => {
await util.goTo(room2);
await util.populateThreads(room1, room2, msg);

Expand All @@ -120,6 +112,17 @@ test.describe("Threads Activity Centre", () => {
await expect(util.getTacPanel()).toMatchScreenshot("tac-panel-notification-unread.png");
});

test("should order by recency after notification level", async ({ room1, room2, util, msg }) => {
await util.goTo(room2);
await util.populateThreads(room1, room2, msg, false);

await util.openTac();
await util.assertRoomsInTac([
{ room: room1.name, notificationLevel: "notification" },
{ room: room2.name, notificationLevel: "notification" },
]);
});

test("should block the Spotlight to open when the TAC is opened", async ({ util, page }) => {
const toggleSpotlight = () => page.keyboard.press(`${CommandOrControl}+k`);

Expand All @@ -134,4 +137,14 @@ test.describe("Threads Activity Centre", () => {
await toggleSpotlight();
await expect(page.locator(".mx_SpotlightDialog")).not.toBeVisible();
});

test("should have the correct hover state", async ({ util, page }) => {
await util.hoverTacButton();
await expect(util.getSpacePanel()).toMatchScreenshot("tac-hovered.png");

// Expand the space panel, hover the button and take a screenshot
await util.expandSpacePanel();
await util.hoverTacButton();
await expect(util.getSpacePanel()).toMatchScreenshot("tac-hovered-expanded.png");
});
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 6 additions & 0 deletions res/css/_common.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,12 @@ legend {
max-height: calc(100% - var(--cpd-space-12x));
display: flex;
flex-direction: column;

.mx_Dialog_lightbox & {
/* The lightbox isn't so much of a dialog as a fullscreen overlay. We
don't want the glass border. */
display: contents;
}
}

.mx_Dialog {
Expand Down
6 changes: 6 additions & 0 deletions res/css/structures/_ThreadsActivityCentre.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
margin: 18px auto auto auto;

&.expanded {
/**
* override compound default background color when hovered
* should disappear when the space panel will be migrated to compound
*/
background-color: transparent !important;

/* align with settings icon */
margin-left: 21px;

Expand Down
10 changes: 7 additions & 3 deletions src/components/views/avatars/DecoratedRoomAvatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ interface IProps {
room: Room;
size: string;
displayBadge?: boolean;
forceCount?: boolean;
/**
* If true, show nothing if the notification would only cause a dot to be shown rather than
* a badge. That is: only display badges and not dots. Default: false.
*/
hideIfDot?: boolean;
oobData?: IOOBData;
viewAvatarOnClick?: boolean;
tooltipProps?: {
Expand Down Expand Up @@ -178,14 +182,14 @@ export default class DecoratedRoomAvatar extends React.PureComponent<IProps, ISt

public render(): React.ReactNode {
// Spread the remaining props to make it work with compound component
const { room, size, displayBadge, forceCount, oobData, viewAvatarOnClick, tooltipProps, ...props } = this.props;
const { room, size, displayBadge, hideIfDot, oobData, viewAvatarOnClick, tooltipProps, ...props } = this.props;

let badge: React.ReactNode;
if (this.props.displayBadge && this.state.notificationState) {
badge = (
<NotificationBadge
notification={this.state.notificationState}
forceCount={this.props.forceCount}
hideIfDot={this.props.hideIfDot}
roomId={this.props.room.roomId}
/>
);
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
<UnreadNotificationBadge
room={room || undefined}
threadId={this.props.mxEvent.getId()}
type="dot"
forceDot={true}
/>
</div>
{isRenderingNotification && room ? (
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/rooms/ExtraTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export default function ExtraTile({

let badge: JSX.Element | null = null;
if (notificationState) {
badge = <NotificationBadge notification={notificationState} forceCount={false} />;
badge = <NotificationBadge notification={notificationState} />;
}

let name = displayName;
Expand Down
15 changes: 8 additions & 7 deletions src/components/views/rooms/NotificationBadge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ interface IProps {
notification: NotificationState;

/**
* If true, the badge will show a count if at all possible. This is typically
* used to override the user's preference for things like room sublists.
* If true, show nothing if the notification would only cause a dot to be shown rather than
* a badge. That is: only display badges and not dots. Default: false.
*/
forceCount?: boolean;
hideIfDot?: boolean;

/**
* The room ID, if any, the badge represents.
Expand All @@ -48,7 +48,7 @@ interface IClickableProps extends IProps, React.InputHTMLAttributes<Element> {
}

interface IState {
showCounts: boolean; // whether to show counts. Independent of props.forceCount
showCounts: boolean; // whether to show counts.
}

export default class NotificationBadge extends React.PureComponent<XOR<IProps, IClickableProps>, IState> {
Expand Down Expand Up @@ -97,11 +97,12 @@ export default class NotificationBadge extends React.PureComponent<XOR<IProps, I

public render(): ReactNode {
/* eslint @typescript-eslint/no-unused-vars: ["error", { "ignoreRestSiblings": true }] */
const { notification, showUnsentTooltip, forceCount, onClick, tabIndex } = this.props;
const { notification, showUnsentTooltip, hideIfDot, onClick, tabIndex } = this.props;

if (notification.isIdle && !notification.knocked) return null;
if (forceCount) {
if (!notification.hasUnreadCount) return null; // Can't render a badge
if (hideIfDot && notification.level < NotificationLevel.Notification) {
// This would just be a dot and we've been told not to show dots, so don't show it
if (!notification.hasUnreadCount) return null;
}

const commonProps: React.ComponentProps<typeof StatelessNotificationBadge> = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,12 @@ interface Props {
count: number;
level: NotificationLevel;
knocked?: boolean;
type?: "badge" | "dot";
/**
* If true, where we would normally show a badge, we instead show a dot. No numeric count will
* be displayed (but may affect whether the the dot is displayed). See class doc
* for the difference between the two.
*/
forceDot?: boolean;
}

interface ClickableProps extends Props {
Expand All @@ -39,8 +44,17 @@ interface ClickableProps extends Props {
tabIndex?: number;
}

/**
* A notification indicator that conveys what activity / notifications the user has in whatever
* context it is being used.
*
* Can either be a 'badge': a small circle with a number in it (the 'count'), or a 'dot': a smaller, empty circle.
* The two can be used to convey the same meaning but in different contexts, for example: for unread
* notifications in the room list, it may have a green badge with the number of unread notifications,
* but somewhere else it may just have a green dot as a more compact representation of the same information.
*/
export const StatelessNotificationBadge = forwardRef<HTMLDivElement, XOR<Props, ClickableProps>>(
({ symbol, count, level, knocked, type = "badge", ...props }, ref) => {
({ symbol, count, level, knocked, forceDot = false, ...props }, ref) => {
const hideBold = useSettingValue("feature_hidebold");

// Don't show a badge if we don't need to
Expand All @@ -61,10 +75,12 @@ export const StatelessNotificationBadge = forwardRef<HTMLDivElement, XOR<Props,
mx_NotificationBadge_visible: isEmptyBadge || knocked ? true : hasUnreadCount,
mx_NotificationBadge_level_notification: level == NotificationLevel.Notification,
mx_NotificationBadge_level_highlight: level >= NotificationLevel.Highlight,
mx_NotificationBadge_dot: (isEmptyBadge && !knocked) || type === "dot",
mx_NotificationBadge_knocked: knocked,
mx_NotificationBadge_2char: type === "badge" && symbol && symbol.length > 0 && symbol.length < 3,
mx_NotificationBadge_3char: type === "badge" && symbol && symbol.length > 2,

// At most one of mx_NotificationBadge_dot, mx_NotificationBadge_2char, mx_NotificationBadge_3char
mx_NotificationBadge_dot: (isEmptyBadge && !knocked) || forceDot,
mx_NotificationBadge_2char: !forceDot && symbol && symbol.length > 0 && symbol.length < 3,
mx_NotificationBadge_3char: !forceDot && symbol && symbol.length > 2,
});

if (props.onClick) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,15 @@ import { StatelessNotificationBadge } from "./StatelessNotificationBadge";
interface Props {
room?: Room;
threadId?: string;
type?: "badge" | "dot";
/**
* If true, where we would normally show a badge, we instead show a dot. No numeric count will
* be displayed.
*/
forceDot?: boolean;
}

export function UnreadNotificationBadge({ room, threadId, type }: Props): JSX.Element {
export function UnreadNotificationBadge({ room, threadId, forceDot }: Props): JSX.Element {
const { symbol, count, level } = useUnreadNotifications(room, threadId);

return <StatelessNotificationBadge symbol={symbol} count={count} level={level} type={type} />;
return <StatelessNotificationBadge symbol={symbol} count={count} level={level} forceDot={forceDot} />;
}
2 changes: 1 addition & 1 deletion src/components/views/rooms/RoomBreadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const RoomBreadcrumbTile: React.FC<{ room: Room; onClick: (ev: ButtonEvent) => v
room={room}
size="32px"
displayBadge={true}
forceCount={true}
hideIfDot={true}
tooltipProps={{ tabIndex: isActive ? 0 : -1 }}
/>
</AccessibleTooltipButton>
Expand Down
Loading

0 comments on commit de3a9bc

Please sign in to comment.