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 <florian.duros@ormaz.fr>
This commit is contained in:
David Langley 2024-08-22 13:54:01 +01:00 committed by GitHub
parent fdc5acd5a4
commit 70665d3ce3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 266 additions and 38 deletions

View file

@ -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();
});
});
}); });
}); });

View file

@ -18,6 +18,7 @@ import { logger } from "matrix-js-sdk/src/logger";
import { MatrixClientPeg } from "./MatrixClientPeg"; import { MatrixClientPeg } from "./MatrixClientPeg";
import { EDITOR_STATE_STORAGE_PREFIX } from "./components/views/rooms/SendMessageComposer"; 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 // The key used to persist the the timestamp we last cleaned up drafts
export const DRAFT_LAST_CLEANUP_KEY = "mx_draft_cleanup"; 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 { function cleaupDrafts(): void {
for (let i = 0; i < localStorage.length; i++) { for (let i = 0; i < localStorage.length; i++) {
const keyName = localStorage.key(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 // 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); const room = MatrixClientPeg.safeGet().getRoom(roomId);
if (!room) { if (!room) {
logger.debug(`Removing draft for unknown room with key ${keyName}`); logger.debug(`Removing draft for unknown room with key ${keyName}`);

View file

@ -26,6 +26,7 @@ import {
} from "matrix-js-sdk/src/matrix"; } from "matrix-js-sdk/src/matrix";
import { Optional } from "matrix-events-sdk"; import { Optional } from "matrix-events-sdk";
import { Tooltip } from "@vector-im/compound-web"; import { Tooltip } from "@vector-im/compound-web";
import { logger } from "matrix-js-sdk/src/logger";
import { _t } from "../../../languageHandler"; import { _t } from "../../../languageHandler";
import { MatrixClientPeg } from "../../../MatrixClientPeg"; import { MatrixClientPeg } from "../../../MatrixClientPeg";
@ -65,6 +66,9 @@ import { createCantStartVoiceMessageBroadcastDialog } from "../dialogs/CantStart
import { UIFeature } from "../../../settings/UIFeature"; import { UIFeature } from "../../../settings/UIFeature";
import { formatTimeLeft } from "../../../DateUtils"; 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; let instanceCount = 0;
interface ISendButtonProps { interface ISendButtonProps {
@ -109,6 +113,12 @@ interface IState {
initialComposerContent: string; initialComposerContent: string;
} }
type WysiwygComposerState = {
content: string;
isRichText: boolean;
replyEventId?: string;
};
export class MessageComposer extends React.Component<IProps, IState> { export class MessageComposer extends React.Component<IProps, IState> {
private dispatcherRef?: string; private dispatcherRef?: string;
private messageComposerInput = createRef<SendMessageComposerClass>(); private messageComposerInput = createRef<SendMessageComposerClass>();
@ -129,11 +139,32 @@ export class MessageComposer extends React.Component<IProps, IState> {
public constructor(props: IProps, context: React.ContextType<typeof RoomContext>) { public constructor(props: IProps, context: React.ContextType<typeof RoomContext>) {
super(props, context); 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); VoiceRecordingStore.instance.on(UPDATE_EVENT, this.onVoiceStoreUpdate);
window.addEventListener("beforeunload", this.saveWysiwygEditorState);
const isWysiwygLabEnabled = SettingsStore.getValue<boolean>("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 = { this.state = {
isComposerEmpty: true, isComposerEmpty: initialComposerContent?.length === 0,
composerContent: "", composerContent: initialComposerContent,
haveRecording: false, haveRecording: false,
recordingTimeLeftSeconds: undefined, // when set to a number, shows a toast recordingTimeLeftSeconds: undefined, // when set to a number, shows a toast
isMenuOpen: false, isMenuOpen: false,
@ -141,9 +172,9 @@ export class MessageComposer extends React.Component<IProps, IState> {
showStickersButton: SettingsStore.getValue("MessageComposerInput.showStickersButton"), showStickersButton: SettingsStore.getValue("MessageComposerInput.showStickersButton"),
showPollsButton: SettingsStore.getValue("MessageComposerInput.showPollsButton"), showPollsButton: SettingsStore.getValue("MessageComposerInput.showPollsButton"),
showVoiceBroadcastButton: SettingsStore.getValue(Features.VoiceBroadcast), showVoiceBroadcastButton: SettingsStore.getValue(Features.VoiceBroadcast),
isWysiwygLabEnabled: SettingsStore.getValue<boolean>("feature_wysiwyg_composer"), isWysiwygLabEnabled: isWysiwygLabEnabled,
isRichTextEnabled: true, isRichTextEnabled: isRichTextEnabled,
initialComposerContent: "", initialComposerContent: initialComposerContent,
}; };
this.instanceId = instanceCount++; this.instanceId = instanceCount++;
@ -154,6 +185,52 @@ export class MessageComposer extends React.Component<IProps, IState> {
SettingsStore.monitorSetting("feature_wysiwyg_composer", null); 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<VoiceMessageRecording> { private get voiceRecording(): Optional<VoiceMessageRecording> {
return this._voiceRecording; return this._voiceRecording;
} }
@ -265,6 +342,8 @@ export class MessageComposer extends React.Component<IProps, IState> {
UIStore.instance.stopTrackingElementDimensions(`MessageComposer${this.instanceId}`); UIStore.instance.stopTrackingElementDimensions(`MessageComposer${this.instanceId}`);
UIStore.instance.removeListener(`MessageComposer${this.instanceId}`, this.onResize); 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) // clean up our listeners by setting our cached recording to falsy (see internal setter)
this.voiceRecording = null; this.voiceRecording = null;
} }

View file

@ -624,6 +624,18 @@ describe("<MatrixChat />", () => {
expect(localStorage.getItem(`mx_cider_state_${unknownRoomId}`)).toBeNull(); 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 () => { it("should not clean up drafts before expiry", async () => {
// Set the last cleanup to the recent past // Set the last cleanup to the recent past
localStorage.setItem(`mx_cider_state_${unknownRoomId}`, "fake_content"); localStorage.setItem(`mx_cider_state_${unknownRoomId}`, "fake_content");

View file

@ -16,7 +16,7 @@ limitations under the License.
import * as React from "react"; import * as React from "react";
import { EventType, MatrixEvent, Room, RoomMember, THREAD_RELATION_TYPE } from "matrix-js-sdk/src/matrix"; 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 userEvent from "@testing-library/user-event";
import { import {
@ -49,10 +49,6 @@ import { VoiceBroadcastInfoState, VoiceBroadcastRecording } from "../../../../sr
import { mkVoiceBroadcastInfoStateEvent } from "../../../voice-broadcast/utils/test-utils"; import { mkVoiceBroadcastInfoStateEvent } from "../../../voice-broadcast/utils/test-utils";
import { SdkContextClass } from "../../../../src/contexts/SDKContext"; import { SdkContextClass } from "../../../../src/contexts/SDKContext";
jest.mock("../../../../src/components/views/rooms/wysiwyg_composer", () => ({
SendWysiwygComposer: jest.fn().mockImplementation(() => <div data-testid="wysiwyg-composer" />),
}));
const openStickerPicker = async (): Promise<void> => { const openStickerPicker = async (): Promise<void> => {
await act(async () => { await act(async () => {
await userEvent.click(screen.getByLabelText("More options")); await userEvent.click(screen.getByLabelText("More options"));
@ -76,12 +72,6 @@ const setCurrentBroadcastRecording = (room: Room, state: VoiceBroadcastInfoState
SdkContextClass.instance.voiceBroadcastRecordingsStore.setCurrent(recording); SdkContextClass.instance.voiceBroadcastRecordingsStore.setCurrent(recording);
}; };
const shouldClearModal = async (): Promise<void> => {
afterEach(async () => {
await clearAllModals();
});
};
const expectVoiceMessageRecordingTriggered = (): void => { const expectVoiceMessageRecordingTriggered = (): void => {
// Checking for the voice message dialog text, if no mic can be found. // 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. // By this we know at least that starting a voice message was triggered.
@ -96,7 +86,8 @@ describe("MessageComposer", () => {
mockPlatformPeg(); mockPlatformPeg();
}); });
afterEach(() => { afterEach(async () => {
await clearAllModals();
jest.useRealTimers(); jest.useRealTimers();
SdkContextClass.instance.voiceBroadcastRecordingsStore.clearCurrent(); SdkContextClass.instance.voiceBroadcastRecordingsStore.clearCurrent();
@ -415,8 +406,6 @@ describe("MessageComposer", () => {
await flushPromises(); await flushPromises();
}); });
shouldClearModal();
it("should try to start a voice message", () => { it("should try to start a voice message", () => {
expectVoiceMessageRecordingTriggered(); expectVoiceMessageRecordingTriggered();
}); });
@ -430,8 +419,6 @@ describe("MessageComposer", () => {
await waitEnoughCyclesForModal(); await waitEnoughCyclesForModal();
}); });
shouldClearModal();
it("should not start a voice message and display the info dialog", async () => { it("should not start a voice message and display the info dialog", async () => {
expect(screen.queryByLabelText("Stop recording")).not.toBeInTheDocument(); expect(screen.queryByLabelText("Stop recording")).not.toBeInTheDocument();
expect(screen.getByText("Can't start voice message")).toBeInTheDocument(); expect(screen.getByText("Can't start voice message")).toBeInTheDocument();
@ -446,8 +433,6 @@ describe("MessageComposer", () => {
await waitEnoughCyclesForModal(); await waitEnoughCyclesForModal();
}); });
shouldClearModal();
it("should try to start a voice message and should not display the info dialog", async () => { 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(); expect(screen.queryByText("Can't start voice message")).not.toBeInTheDocument();
expectVoiceMessageRecordingTriggered(); 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); 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 }); await act(async () => {
expect(screen.getByTestId("wysiwyg-composer")).toBeInTheDocument(); 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( function wrapAndRender(
@ -506,14 +528,16 @@ function wrapAndRender(
permalinkCreator: new RoomPermalinkCreator(room), permalinkCreator: new RoomPermalinkCreator(room),
}; };
const getRawComponent = (props = {}, context = roomContext, client = mockClient) => (
<MatrixClientContext.Provider value={client}>
<RoomContext.Provider value={context}>
<MessageComposer {...defaultProps} {...props} />
</RoomContext.Provider>
</MatrixClientContext.Provider>
);
return { return {
renderResult: render( rawComponent: getRawComponent(props, roomContext, mockClient),
<MatrixClientContext.Provider value={mockClient}> renderResult: render(getRawComponent(props, roomContext, mockClient)),
<RoomContext.Provider value={roomContext}>
<MessageComposer {...defaultProps} {...props} />
</RoomContext.Provider>
</MatrixClientContext.Provider>,
),
roomContext, roomContext,
}; };
} }