Focus the thread panel when clicking on an item in the TAC (#12410)

* Focus the thread panel when clicking on an item in the TAC

actually the 'close' button in the threads panel as it's the only
interactive element: we can improve this later when we use landmarks
& generally have better a11y.

* Undo minor refactoring

as none of it is test3ed, it's not worth it.

* add unit test

* Add matrixchat tests

* Needs awaits

* ts-ignore

* Fix test (I think...)

* Remove unnecessary value set

* Not how assignments work
t3chguy/dedup-icons-17oct
David Baker 2024-04-10 14:13:08 +01:00 committed by GitHub
parent 0daf0cfa80
commit 59395abb6b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 136 additions and 11 deletions

View File

@ -336,6 +336,15 @@ export class Helpers {
return expect(this.page.locator(".mx_ThreadPanel")).toBeVisible();
}
/**
* Assert that the thread panel is focused (actually the 'close' button, specifically)
*/
assertThreadPanelFocused() {
return expect(
this.page.locator(".mx_ThreadPanel").locator(".mx_BaseCard_header").getByTitle("Close"),
).toBeFocused();
}
/**
* Populate the rooms with messages and threads
* @param room1

View File

@ -160,4 +160,18 @@ test.describe("Threads Activity Centre", () => {
await util.assertNoTacIndicator();
});
test("should focus the thread panel close button when clicking an item in the TAC", async ({
room1,
room2,
util,
msg,
}) => {
await util.receiveMessages(room1, ["Msg1", msg.threadedOff("Msg1", "Resp1")]);
await util.openTac();
await util.clickRoomInTac(room1.name);
await util.assertThreadPanelFocused();
});
});

View File

@ -116,7 +116,7 @@ import { ButtonEvent } from "../views/elements/AccessibleButton";
import { ActionPayload } from "../../dispatcher/payloads";
import { SummarizedNotificationState } from "../../stores/notifications/SummarizedNotificationState";
import Views from "../../Views";
import { ViewRoomPayload } from "../../dispatcher/payloads/ViewRoomPayload";
import { FocusNextType, ViewRoomPayload } from "../../dispatcher/payloads/ViewRoomPayload";
import { ViewHomePagePayload } from "../../dispatcher/payloads/ViewHomePagePayload";
import { AfterLeaveRoomPayload } from "../../dispatcher/payloads/AfterLeaveRoomPayload";
import { DoAfterSyncPreparedPayload } from "../../dispatcher/payloads/DoAfterSyncPreparedPayload";
@ -229,7 +229,8 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
private screenAfterLogin?: IScreen;
private tokenLogin?: boolean;
private focusComposer: boolean;
// What to focus on next component update, if anything
private focusNext: FocusNextType;
private subTitleStatus: string;
private prevWindowWidth: number;
private voiceBroadcastResumer?: VoiceBroadcastResumer;
@ -298,8 +299,6 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
this.themeWatcher.start();
this.fontWatcher.start();
this.focusComposer = false;
// object field used for tracking the status info appended to the title tag.
// we don't do it as react state as i'm scared about triggering needless react refreshes.
this.subTitleStatus = "";
@ -483,9 +482,11 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
PosthogTrackers.instance.trackPageChange(this.state.view, this.state.page_type, durationMs);
}
}
if (this.focusComposer) {
if (this.focusNext === "composer") {
dis.fire(Action.FocusSendMessageComposer);
this.focusComposer = false;
this.focusNext = undefined;
} else if (this.focusNext === "threadsPanel") {
dis.fire(Action.FocusThreadsPanel);
}
}
@ -985,7 +986,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
// switch view to the given room
private async viewRoom(roomInfo: ViewRoomPayload): Promise<void> {
this.focusComposer = true;
this.focusNext = roomInfo.focusNext ?? "composer";
if (roomInfo.room_alias) {
logger.log(`Switching to room alias ${roomInfo.room_alias} at event ${roomInfo.event_id}`);

View File

@ -1268,7 +1268,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
case Action.FocusAComposer: {
dis.dispatch<FocusComposerPayload>({
...(payload as FocusComposerPayload),
// re-dispatch to the correct composer
// re-dispatch to the correct composer (the send message will still be on screen even when editing a message)
action: this.state.editState ? Action.FocusEditMessageComposer : Action.FocusSendMessageComposer,
});
break;

View File

@ -37,6 +37,9 @@ import { ButtonEvent } from "../views/elements/AccessibleButton";
import Spinner from "../views/elements/Spinner";
import Heading from "../views/typography/Heading";
import { clearRoomNotification } from "../../utils/notifications";
import { useDispatcher } from "../../hooks/useDispatcher";
import dis from "../../dispatcher/dispatcher";
import { Action } from "../../dispatcher/actions";
interface IProps {
roomId: string;
@ -229,6 +232,7 @@ const ThreadPanel: React.FC<IProps> = ({ roomId, onClose, permalinkCreator }) =>
const roomContext = useContext(RoomContext);
const timelinePanel = useRef<TimelinePanel | null>(null);
const card = useRef<HTMLDivElement | null>(null);
const closeButonRef = useRef<HTMLDivElement | null>(null);
const [filterOption, setFilterOption] = useState<ThreadFilterType>(ThreadFilterType.All);
const [room, setRoom] = useState<Room | null>(null);
@ -255,6 +259,14 @@ const ThreadPanel: React.FC<IProps> = ({ roomId, onClose, permalinkCreator }) =>
}
}, [timelineSet, timelinePanel]);
useDispatcher(dis, (payload) => {
// This actually foucses the close button on the threads panel, as its the only interactive element,
// but at least it puts the user in the right area of the app.
if (payload.action === Action.FocusThreadsPanel) {
closeButonRef.current?.focus();
}
});
return (
<RoomContext.Provider
value={{
@ -276,6 +288,7 @@ const ThreadPanel: React.FC<IProps> = ({ roomId, onClose, permalinkCreator }) =>
onClose={onClose}
withoutScrollContainer={true}
ref={card}
closeButtonRef={closeButonRef}
>
{card.current && <Measured sensor={card.current} onMeasurement={setNarrow} />}
{timelineSet ? (

View File

@ -35,6 +35,8 @@ interface IProps {
onKeyDown?(ev: KeyboardEvent): void;
cardState?: any;
ref?: Ref<HTMLDivElement>;
// Ref for the 'close' button the the card
closeButtonRef?: Ref<HTMLDivElement>;
children: ReactNode;
}
@ -54,7 +56,21 @@ export const Group: React.FC<IGroupProps> = ({ className, title, children }) =>
};
const BaseCard: React.FC<IProps> = forwardRef<HTMLDivElement, IProps>(
({ closeLabel, onClose, onBack, className, header, footer, withoutScrollContainer, children, onKeyDown }, ref) => {
(
{
closeLabel,
onClose,
onBack,
className,
header,
footer,
withoutScrollContainer,
children,
onKeyDown,
closeButtonRef,
},
ref,
) => {
let backButton;
const cardHistory = RightPanelStore.instance.roomPhaseHistory;
if (cardHistory.length > 1) {
@ -75,6 +91,7 @@ const BaseCard: React.FC<IProps> = forwardRef<HTMLDivElement, IProps>(
className="mx_BaseCard_close"
onClick={onClose}
title={closeLabel || _t("action|close")}
ref={closeButtonRef}
/>
);
}

View File

@ -159,6 +159,7 @@ function ThreadsActivityCentreRow({ room, onClick, notificationLevel }: ThreadsA
show_room_tile: true, // make sure the room is visible in the list
room_id: room.roomId,
metricsTrigger: "WebThreadsActivityCentre",
focusNext: "threadsPanel",
});
}}
label={room.name}

View File

@ -27,7 +27,7 @@ import { notificationLevelToIndicator } from "../../../../utils/notifications";
interface ThreadsActivityCentreButtonProps extends ComponentProps<typeof IconButton> {
/**
* Display the `Treads` label next to the icon.
* Display the `Threads` label next to the icon.
*/
displayLabel?: boolean;
/**

View File

@ -91,6 +91,11 @@ export enum Action {
*/
FocusAComposer = "focus_a_composer",
/**
* Focuses the threads panel.
*/
FocusThreadsPanel = "focus_threads_panel",
/**
* Opens the user menu (previously known as the top left menu). No additional payload information required.
*/

View File

@ -24,6 +24,8 @@ import { IOpts } from "../../createRoom";
import { JoinRoomPayload } from "./JoinRoomPayload";
import { AtLeastOne } from "../../@types/common";
export type FocusNextType = "composer" | "threadsPanel" | undefined;
/* eslint-disable camelcase */
interface BaseViewRoomPayload extends Pick<ActionPayload, "action"> {
action: Action.ViewRoom;
@ -61,5 +63,6 @@ export type ViewRoomPayload = BaseViewRoomPayload &
// the number of API calls required.
room_id?: string;
room_alias?: string;
focusNext: FocusNextType; // wat to focus after room switch. Defaults to 'composer' if undefined.
}>;
/* eslint-enable camelcase */

View File

@ -15,7 +15,7 @@ limitations under the License.
*/
import React, { ComponentProps } from "react";
import { fireEvent, render, RenderResult, screen, within } from "@testing-library/react";
import { fireEvent, render, RenderResult, screen, waitFor, within } from "@testing-library/react";
import fetchMock from "fetch-mock-jest";
import { Mocked, mocked } from "jest-mock";
import { ClientEvent, MatrixClient, MatrixEvent, Room, SyncState } from "matrix-js-sdk/src/matrix";
@ -59,6 +59,7 @@ import { SSO_HOMESERVER_URL_KEY, SSO_ID_SERVER_URL_KEY } from "../../../src/Base
import SettingsStore from "../../../src/settings/SettingsStore";
import { SettingLevel } from "../../../src/settings/SettingLevel";
import { MatrixClientPeg as peg } from "../../../src/MatrixClientPeg";
import DMRoomMap from "../../../src/utils/DMRoomMap";
jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({
completeAuthorizationCodeGrant: jest.fn(),
@ -220,6 +221,9 @@ describe("<MatrixChat />", () => {
jest.spyOn(StorageManager, "idbLoad").mockReset();
jest.spyOn(StorageManager, "idbSave").mockResolvedValue(undefined);
jest.spyOn(defaultDispatcher, "dispatch").mockClear();
jest.spyOn(defaultDispatcher, "fire").mockClear();
DMRoomMap.makeShared(mockClient);
await clearAllModals();
});
@ -227,6 +231,9 @@ describe("<MatrixChat />", () => {
resetJsDomAfterEach();
afterEach(() => {
// @ts-ignore
DMRoomMap.setShared(null);
jest.restoreAllMocks();
// emit a loggedOut event so that all of the Store singletons forget about their references to the mock client
@ -239,6 +246,22 @@ describe("<MatrixChat />", () => {
expect(container).toMatchSnapshot();
});
it("should fire to focus the message composer", async () => {
getComponent();
defaultDispatcher.dispatch({ action: Action.ViewRoom, room_id: "!room:server.org", focusNext: "composer" });
await waitFor(() => {
expect(defaultDispatcher.fire).toHaveBeenCalledWith(Action.FocusSendMessageComposer);
});
});
it("should fire to focus the threads panel", async () => {
getComponent();
defaultDispatcher.dispatch({ action: Action.ViewRoom, room_id: "!room:server.org", focusNext: "threadsPanel" });
await waitFor(() => {
expect(defaultDispatcher.fire).toHaveBeenCalledWith(Action.FocusThreadsPanel);
});
});
describe("when query params have a OIDC params", () => {
const issuer = "https://auth.com/";
const homeserverUrl = "https://matrix.org";

View File

@ -37,6 +37,8 @@ import ResizeNotifier from "../../../src/utils/ResizeNotifier";
import { createTestClient, getRoomContext, mkRoom, mockPlatformPeg, stubClient } from "../../test-utils";
import { mkThread } from "../../test-utils/threads";
import { IRoomState } from "../../../src/components/structures/RoomView";
import defaultDispatcher from "../../../src/dispatcher/dispatcher";
import { Action } from "../../../src/dispatcher/actions";
jest.mock("../../../src/utils/Feedback");
@ -156,6 +158,43 @@ describe("ThreadPanel", () => {
fireEvent.click(getByRole(container, "button", { name: "Mark all as read" }));
await waitFor(() => expect(mockClient.sendReadReceipt).not.toHaveBeenCalled());
});
it("focuses the close button on FocusThreadsPanel dispatch", () => {
const ROOM_ID = "!roomId:example.org";
stubClient();
mockPlatformPeg();
const mockClient = mocked(MatrixClientPeg.safeGet());
const room = new Room(ROOM_ID, mockClient, mockClient.getUserId() ?? "", {
pendingEventOrdering: PendingEventOrdering.Detached,
});
render(
<MatrixClientContext.Provider value={mockClient}>
<RoomContext.Provider
value={getRoomContext(room, {
canSendMessages: true,
})}
>
<ThreadPanel
roomId={ROOM_ID}
onClose={jest.fn()}
resizeNotifier={new ResizeNotifier()}
permalinkCreator={new RoomPermalinkCreator(room)}
/>
</RoomContext.Provider>
</MatrixClientContext.Provider>,
);
// Unfocus it first so we know it's not just focused by coincidence
screen.getByTestId("base-card-close-button").blur();
expect(screen.getByTestId("base-card-close-button")).not.toHaveFocus();
defaultDispatcher.dispatch({ action: Action.FocusThreadsPanel }, true);
expect(screen.getByTestId("base-card-close-button")).toHaveFocus();
});
});
describe("Filtering", () => {