From bf558dbc185356046a50c4bdcfd9b1fa5c5ff6ea Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 15 May 2026 13:40:56 +0100 Subject: [PATCH] Bunch of refactors --- .../playwright/sample-files/upload-module.js | 9 +- .../src/components/structures/RoomView.tsx | 8 +- .../views/right_panel/TimelineCard.tsx | 81 ++++++++--------- .../views/rooms/EditMessageComposer.tsx | 2 +- .../views/rooms/SendMessageComposer.tsx | 24 +++-- .../hooks/useWysiwygSendActionHandler.ts | 22 ++--- apps/web/src/dispatcher/actions.ts | 5 ++ .../payloads/ComposerInsertFilePayload.ts | 16 ++++ .../payloads/ComposerInsertPayload.ts | 11 +-- apps/web/src/modules/ComposerApi.ts | 23 +++-- .../viewmodels/room/RoomUploadViewModel.tsx | 88 +++++++++++++------ .../unit-tests/modules/ComposerApi-test.ts | 8 +- .../module-api/element-web-module-api.api.md | 13 ++- packages/module-api/package.json | 1 + packages/module-api/project.json | 2 +- packages/module-api/src/api/composer.ts | 26 ++++-- .../UploadButton/UploadButton.test.tsx | 9 +- .../__snapshots__/UploadButton.test.tsx.snap | 6 +- 18 files changed, 216 insertions(+), 138 deletions(-) create mode 100644 apps/web/src/dispatcher/payloads/ComposerInsertFilePayload.ts diff --git a/apps/web/playwright/sample-files/upload-module.js b/apps/web/playwright/sample-files/upload-module.js index 8d072c8cf9..11edfbca95 100644 --- a/apps/web/playwright/sample-files/upload-module.js +++ b/apps/web/playwright/sample-files/upload-module.js @@ -24,10 +24,11 @@ export default class CustomComponentModule { this.api.composer.addFileUploadOption({ type: "org.example.uploader", label: "Example uploader", - onSelected: () => { - this.api.composer.openFileUploadConfirmation([ - new File(["test"], "testfile.txt", { type: "text/plain" }), - ]); + onSelected: (_roomId, view) => { + this.api.composer.openFileUploadConfirmation( + [new File(["test"], "testfile.txt", { type: "text/plain" })], + view, + ); }, }); } diff --git a/apps/web/src/components/structures/RoomView.tsx b/apps/web/src/components/structures/RoomView.tsx index 9483b05a9c..0ea1168944 100644 --- a/apps/web/src/components/structures/RoomView.tsx +++ b/apps/web/src/components/structures/RoomView.tsx @@ -1299,7 +1299,7 @@ export class RoomView extends React.Component { const composerInsertPayload = payload as ComposerInsertPayload; if (composerInsertPayload.composerType) break; - let timelineRenderingType: TimelineRenderingType | undefined; + let timelineRenderingType = composerInsertPayload.timelineRenderingType; // ThreadView handles Action.ComposerInsert itself due to it having its own editState if (composerInsertPayload.timelineRenderingType === TimelineRenderingType.Thread) break; if ( @@ -1311,12 +1311,6 @@ export class RoomView extends React.Component { timelineRenderingType = TimelineRenderingType.Room; } - // If the dispatchee didn't request a timeline rendering type, use the current one. - timelineRenderingType = - timelineRenderingType ?? - composerInsertPayload.timelineRenderingType ?? - this.state.timelineRenderingType; - // re-dispatch to the correct composer defaultDispatcher.dispatch({ ...composerInsertPayload, diff --git a/apps/web/src/components/views/right_panel/TimelineCard.tsx b/apps/web/src/components/views/right_panel/TimelineCard.tsx index 3d5cabd1f9..0ec7b8bedd 100644 --- a/apps/web/src/components/views/right_panel/TimelineCard.tsx +++ b/apps/web/src/components/views/right_panel/TimelineCard.tsx @@ -38,7 +38,6 @@ import { type ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPaylo import Measured from "../elements/Measured"; import { UPDATE_EVENT } from "../../../stores/AsyncStore"; import { ScopedRoomContextProvider } from "../../../contexts/ScopedRoomContext.tsx"; -import { RoomUploadContextProvider } from "../../../viewmodels/room/RoomUploadViewModel.tsx"; import { EventPresentationContextProvider } from "../../../utils/EventPresentationContextProvider"; interface IProps { @@ -215,49 +214,47 @@ export default class TimelineCard extends React.Component { header={_t("right_panel|video_room_chat|title")} ref={this.card} > - - -
- {jumpToBottom} - - -
- - {isUploading && } - - {showComposer && ( - +
+ {jumpToBottom} + + +
+ + {isUploading && } + + {showComposer && ( + + )} ); diff --git a/apps/web/src/components/views/rooms/EditMessageComposer.tsx b/apps/web/src/components/views/rooms/EditMessageComposer.tsx index 598765881d..970d8cffcd 100644 --- a/apps/web/src/components/views/rooms/EditMessageComposer.tsx +++ b/apps/web/src/components/views/rooms/EditMessageComposer.tsx @@ -439,7 +439,7 @@ class EditMessageComposer extends React.Component(null); const handler = useCallback( @@ -52,25 +50,21 @@ export function useWysiwygSendActionHandler( composerFunctions.clear(); focusComposer(composerElement, context, roomContext, timeoutId); break; - case Action.ComposerInsert: { - const insertPayload = payload as ComposerInsertPayload; - if (insertPayload.timelineRenderingType !== roomContext.timelineRenderingType) break; - if (insertPayload.composerType !== ComposerType.Send) break; + case Action.ComposerInsert: + if (payload.timelineRenderingType !== roomContext.timelineRenderingType) break; + if (payload.composerType !== ComposerType.Send) break; - if (insertPayload.userId) { + if (payload.userId) { // TODO insert mention - see SendMessageComposer - } else if (insertPayload.event) { + } else if (payload.event) { // TODO insert quote message - see SendMessageComposer - } else if (insertPayload.text) { + } else if (payload.text) { setSelection(composerContext.selection).then(() => composerFunctions.insertText(payload.text)); - } else if (insertPayload.files) { - uploadVm.initiateViaInputFiles(insertPayload.files); } break; - } } }, - [disabled, composerElement, roomContext, composerFunctions, composerContext, uploadVm], + [disabled, composerElement, roomContext, composerFunctions, composerContext], ); useDispatcher(defaultDispatcher, handler); diff --git a/apps/web/src/dispatcher/actions.ts b/apps/web/src/dispatcher/actions.ts index ad19d1c7d8..b8a4ebdf88 100644 --- a/apps/web/src/dispatcher/actions.ts +++ b/apps/web/src/dispatcher/actions.ts @@ -195,6 +195,11 @@ export enum Action { */ ComposerInsert = "composer_insert", + /** + * Inserts a file into a target composer. + */ + ComposerFileInsert = "composer_insert_file", + /** * Switches space. Should be used with SwitchSpacePayload. */ diff --git a/apps/web/src/dispatcher/payloads/ComposerInsertFilePayload.ts b/apps/web/src/dispatcher/payloads/ComposerInsertFilePayload.ts new file mode 100644 index 0000000000..8445b1610f --- /dev/null +++ b/apps/web/src/dispatcher/payloads/ComposerInsertFilePayload.ts @@ -0,0 +1,16 @@ +/* +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +*/ + +import { type ActionPayload } from "../payloads"; +import { type Action } from "../actions"; +import { type TimelineRenderingType } from "../../contexts/RoomContext"; + +export interface ComposerInsertFilesPayload extends ActionPayload { + action: Action.ComposerFileInsert; + files: File[]; + timelineRenderingType: TimelineRenderingType; +} diff --git a/apps/web/src/dispatcher/payloads/ComposerInsertPayload.ts b/apps/web/src/dispatcher/payloads/ComposerInsertPayload.ts index 7981d1e87b..9712a8303a 100644 --- a/apps/web/src/dispatcher/payloads/ComposerInsertPayload.ts +++ b/apps/web/src/dispatcher/payloads/ComposerInsertPayload.ts @@ -17,7 +17,7 @@ export enum ComposerType { interface IBaseComposerInsertPayload extends ActionPayload { action: Action.ComposerInsert; - timelineRenderingType?: TimelineRenderingType; // undefined if this should just use the current in-focus type. + timelineRenderingType: TimelineRenderingType; composerType?: ComposerType; // falsy if should be re-dispatched to the correct composer } @@ -29,11 +29,4 @@ interface IComposerInsertPlaintextPayload extends IBaseComposerInsertPayload { text: string; } -interface IComposerInsertFilesPayload extends IBaseComposerInsertPayload { - files: File[]; -} - -export type ComposerInsertPayload = - | IComposerInsertMentionPayload - | IComposerInsertPlaintextPayload - | IComposerInsertFilesPayload; +export type ComposerInsertPayload = IComposerInsertMentionPayload | IComposerInsertPlaintextPayload; diff --git a/apps/web/src/modules/ComposerApi.ts b/apps/web/src/modules/ComposerApi.ts index 04314c7d52..c592b1e56f 100644 --- a/apps/web/src/modules/ComposerApi.ts +++ b/apps/web/src/modules/ComposerApi.ts @@ -8,12 +8,15 @@ Please see LICENSE files in the repository root for full details. import { type ComposerApi as ModuleComposerApi, type ComposerApiFileUploadOption, + type ComposerApiView, } from "@element-hq/element-web-module-api"; import { TypedEventEmitter } from "matrix-js-sdk/src/matrix"; import type { MatrixDispatcher } from "../dispatcher/dispatcher"; import { Action } from "../dispatcher/actions"; -import type { ComposerInsertPayload } from "../dispatcher/payloads/ComposerInsertPayload"; +import { ComposerType, type ComposerInsertPayload } from "../dispatcher/payloads/ComposerInsertPayload"; +import { TimelineRenderingType } from "../contexts/RoomContext"; +import type { ComposerInsertFilesPayload } from "../dispatcher/payloads/ComposerInsertFilePayload"; export enum ModuleComposerApiEvents { UploaderOptionsChanged = "uploaderOptionsChanged", @@ -51,17 +54,27 @@ export class ComposerApi this.emit(ModuleComposerApiEvents.UploaderOptionsChanged, option); } - public openFileUploadConfirmation(files: File[]): void { + public openFileUploadConfirmation(files: File[], view: ComposerApiView): void { + if (!["room", "thread"].includes(view.view)) { + throw new Error(`Invalid view '${view.view}'`); + } this.dispatcher.dispatch({ - action: Action.ComposerInsert, + action: Action.ComposerFileInsert, files, - } satisfies ComposerInsertPayload); + timelineRenderingType: view.view === "room" ? TimelineRenderingType.Room : TimelineRenderingType.Thread, + } satisfies ComposerInsertFilesPayload); } - public insertPlaintextIntoComposer(plaintext: string): void { + public insertPlaintextIntoComposer(plaintext: string, view: ComposerApiView): void { + if (!["room", "thread"].includes(view.view)) { + throw new Error(`Invalid view '${view.view}'`); + } this.dispatcher.dispatch({ action: Action.ComposerInsert, text: plaintext, + timelineRenderingType: view.view === "room" ? TimelineRenderingType.Room : TimelineRenderingType.Thread, + // We only support send. + composerType: ComposerType.Send, } satisfies ComposerInsertPayload); } } diff --git a/apps/web/src/viewmodels/room/RoomUploadViewModel.tsx b/apps/web/src/viewmodels/room/RoomUploadViewModel.tsx index 95cdf5ee1e..1875bf8e4e 100644 --- a/apps/web/src/viewmodels/room/RoomUploadViewModel.tsx +++ b/apps/web/src/viewmodels/room/RoomUploadViewModel.tsx @@ -35,12 +35,16 @@ import type { ComposerApiFileUploadOption } from "@element-hq/element-web-module import { useScopedRoomContext } from "../../contexts/ScopedRoomContext"; import { useMatrixClientContext } from "../../contexts/MatrixClientContext"; import ContentMessages from "../../ContentMessages"; -import type { TimelineRenderingType } from "../../contexts/RoomContext"; +import { TimelineRenderingType } from "../../contexts/RoomContext"; import { chromeFileInputFix } from "../../utils/BrowserWorkarounds"; import type { MatrixDispatcher } from "../../dispatcher/dispatcher"; import defaultDispatcher from "../../dispatcher/dispatcher"; import { ModuleApi } from "../../modules/Api"; import { ModuleComposerApiEvents } from "../../modules/ComposerApi"; +import { Action } from "../../dispatcher/actions"; +import type { ComposerInsertFilesPayload } from "../../dispatcher/payloads/ComposerInsertFilePayload"; +import { useDispatcher } from "../../hooks/useDispatcher"; +import { ActionPayload } from "../../dispatcher/payloads"; const logger = rootLogger.getChild("RoomUploadViewModel"); @@ -49,6 +53,7 @@ export class RoomUploadViewModel implements UploadButtonViewActions { private readonly uploadSelectFns = new Map(); + private readonly instanceId = window.crypto.randomUUID(); public constructor( private readonly room: Room, private readonly client: MatrixClient, @@ -65,21 +70,29 @@ export class RoomUploadViewModel options: [], }, ); + logger.info(`Creating ${this.instanceId}`); // Initial check. this.onRoomCurrentStateUpdated(); - // Configure upload functions for (const option of moduleComposerApi.fileUploadOptions) { this.uploadSelectFns.set(option.type, option.onSelected); } this.uploadSelectFns.set("local", this.openUploadDialog); - room.on(RoomEvent.CurrentStateUpdated, this.onRoomCurrentStateUpdated); + this.disposables.trackListener(room, RoomEvent.CurrentStateUpdated, this.onRoomCurrentStateUpdated); + moduleComposerApi.on(ModuleComposerApiEvents.UploaderOptionsChanged, this.onUploaderOptionsChanged); - this.disposables.track(() => { - room.off(RoomEvent.CurrentStateUpdated, this.onRoomCurrentStateUpdated); - moduleComposerApi.off(ModuleComposerApiEvents.UploaderOptionsChanged, this.onUploaderOptionsChanged); - }); + this.disposables.trackListener( + moduleComposerApi, + ModuleComposerApiEvents.UploaderOptionsChanged, + // Types issue. + this.onUploaderOptionsChanged as any, + ); + } + + public dispose(): void { + logger.info(`Disposing of ${this.instanceId}`); + super.dispose(); } private onRoomCurrentStateUpdated = (): void => { @@ -168,14 +181,23 @@ export class RoomUploadViewModel }; public onUploadOptionSelected = (type: ComposerApiFileUploadOption["type"]): void => { + if (![TimelineRenderingType.Room, TimelineRenderingType.Thread].includes(this.timelineRenderingType)) { + throw new Error("Unexpectedly called onUploadOptionSelected outside the context of a room or thread"); + } const fn = this.uploadSelectFns.get(type); if (!fn) { throw new Error("Unexpectedly called onUploadOptionSelected with an unknown type"); } - fn(this.room.roomId, { - inReplyToEventId: this.replyToEvent?.getId(), - relType: this.threadRelation?.rel_type, - }); + fn( + this.room.roomId, + { + view: this.timelineRenderingType === TimelineRenderingType.Room ? "room" : "thread", + }, + { + inReplyToEventId: this.replyToEvent?.getId(), + relType: this.threadRelation?.rel_type, + }, + ); }; private checkCanUpload(): boolean { @@ -209,6 +231,9 @@ export function RoomUploadContextProvider({ "timelineRenderingType", "replyToEvent", ); + if (!room) { + throw new Error("RoomUploadContextProvider must have a room"); + } const client = useMatrixClientContext(); const uploadInput = useRef(null); @@ -219,20 +244,18 @@ export function RoomUploadContextProvider({ uploadInput.current.click(); }, [uploadInput]); - const vm = useCreateAutoDisposedViewModel(() => { - if (!room) { - throw new Error("RoomUploadContextProvider must have a room"); - } - return new RoomUploadViewModel( - room, - client, - timelineRenderingType, - defaultDispatcher, - replyToEvent, - threadRelation, - openFilePicker, - ); - }); + const vm = useCreateAutoDisposedViewModel( + () => + new RoomUploadViewModel( + room, + client, + timelineRenderingType, + defaultDispatcher, + replyToEvent, + threadRelation, + openFilePicker, + ), + ); useEffect(() => { vm.setReplyToEvent(replyToEvent); @@ -259,6 +282,21 @@ export function RoomUploadContextProvider({ [vm], ); + useDispatcher(defaultDispatcher, (payload: ActionPayload) => { + if (payload.action !== Action.ComposerFileInsert) { + return; + } + const fileInsert = payload as ComposerInsertFilesPayload; + if (fileInsert.timelineRenderingType === timelineRenderingType) { + logger.info( + `Got ComposerFileInsert with ${fileInsert.files.length} files`, + timelineRenderingType, + threadRelation, + ); + vm.initiateViaInputFiles(fileInsert.files); + } + }); + // Note, while this logic could be largely replaced with https://developer.mozilla.org/en-US/docs/Web/API/Window/showOpenFilePicker // it does not enjoy support across all our target platforms. // Therefore, we use the invisible input element trick. diff --git a/apps/web/test/unit-tests/modules/ComposerApi-test.ts b/apps/web/test/unit-tests/modules/ComposerApi-test.ts index af70ce0125..2d3c24bc9c 100644 --- a/apps/web/test/unit-tests/modules/ComposerApi-test.ts +++ b/apps/web/test/unit-tests/modules/ComposerApi-test.ts @@ -5,8 +5,10 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ +import { TimelineRenderingType } from "../../../src/contexts/RoomContext"; import { Action } from "../../../src/dispatcher/actions"; import type { MatrixDispatcher } from "../../../src/dispatcher/dispatcher"; +import { type ComposerInsertPayload, ComposerType } from "../../../src/dispatcher/payloads/ComposerInsertPayload"; import { ComposerApi } from "../../../src/modules/ComposerApi"; describe("ComposerApi", () => { @@ -15,10 +17,12 @@ describe("ComposerApi", () => { dispatch: jest.fn(), } as unknown as MatrixDispatcher; const api = new ComposerApi(dispatcher); - api.insertPlaintextIntoComposer("Hello world"); + api.insertPlaintextIntoComposer("Hello world", { view: "room" }); expect(dispatcher.dispatch).toHaveBeenCalledWith({ action: Action.ComposerInsert, text: "Hello world", - }); + timelineRenderingType: TimelineRenderingType.Room, + composerType: ComposerType.Send, + } satisfies ComposerInsertPayload); }); }); diff --git a/packages/module-api/element-web-module-api.api.md b/packages/module-api/element-web-module-api.api.md index a2cdbe9120..47ef6a5828 100644 --- a/packages/module-api/element-web-module-api.api.md +++ b/packages/module-api/element-web-module-api.api.md @@ -103,8 +103,8 @@ export interface ComponentVisibilityCustomisations { // @alpha export interface ComposerApi { addFileUploadOption(option: ComposerApiFileUploadOption): void; - insertPlaintextIntoComposer(plaintext: string): void; - openFileUploadConfirmation(file: File[]): void; + insertPlaintextIntoComposer(plaintext: string, view: ComposerApiTarget): void; + openFileUploadConfirmation(files: File[], view: ComposerApiTarget): void; } // @alpha @@ -112,12 +112,19 @@ export type ComposerApiFileUploadOption = { type: string; label: string; icon?: ComponentType>; - onSelected: (roomId: string, relation?: { + onSelected: (roomId?: string, view?: ComposerApiTarget, relation?: { inReplyToEventId?: string; relType?: string; }) => Promise | void; }; +// @alpha +export type ComposerApiTarget = { + view: "room"; +} | { + view: "thread"; +}; + // @public export interface Config { // (undocumented) diff --git a/packages/module-api/package.json b/packages/module-api/package.json index 2ec60cc7dc..30bb9bc710 100644 --- a/packages/module-api/package.json +++ b/packages/module-api/package.json @@ -26,6 +26,7 @@ ], "scripts": { "prepack": "nx build", + "start": "nx start", "lint:types": "nx lint:types", "test:unit": "vitest" }, diff --git a/packages/module-api/project.json b/packages/module-api/project.json index 2b7e0be792..568eaaf668 100644 --- a/packages/module-api/project.json +++ b/packages/module-api/project.json @@ -9,7 +9,7 @@ "inputs": ["src"], "outputs": ["{projectRoot}/lib"], "options": { - "commands": ["vite build", "api-extractor run"], + "commands": ["vite build", "api-extractor run -l"], "parallel": false, "cwd": "packages/module-api" } diff --git a/packages/module-api/src/api/composer.ts b/packages/module-api/src/api/composer.ts index 636ec41b61..30240a5dac 100644 --- a/packages/module-api/src/api/composer.ts +++ b/packages/module-api/src/api/composer.ts @@ -32,7 +32,8 @@ export type ComposerApiFileUploadOption = { * @returns */ onSelected: ( - roomId: string, + roomId?: string, + view?: ComposerApiTarget, relation?: { inReplyToEventId?: string; relType?: string; @@ -40,6 +41,12 @@ export type ComposerApiFileUploadOption = { ) => Promise | void; }; +/** + * When handling composer interactions, this represents the target composer. + * @alpha Likely to change. This is intentionally left as an object so it may be extended later. + */ +export type ComposerApiTarget = { view: "room" } | { view: "thread" }; + /** * API to interact with the message composer. * @alpha Likely to change @@ -54,13 +61,18 @@ export interface ComposerApi { /** * Open the file upload confirmation dialog. This may be used in conjunction * with `addFileUploadOption` to support an alternative file upload kind. - */ - openFileUploadConfirmation(file: File[]): void; - /** - * Insert plaintext into the current composer. - * @param plaintext - The plain text to insert + * @param files - The files to prompt for + * @param view - The target view to send the file into. * @returns Returns immediately, does not await action. * @alpha Likely to change */ - insertPlaintextIntoComposer(plaintext: string): void; + openFileUploadConfirmation(files: File[], view: ComposerApiTarget): void; + /** + * Insert plaintext into the current composer. + * @param plaintext - The plain text to insert + * @param view - The target view to insert into + * @returns Returns immediately, does not await action. + * @alpha Likely to change + */ + insertPlaintextIntoComposer(plaintext: string, view: ComposerApiTarget): void; } diff --git a/packages/shared-components/src/room/composer/UploadButton/UploadButton.test.tsx b/packages/shared-components/src/room/composer/UploadButton/UploadButton.test.tsx index 13a754b997..8781a58ba2 100644 --- a/packages/shared-components/src/room/composer/UploadButton/UploadButton.test.tsx +++ b/packages/shared-components/src/room/composer/UploadButton/UploadButton.test.tsx @@ -13,13 +13,20 @@ import { fn } from "storybook/test"; import * as stories from "./UploadButton.stories.tsx"; -const { Default } = composeStories(stories); +const { Default, WithOneOption } = composeStories(stories); describe("UploadButton", () => { it("renders a default button", () => { const { container } = render(); expect(container).toMatchSnapshot(); }); + it("will provide one option when only one is available", async () => { + userEvent.setup(); + const onUploadOptionSelected = fn(); + const { getByRole } = render(); + await userEvent.click(getByRole("button", { name: "Attachment" })); + expect(onUploadOptionSelected).toHaveBeenCalledWith("local"); + }); it("can open the menu and select an option", async () => { const onUploadOptionSelected = fn(); const { container, getByRole } = render(); diff --git a/packages/shared-components/src/room/composer/UploadButton/__snapshots__/UploadButton.test.tsx.snap b/packages/shared-components/src/room/composer/UploadButton/__snapshots__/UploadButton.test.tsx.snap index 4a7c958505..11d4ef2724 100644 --- a/packages/shared-components/src/room/composer/UploadButton/__snapshots__/UploadButton.test.tsx.snap +++ b/packages/shared-components/src/room/composer/UploadButton/__snapshots__/UploadButton.test.tsx.snap @@ -6,15 +6,15 @@ exports[`UploadButton > can open the menu and select an option 1`] = ` data-aria-hidden="true" >