From 70665d3ce38ffb08a89bd23295628ca362847535 Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 22 Aug 2024 13:54:01 +0100 Subject: [PATCH] RTE drafts (#12674) * Add drafts to the RTE and tests * test drafts in threads * lint * Add unit test. * Fix test failure * Remove unused import * Clean up wysiwyg drafts and add test. * Fix typo * Add timeout to allow for wasm loading. --------- Co-authored-by: Florian Duros --- playwright/e2e/composer/RTE.spec.ts | 105 ++++++++++++++++++ src/DraftCleaner.ts | 14 ++- .../views/rooms/MessageComposer.tsx | 89 ++++++++++++++- .../components/structures/MatrixChat-test.tsx | 12 ++ .../views/rooms/MessageComposer-test.tsx | 84 +++++++++----- 5 files changed, 266 insertions(+), 38 deletions(-) diff --git a/playwright/e2e/composer/RTE.spec.ts b/playwright/e2e/composer/RTE.spec.ts index 53599d5320..47243e1c82 100644 --- a/playwright/e2e/composer/RTE.spec.ts +++ b/playwright/e2e/composer/RTE.spec.ts @@ -249,5 +249,110 @@ test.describe("Composer", () => { ); }); }); + + test.describe("Drafts", () => { + test("drafts with rich and plain text", async ({ page, app }) => { + // Set up a second room to swtich to, to test drafts + const firstRoomname = "Composing Room"; + const secondRoomname = "Second Composing Room"; + await app.client.createRoom({ name: secondRoomname }); + + // Composer is visible + const composer = page.locator("div[contenteditable=true]"); + await expect(composer).toBeVisible(); + + // Type some formatted text + await composer.pressSequentially("my "); + await composer.press(`${CtrlOrMeta}+KeyB`); + await composer.pressSequentially("bold"); + + // Change to plain text mode + await page.getByRole("button", { name: "Hide formatting" }).click(); + + // Change to another room and back again + await app.viewRoomByName(secondRoomname); + await app.viewRoomByName(firstRoomname); + + // assert the markdown + await expect(page.locator("div[contenteditable=true]", { hasText: "my __bold__" })).toBeVisible(); + + // Change to plain text mode and assert the markdown + await page.getByRole("button", { name: "Show formatting" }).click(); + + // Change to another room and back again + await app.viewRoomByName(secondRoomname); + await app.viewRoomByName(firstRoomname); + + // Send the message and assert the message + await page.getByRole("button", { name: "Send message" }).click(); + await expect(page.locator(".mx_EventTile_last .mx_EventTile_body").getByText("my bold")).toBeVisible(); + }); + + test("draft with replies", async ({ page, app }) => { + // Set up a second room to swtich to, to test drafts + const firstRoomname = "Composing Room"; + const secondRoomname = "Second Composing Room"; + await app.client.createRoom({ name: secondRoomname }); + + // Composer is visible + const composer = page.locator("div[contenteditable=true]"); + await expect(composer).toBeVisible(); + + // Send a message + await composer.pressSequentially("my first message"); + await page.getByRole("button", { name: "Send message" }).click(); + + // Click reply + const tile = page.locator(".mx_EventTile_last"); + await tile.hover(); + await tile.getByRole("button", { name: "Reply", exact: true }).click(); + + // Type reply text + await composer.pressSequentially("my reply"); + + // Change to another room and back again + await app.viewRoomByName(secondRoomname); + await app.viewRoomByName(firstRoomname); + + // Assert reply mode and reply text + await expect(page.getByText("Replying")).toBeVisible(); + await expect(page.locator("div[contenteditable=true]", { hasText: "my reply" })).toBeVisible(); + }); + + test("draft in threads", async ({ page, app }) => { + // Set up a second room to swtich to, to test drafts + const firstRoomname = "Composing Room"; + const secondRoomname = "Second Composing Room"; + await app.client.createRoom({ name: secondRoomname }); + + // Composer is visible + const composer = page.locator("div[contenteditable=true]"); + await expect(composer).toBeVisible(); + + // Send a message + await composer.pressSequentially("my first message"); + await page.getByRole("button", { name: "Send message" }).click(); + + // Click reply + const tile = page.locator(".mx_EventTile_last"); + await tile.hover(); + await tile.getByRole("button", { name: "Reply in thread" }).click(); + + const thread = page.locator(".mx_ThreadView"); + const threadComposer = thread.locator("div[contenteditable=true]"); + + // Type threaded text + await threadComposer.pressSequentially("my threaded message"); + + // Change to another room and back again + await app.viewRoomByName(secondRoomname); + await app.viewRoomByName(firstRoomname); + + // Assert threaded draft + await expect( + thread.locator("div[contenteditable=true]", { hasText: "my threaded message" }), + ).toBeVisible(); + }); + }); }); }); diff --git a/src/DraftCleaner.ts b/src/DraftCleaner.ts index 5e6c1cbae7..cede027f22 100644 --- a/src/DraftCleaner.ts +++ b/src/DraftCleaner.ts @@ -18,6 +18,7 @@ import { logger } from "matrix-js-sdk/src/logger"; import { MatrixClientPeg } from "./MatrixClientPeg"; import { EDITOR_STATE_STORAGE_PREFIX } from "./components/views/rooms/SendMessageComposer"; +import { WYSIWYG_EDITOR_STATE_STORAGE_PREFIX } from "./components/views/rooms/MessageComposer"; // The key used to persist the the timestamp we last cleaned up drafts export const DRAFT_LAST_CLEANUP_KEY = "mx_draft_cleanup"; @@ -61,14 +62,21 @@ function shouldCleanupDrafts(): boolean { } /** - * Clear all drafts for the CIDER editor if the room does not exist in the known rooms. + * Clear all drafts for the CIDER and WYSIWYG editors if the room does not exist in the known rooms. */ function cleaupDrafts(): void { for (let i = 0; i < localStorage.length; i++) { const keyName = localStorage.key(i); - if (!keyName?.startsWith(EDITOR_STATE_STORAGE_PREFIX)) continue; + if (!keyName) continue; + let roomId: string | undefined = undefined; + if (keyName.startsWith(EDITOR_STATE_STORAGE_PREFIX)) { + roomId = keyName.slice(EDITOR_STATE_STORAGE_PREFIX.length).split("_$")[0]; + } + if (keyName.startsWith(WYSIWYG_EDITOR_STATE_STORAGE_PREFIX)) { + roomId = keyName.slice(WYSIWYG_EDITOR_STATE_STORAGE_PREFIX.length).split("_$")[0]; + } + if (!roomId) continue; // Remove the prefix and the optional event id suffix to leave the room id - const roomId = keyName.slice(EDITOR_STATE_STORAGE_PREFIX.length).split("_$")[0]; const room = MatrixClientPeg.safeGet().getRoom(roomId); if (!room) { logger.debug(`Removing draft for unknown room with key ${keyName}`); diff --git a/src/components/views/rooms/MessageComposer.tsx b/src/components/views/rooms/MessageComposer.tsx index a4a73b495f..b99c1c401e 100644 --- a/src/components/views/rooms/MessageComposer.tsx +++ b/src/components/views/rooms/MessageComposer.tsx @@ -26,6 +26,7 @@ import { } from "matrix-js-sdk/src/matrix"; import { Optional } from "matrix-events-sdk"; import { Tooltip } from "@vector-im/compound-web"; +import { logger } from "matrix-js-sdk/src/logger"; import { _t } from "../../../languageHandler"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; @@ -65,6 +66,9 @@ import { createCantStartVoiceMessageBroadcastDialog } from "../dialogs/CantStart import { UIFeature } from "../../../settings/UIFeature"; import { formatTimeLeft } from "../../../DateUtils"; +// The prefix used when persisting editor drafts to localstorage. +export const WYSIWYG_EDITOR_STATE_STORAGE_PREFIX = "mx_wysiwyg_state_"; + let instanceCount = 0; interface ISendButtonProps { @@ -109,6 +113,12 @@ interface IState { initialComposerContent: string; } +type WysiwygComposerState = { + content: string; + isRichText: boolean; + replyEventId?: string; +}; + export class MessageComposer extends React.Component { private dispatcherRef?: string; private messageComposerInput = createRef(); @@ -129,11 +139,32 @@ export class MessageComposer extends React.Component { public constructor(props: IProps, context: React.ContextType) { super(props, context); + this.context = context; // otherwise React will only set it prior to render due to type def above + VoiceRecordingStore.instance.on(UPDATE_EVENT, this.onVoiceStoreUpdate); + window.addEventListener("beforeunload", this.saveWysiwygEditorState); + const isWysiwygLabEnabled = SettingsStore.getValue("feature_wysiwyg_composer"); + let isRichTextEnabled = true; + let initialComposerContent = ""; + if (isWysiwygLabEnabled) { + const wysiwygState = this.restoreWysiwygEditorState(); + if (wysiwygState) { + isRichTextEnabled = wysiwygState.isRichText; + initialComposerContent = wysiwygState.content; + if (wysiwygState.replyEventId) { + dis.dispatch({ + action: "reply_to_event", + event: this.props.room.findEventById(wysiwygState.replyEventId), + context: this.context.timelineRenderingType, + }); + } + } + } + this.state = { - isComposerEmpty: true, - composerContent: "", + isComposerEmpty: initialComposerContent?.length === 0, + composerContent: initialComposerContent, haveRecording: false, recordingTimeLeftSeconds: undefined, // when set to a number, shows a toast isMenuOpen: false, @@ -141,9 +172,9 @@ export class MessageComposer extends React.Component { showStickersButton: SettingsStore.getValue("MessageComposerInput.showStickersButton"), showPollsButton: SettingsStore.getValue("MessageComposerInput.showPollsButton"), showVoiceBroadcastButton: SettingsStore.getValue(Features.VoiceBroadcast), - isWysiwygLabEnabled: SettingsStore.getValue("feature_wysiwyg_composer"), - isRichTextEnabled: true, - initialComposerContent: "", + isWysiwygLabEnabled: isWysiwygLabEnabled, + isRichTextEnabled: isRichTextEnabled, + initialComposerContent: initialComposerContent, }; this.instanceId = instanceCount++; @@ -154,6 +185,52 @@ export class MessageComposer extends React.Component { SettingsStore.monitorSetting("feature_wysiwyg_composer", null); } + private get editorStateKey(): string { + let key = WYSIWYG_EDITOR_STATE_STORAGE_PREFIX + this.props.room.roomId; + if (this.props.relation?.rel_type === THREAD_RELATION_TYPE.name) { + key += `_${this.props.relation.event_id}`; + } + return key; + } + + private restoreWysiwygEditorState(): WysiwygComposerState | undefined { + const json = localStorage.getItem(this.editorStateKey); + if (json) { + try { + const state: WysiwygComposerState = JSON.parse(json); + return state; + } catch (e) { + logger.error(e); + } + } + return undefined; + } + + private saveWysiwygEditorState = (): void => { + if (this.shouldSaveWysiwygEditorState()) { + const { isRichTextEnabled, composerContent } = this.state; + const replyEventId = this.props.replyToEvent ? this.props.replyToEvent.getId() : undefined; + const item: WysiwygComposerState = { + content: composerContent, + isRichText: isRichTextEnabled, + replyEventId: replyEventId, + }; + localStorage.setItem(this.editorStateKey, JSON.stringify(item)); + } else { + this.clearStoredEditorState(); + } + }; + + // should save state when wysiwyg is enabled and has contents or reply is open + private shouldSaveWysiwygEditorState = (): boolean => { + const { isWysiwygLabEnabled, isComposerEmpty } = this.state; + return isWysiwygLabEnabled && (!isComposerEmpty || !!this.props.replyToEvent); + }; + + private clearStoredEditorState(): void { + localStorage.removeItem(this.editorStateKey); + } + private get voiceRecording(): Optional { return this._voiceRecording; } @@ -265,6 +342,8 @@ export class MessageComposer extends React.Component { UIStore.instance.stopTrackingElementDimensions(`MessageComposer${this.instanceId}`); UIStore.instance.removeListener(`MessageComposer${this.instanceId}`, this.onResize); + window.removeEventListener("beforeunload", this.saveWysiwygEditorState); + this.saveWysiwygEditorState(); // clean up our listeners by setting our cached recording to falsy (see internal setter) this.voiceRecording = null; } diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 3b49ec2139..8d7f07e7df 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -624,6 +624,18 @@ describe("", () => { expect(localStorage.getItem(`mx_cider_state_${unknownRoomId}`)).toBeNull(); }); + it("should clean up wysiwyg drafts", async () => { + Date.now = jest.fn(() => timestamp); + localStorage.setItem(`mx_wysiwyg_state_${roomId}`, "fake_content"); + localStorage.setItem(`mx_wysiwyg_state_${unknownRoomId}`, "fake_content"); + await getComponentAndWaitForReady(); + mockClient.emit(ClientEvent.Sync, SyncState.Syncing, SyncState.Syncing); + // let things settle + await flushPromises(); + expect(localStorage.getItem(`mx_wysiwyg_state_${roomId}`)).not.toBeNull(); + expect(localStorage.getItem(`mx_wysiwyg_state_${unknownRoomId}`)).toBeNull(); + }); + it("should not clean up drafts before expiry", async () => { // Set the last cleanup to the recent past localStorage.setItem(`mx_cider_state_${unknownRoomId}`, "fake_content"); diff --git a/test/components/views/rooms/MessageComposer-test.tsx b/test/components/views/rooms/MessageComposer-test.tsx index 1aea150a8c..6480a84007 100644 --- a/test/components/views/rooms/MessageComposer-test.tsx +++ b/test/components/views/rooms/MessageComposer-test.tsx @@ -16,7 +16,7 @@ limitations under the License. import * as React from "react"; import { EventType, MatrixEvent, Room, RoomMember, THREAD_RELATION_TYPE } from "matrix-js-sdk/src/matrix"; -import { act, render, screen } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { @@ -49,10 +49,6 @@ import { VoiceBroadcastInfoState, VoiceBroadcastRecording } from "../../../../sr import { mkVoiceBroadcastInfoStateEvent } from "../../../voice-broadcast/utils/test-utils"; import { SdkContextClass } from "../../../../src/contexts/SDKContext"; -jest.mock("../../../../src/components/views/rooms/wysiwyg_composer", () => ({ - SendWysiwygComposer: jest.fn().mockImplementation(() =>
), -})); - const openStickerPicker = async (): Promise => { await act(async () => { await userEvent.click(screen.getByLabelText("More options")); @@ -76,12 +72,6 @@ const setCurrentBroadcastRecording = (room: Room, state: VoiceBroadcastInfoState SdkContextClass.instance.voiceBroadcastRecordingsStore.setCurrent(recording); }; -const shouldClearModal = async (): Promise => { - afterEach(async () => { - await clearAllModals(); - }); -}; - const expectVoiceMessageRecordingTriggered = (): void => { // Checking for the voice message dialog text, if no mic can be found. // By this we know at least that starting a voice message was triggered. @@ -96,7 +86,8 @@ describe("MessageComposer", () => { mockPlatformPeg(); }); - afterEach(() => { + afterEach(async () => { + await clearAllModals(); jest.useRealTimers(); SdkContextClass.instance.voiceBroadcastRecordingsStore.clearCurrent(); @@ -415,8 +406,6 @@ describe("MessageComposer", () => { await flushPromises(); }); - shouldClearModal(); - it("should try to start a voice message", () => { expectVoiceMessageRecordingTriggered(); }); @@ -430,8 +419,6 @@ describe("MessageComposer", () => { await waitEnoughCyclesForModal(); }); - shouldClearModal(); - it("should not start a voice message and display the info dialog", async () => { expect(screen.queryByLabelText("Stop recording")).not.toBeInTheDocument(); expect(screen.getByText("Can't start voice message")).toBeInTheDocument(); @@ -446,8 +433,6 @@ describe("MessageComposer", () => { await waitEnoughCyclesForModal(); }); - shouldClearModal(); - it("should try to start a voice message and should not display the info dialog", async () => { expect(screen.queryByText("Can't start voice message")).not.toBeInTheDocument(); expectVoiceMessageRecordingTriggered(); @@ -467,13 +452,50 @@ describe("MessageComposer", () => { }); }); - it("should render SendWysiwygComposer when enabled", () => { + it("wysiwyg correctly persists state to and from localStorage", async () => { const room = mkStubRoom("!roomId:server", "Room 1", cli); - SettingsStore.setValue("feature_wysiwyg_composer", null, SettingLevel.DEVICE, true); + const messageText = "Test Text"; + await SettingsStore.setValue("feature_wysiwyg_composer", null, SettingLevel.DEVICE, true); + const { renderResult, rawComponent } = wrapAndRender({ room }); + const { unmount, rerender } = renderResult; - wrapAndRender({ room }); - expect(screen.getByTestId("wysiwyg-composer")).toBeInTheDocument(); - }); + await act(async () => { + await flushPromises(); + }); + + const key = `mx_wysiwyg_state_${room.roomId}`; + + await act(async () => { + await userEvent.click(screen.getByRole("textbox")); + }); + fireEvent.input(screen.getByRole("textbox"), { + data: messageText, + inputType: "insertText", + }); + + await waitFor(() => expect(screen.getByRole("textbox")).toHaveTextContent(messageText)); + + // Wait for event dispatch to happen + await act(async () => { + await flushPromises(); + }); + + // assert there is state persisted + expect(localStorage.getItem(key)).toBeNull(); + + // ensure the right state was persisted to localStorage + unmount(); + + // assert the persisted state + expect(JSON.parse(localStorage.getItem(key)!)).toStrictEqual({ + content: messageText, + isRichText: true, + }); + + // ensure the correct state is re-loaded + rerender(rawComponent); + await waitFor(() => expect(screen.getByRole("textbox")).toHaveTextContent(messageText)); + }, 10000); }); function wrapAndRender( @@ -506,14 +528,16 @@ function wrapAndRender( permalinkCreator: new RoomPermalinkCreator(room), }; + const getRawComponent = (props = {}, context = roomContext, client = mockClient) => ( + + + + + + ); return { - renderResult: render( - - - - - , - ), + rawComponent: getRawComponent(props, roomContext, mockClient), + renderResult: render(getRawComponent(props, roomContext, mockClient)), roomContext, }; }