Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Disable jump to read receipt button instead of hiding when nothing to jump to #12863

Merged
merged 5 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
62 changes: 34 additions & 28 deletions src/components/views/right_panel/UserInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import { KnownMembership } from "matrix-js-sdk/src/types";
import { UserVerificationStatus, VerificationRequest } from "matrix-js-sdk/src/crypto-api";
import { logger } from "matrix-js-sdk/src/logger";
import { CryptoEvent } from "matrix-js-sdk/src/crypto";
import { Heading, MenuItem, Text } from "@vector-im/compound-web";
import { Heading, MenuItem, Text, Tooltip } from "@vector-im/compound-web";
import ChatIcon from "@vector-im/compound-design-tokens/assets/web/icons/chat";
import CheckIcon from "@vector-im/compound-design-tokens/assets/web/icons/check";
import ShareIcon from "@vector-im/compound-design-tokens/assets/web/icons/share";
Expand Down Expand Up @@ -442,19 +442,40 @@ export const UserOptionsSection: React.FC<{
// Only allow the user to ignore the user if its not ourselves
// same goes for jumping to read receipt
if (!isMe) {
if (member instanceof RoomMember && member.roomId && !isSpace) {
const onReadReceiptButton = function (): void {
const room = cli.getRoom(member.roomId);
dis.dispatch<ViewRoomPayload>({
action: Action.ViewRoom,
highlighted: true,
// this could return null, the default prevents a type error
event_id: room?.getEventReadUpTo(member.userId) || undefined,
room_id: member.roomId,
metricsTrigger: undefined, // room doesn't change
});
};
const onReadReceiptButton = function (room: Room): void {
dis.dispatch<ViewRoomPayload>({
action: Action.ViewRoom,
highlighted: true,
// this could return null, the default prevents a type error
event_id: room.getEventReadUpTo(member.userId) || undefined,
room_id: room.roomId,
metricsTrigger: undefined, // room doesn't change
});
};

const room = member instanceof RoomMember ? cli.getRoom(member.roomId) : null;
const readReceiptButtonDisabled = isSpace || !room?.getEventReadUpTo(member.userId);
readReceiptButton = (
<MenuItem
role="button"
onSelect={async (ev) => {
ev.preventDefault();
if (room && !readReceiptButtonDisabled) {
onReadReceiptButton(room);
}
}}
label={_t("user_info|jump_to_rr_button")}
disabled={readReceiptButtonDisabled}
Icon={CheckIcon}
/>
);
if (readReceiptButtonDisabled) {
readReceiptButton = (
<Tooltip label={_t("user_info|jump_to_rr_button_disabled")}>{readReceiptButton}</Tooltip>
);
}

if (member instanceof RoomMember && member.roomId && !isSpace) {
const onInsertPillButton = function (): void {
dis.dispatch<ComposerInsertPayload>({
action: Action.ComposerInsert,
Expand All @@ -463,21 +484,6 @@ export const UserOptionsSection: React.FC<{
});
};

const room = member instanceof RoomMember ? cli.getRoom(member.roomId) : undefined;
if (room?.getEventReadUpTo(member.userId)) {
readReceiptButton = (
<MenuItem
role="button"
onSelect={async (ev) => {
ev.preventDefault();
onReadReceiptButton();
}}
label={_t("user_info|jump_to_rr_button")}
Icon={CheckIcon}
/>
);
}

insertPillButton = (
<MenuItem
role="button"
Expand Down
1 change: 1 addition & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -3782,6 +3782,7 @@
"ignore_confirm_title": "Ignore %(user)s",
"invited_by": "Invited by %(sender)s",
"jump_to_rr_button": "Jump to read receipt",
"jump_to_rr_button_disabled": "This feature is disabled",
Copy link
Member

Choose a reason for hiding this comment

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

This feels a bit overly unhelpful. Conditional that shows either, "Not available in spaces" or, "No receipts for ${user}" depending on isSpace?

Copy link
Member Author

@t3chguy t3chguy Aug 6, 2024

Choose a reason for hiding this comment

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

The tooltip is now gone (forgot to push), but this would have been as specified by design. See element-hq/element-web#27883 (comment)

"kick_button_room": "Remove from room",
"kick_button_room_name": "Remove from %(roomName)s",
"kick_button_space": "Remove from space",
Expand Down
8 changes: 4 additions & 4 deletions test/components/views/right_panel/UserInfo-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -927,19 +927,19 @@ describe("<UserOptionsSection />", () => {
});
});

it("when call to client.getRoom is null, does not show read receipt button", () => {
it("when call to client.getRoom is null, shows disabled read receipt button", () => {
mockClient.getRoom.mockReturnValueOnce(null);
renderComponent();

expect(screen.queryByRole("button", { name: "Jump to read receipt" })).not.toBeInTheDocument();
expect(screen.queryByRole("button", { name: "Jump to read receipt" })).toBeDisabled();
});

it("when call to client.getRoom is non-null and room.getEventReadUpTo is null, does not show read receipt button", () => {
it("when call to client.getRoom is non-null and room.getEventReadUpTo is null, shows disabled read receipt button", () => {
mockRoom.getEventReadUpTo.mockReturnValueOnce(null);
mockClient.getRoom.mockReturnValueOnce(mockRoom);
renderComponent();

expect(screen.queryByRole("button", { name: "Jump to read receipt" })).not.toBeInTheDocument();
expect(screen.queryByRole("button", { name: "Jump to read receipt" })).toBeDisabled();
});

it("when calls to client.getRoom and room.getEventReadUpTo are non-null, shows read receipt button", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,44 @@ exports[`<UserInfo /> with crypto enabled renders <BasicUserInfo /> 1`] = `
/>
</svg>
</button>
<button
class="_item_1gwvj_17 _interactive_1gwvj_36 _disabled_1gwvj_119"
data-kind="primary"
disabled=""
role="button"
>
<svg
aria-hidden="true"
class="_icon_1gwvj_44"
fill="currentColor"
height="24"
viewBox="0 0 24 24"
width="24"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M9.55 17.575c-.133 0-.258-.02-.375-.063a.878.878 0 0 1-.325-.212L4.55 13c-.183-.183-.27-.42-.263-.713.009-.291.105-.529.288-.712a.948.948 0 0 1 .7-.275.95.95 0 0 1 .7.275L9.55 15.15l8.475-8.475c.183-.183.42-.275.712-.275s.53.092.713.275c.183.183.275.42.275.712s-.092.53-.275.713l-9.2 9.2c-.1.1-.208.17-.325.212a1.106 1.106 0 0 1-.375.063Z"
/>
</svg>
<span
class="_typography_yh5dq_162 _font-body-md-medium_yh5dq_69 _label_1gwvj_53"
>
Jump to read receipt
</span>
<svg
aria-hidden="true"
class="_nav-hint_1gwvj_60"
fill="currentColor"
height="24"
viewBox="8 0 8 24"
width="8"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M8.7 17.3a.948.948 0 0 1-.275-.7.95.95 0 0 1 .275-.7l3.9-3.9-3.9-3.9a.948.948 0 0 1-.275-.7.95.95 0 0 1 .275-.7.948.948 0 0 1 .7-.275.95.95 0 0 1 .7.275l4.6 4.6c.1.1.17.208.213.325.041.117.062.242.062.375s-.02.258-.063.375a.876.876 0 0 1-.212.325l-4.6 4.6a.948.948 0 0 1-.7.275.948.948 0 0 1-.7-.275Z"
/>
</svg>
</button>
<button
class="_item_1gwvj_17 _interactive_1gwvj_36"
data-kind="primary"
Expand Down Expand Up @@ -525,6 +563,44 @@ exports[`<UserInfo /> with crypto enabled should render a deactivate button for
/>
</svg>
</button>
<button
class="_item_1gwvj_17 _interactive_1gwvj_36 _disabled_1gwvj_119"
data-kind="primary"
disabled=""
role="button"
>
<svg
aria-hidden="true"
class="_icon_1gwvj_44"
fill="currentColor"
height="24"
viewBox="0 0 24 24"
width="24"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M9.55 17.575c-.133 0-.258-.02-.375-.063a.878.878 0 0 1-.325-.212L4.55 13c-.183-.183-.27-.42-.263-.713.009-.291.105-.529.288-.712a.948.948 0 0 1 .7-.275.95.95 0 0 1 .7.275L9.55 15.15l8.475-8.475c.183-.183.42-.275.712-.275s.53.092.713.275c.183.183.275.42.275.712s-.092.53-.275.713l-9.2 9.2c-.1.1-.208.17-.325.212a1.106 1.106 0 0 1-.375.063Z"
/>
</svg>
<span
class="_typography_yh5dq_162 _font-body-md-medium_yh5dq_69 _label_1gwvj_53"
>
Jump to read receipt
</span>
<svg
aria-hidden="true"
class="_nav-hint_1gwvj_60"
fill="currentColor"
height="24"
viewBox="8 0 8 24"
width="8"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M8.7 17.3a.948.948 0 0 1-.275-.7.95.95 0 0 1 .275-.7l3.9-3.9-3.9-3.9a.948.948 0 0 1-.275-.7.95.95 0 0 1 .275-.7.948.948 0 0 1 .7-.275.95.95 0 0 1 .7.275l4.6 4.6c.1.1.17.208.213.325.041.117.062.242.062.375s-.02.258-.063.375a.876.876 0 0 1-.212.325l-4.6 4.6a.948.948 0 0 1-.7.275.948.948 0 0 1-.7-.275Z"
/>
</svg>
</button>
<button
class="_item_1gwvj_17 _interactive_1gwvj_36"
data-kind="primary"
Expand Down
Loading