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

Fix threads fallback incorrectly targets root event #9229

Merged
merged 27 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e6437c8
Use RelationType enum instead of hardcoded value
Aug 31, 2022
c84d4ed
Fix threads replies fallback to target last reply
Aug 31, 2022
0fc15ee
Only unsubscribe from threads events if needed
Sep 1, 2022
29b9a26
fix strict null check
Sep 1, 2022
3e9912b
Merge branch 'develop' into gsouquet/threads-fallback-23147
Sep 1, 2022
52cfc36
fix strict null checks
Sep 2, 2022
f1b51d6
strict null checks
Sep 2, 2022
7011e9a
fix typing
Sep 5, 2022
7141aa5
Unsubscribe listeners if new thread is `null`
Sep 14, 2022
d8f452d
Merge branch 'develop' into gsouquet/threads-fallback-23147
Sep 14, 2022
482b160
Update strict null checks
Sep 14, 2022
3fd4f7c
Type HTMLElement as nullable
Sep 14, 2022
50011b8
Merge branch 'develop' into gsouquet/threads-fallback-23147
Sep 21, 2022
1bebc60
Merge branch 'develop' into gsouquet/threads-fallback-23147
germain-gg Oct 19, 2022
5006458
Add thread fallback integration test
germain-gg Oct 19, 2022
03b0ed0
lint fix
germain-gg Oct 19, 2022
2757cfc
Update snapshots
germain-gg Oct 19, 2022
7be06dc
Merge branch 'develop' into gsouquet/threads-fallback-23147
germain-gg Oct 19, 2022
1d60010
Add test after changing thread
germain-gg Oct 20, 2022
24ea206
Merge branch 'develop' into gsouquet/threads-fallback-23147
germain-gg Oct 20, 2022
2e4d29b
Remove test comment
germain-gg Oct 20, 2022
d29e244
update snapshot
germain-gg Oct 20, 2022
a314590
fix room context test utility
germain-gg Oct 20, 2022
4ad2e39
Add ThreadListContextMenu test
germain-gg Oct 20, 2022
3ce426c
lint fix
germain-gg Oct 20, 2022
f6d6f65
fix thread rendering
germain-gg Oct 20, 2022
5ceed33
Merge branch 'develop' into gsouquet/threads-fallback-23147
t3chguy Oct 20, 2022
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
18 changes: 9 additions & 9 deletions src/components/structures/FileDropTarget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import React, { useEffect, useState } from "react";
import { _t } from "../../languageHandler";

interface IProps {
parent: HTMLElement;
parent: HTMLElement | null;
onFileDrop(dataTransfer: DataTransfer): void;
}

Expand Down Expand Up @@ -90,20 +90,20 @@ const FileDropTarget: React.FC<IProps> = ({ parent, onFileDrop }) => {
}));
};

parent.addEventListener("drop", onDrop);
parent.addEventListener("dragover", onDragOver);
parent.addEventListener("dragenter", onDragEnter);
parent.addEventListener("dragleave", onDragLeave);
parent?.addEventListener("drop", onDrop);
parent?.addEventListener("dragover", onDragOver);
parent?.addEventListener("dragenter", onDragEnter);
parent?.addEventListener("dragleave", onDragLeave);

return () => {
// disconnect the D&D event listeners from the room view. This
// is really just for hygiene - we're going to be
// deleted anyway, so it doesn't matter if the event listeners
// don't get cleaned up.
parent.removeEventListener("drop", onDrop);
parent.removeEventListener("dragover", onDragOver);
parent.removeEventListener("dragenter", onDragEnter);
parent.removeEventListener("dragleave", onDragLeave);
parent?.removeEventListener("drop", onDrop);
parent?.removeEventListener("dragover", onDragOver);
parent?.removeEventListener("dragenter", onDragEnter);
parent?.removeEventListener("dragleave", onDragLeave);
};
}, [parent, onFileDrop]);

Expand Down
111 changes: 81 additions & 30 deletions src/components/structures/ThreadView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

import React, { createRef, KeyboardEvent } from 'react';
import { Thread, THREAD_RELATION_TYPE, ThreadEvent } from 'matrix-js-sdk/src/models/thread';
import { Room } from 'matrix-js-sdk/src/models/room';
import { Room, RoomEvent } from 'matrix-js-sdk/src/models/room';
import { IEventRelation, MatrixEvent } from 'matrix-js-sdk/src/models/event';
import { TimelineWindow } from 'matrix-js-sdk/src/timeline-window';
import { Direction } from 'matrix-js-sdk/src/models/event-timeline';
Expand Down Expand Up @@ -70,6 +70,7 @@ interface IProps {

interface IState {
thread?: Thread;
lastReply?: MatrixEvent | null;
layout: Layout;
editState?: EditorStateTransfer;
replyToEvent?: MatrixEvent;
Expand All @@ -88,9 +89,16 @@ export default class ThreadView extends React.Component<IProps, IState> {
constructor(props: IProps) {
super(props);

const thread = this.props.room.getThread(this.props.mxEvent.getId());

this.setupThreadListeners(thread);
this.state = {
layout: SettingsStore.getValue("layout"),
narrow: false,
thread,
lastReply: thread?.lastReply((ev: MatrixEvent) => {
return ev.isRelation(THREAD_RELATION_TYPE.name) && !ev.status;
}),
};

this.layoutWatcherRef = SettingsStore.watchSetting("layout", null, (...[,,, value]) =>
Expand All @@ -99,6 +107,9 @@ export default class ThreadView extends React.Component<IProps, IState> {
}

public componentDidMount(): void {
if (this.state.thread) {
this.postThreadUpdate(this.state.thread);
}
this.setupThread(this.props.mxEvent);
this.dispatcherRef = dis.register(this.onAction);

Expand Down Expand Up @@ -189,19 +200,49 @@ export default class ThreadView extends React.Component<IProps, IState> {
}
};

private updateThreadRelation = (): void => {
this.setState({
lastReply: this.threadLastReply,
});
};

private get threadLastReply(): MatrixEvent | undefined {
return this.state.thread?.lastReply((ev: MatrixEvent) => {
return ev.isRelation(THREAD_RELATION_TYPE.name) && !ev.status;
});
}

private updateThread = (thread?: Thread) => {
if (thread && this.state.thread !== thread) {
if (this.state.thread === thread) return;

this.setupThreadListeners(thread, this.state.thread);
if (thread) {
this.setState({
thread,
}, async () => {
thread.emit(ThreadEvent.ViewThread);
await thread.fetchInitialEvents();
this.nextBatch = thread.liveTimeline.getPaginationToken(Direction.Backward);
this.timelinePanel.current?.refreshTimeline();
});
lastReply: this.threadLastReply,
}, async () => this.postThreadUpdate(thread));
}
};

private async postThreadUpdate(thread: Thread): Promise<void> {
thread.emit(ThreadEvent.ViewThread);
await thread.fetchInitialEvents();
this.updateThreadRelation();
this.nextBatch = thread.liveTimeline.getPaginationToken(Direction.Backward);
this.timelinePanel.current?.refreshTimeline();
}

private setupThreadListeners(thread?: Thread | undefined, oldThread?: Thread | undefined): void {
if (oldThread) {
this.state.thread.off(ThreadEvent.NewReply, this.updateThreadRelation);
this.props.room.off(RoomEvent.LocalEchoUpdated, this.updateThreadRelation);
}
if (thread) {
thread.on(ThreadEvent.NewReply, this.updateThreadRelation);
this.props.room.on(RoomEvent.LocalEchoUpdated, this.updateThreadRelation);
}
}

private resetJumpToEvent = (event?: string): void => {
if (this.props.initialEvent && this.props.initialEventScrollIntoView &&
event === this.props.initialEvent?.getId()) {
Expand Down Expand Up @@ -242,14 +283,14 @@ export default class ThreadView extends React.Component<IProps, IState> {
}
};

private nextBatch: string;
private nextBatch: string | undefined | null = null;

private onPaginationRequest = async (
timelineWindow: TimelineWindow | null,
direction = Direction.Backward,
limit = 20,
): Promise<boolean> => {
if (!Thread.hasServerSideSupport) {
if (!Thread.hasServerSideSupport && timelineWindow) {
timelineWindow.extend(direction, limit);
return true;
}
Expand All @@ -262,40 +303,50 @@ export default class ThreadView extends React.Component<IProps, IState> {
opts.from = this.nextBatch;
}

const { nextBatch } = await this.state.thread.fetchEvents(opts);

this.nextBatch = nextBatch;
let nextBatch: string | null | undefined = null;
if (this.state.thread) {
const response = await this.state.thread.fetchEvents(opts);
nextBatch = response.nextBatch;
this.nextBatch = nextBatch;
}

// Advances the marker on the TimelineWindow to define the correct
// window of events to display on screen
timelineWindow.extend(direction, limit);
timelineWindow?.extend(direction, limit);

return !!nextBatch;
};

private onFileDrop = (dataTransfer: DataTransfer) => {
ContentMessages.sharedInstance().sendContentListToRoom(
Array.from(dataTransfer.files),
this.props.mxEvent.getRoomId(),
this.threadRelation,
MatrixClientPeg.get(),
TimelineRenderingType.Thread,
);
const roomId = this.props.mxEvent.getRoomId();
if (roomId) {
ContentMessages.sharedInstance().sendContentListToRoom(
Array.from(dataTransfer.files),
roomId,
this.threadRelation,
MatrixClientPeg.get(),
TimelineRenderingType.Thread,
);
} else {
console.warn("Unknwon roomId for event", this.props.mxEvent);
}
};

private get threadRelation(): IEventRelation {
const lastThreadReply = this.state.thread?.lastReply((ev: MatrixEvent) => {
return ev.isRelation(THREAD_RELATION_TYPE.name) && !ev.status;
});

return {
const relation = {
"rel_type": THREAD_RELATION_TYPE.name,
"event_id": this.state.thread?.id,
"is_falling_back": true,
"m.in_reply_to": {
"event_id": lastThreadReply?.getId() ?? this.state.thread?.id,
},
};

const fallbackEventId = this.state.lastReply?.getId() ?? this.state.thread?.id;
if (fallbackEventId) {
relation["m.in_reply_to"] = {
"event_id": fallbackEventId,
};
}

return relation;
}

private renderThreadViewHeader = (): JSX.Element => {
Expand All @@ -314,7 +365,7 @@ export default class ThreadView extends React.Component<IProps, IState> {

const threadRelation = this.threadRelation;

let timeline: JSX.Element;
let timeline: JSX.Element | null;
if (this.state.thread) {
if (this.props.initialEvent && this.props.initialEvent.getRoomId() !== this.state.thread.roomId) {
logger.warn("ThreadView attempting to render TimelinePanel with mismatched initialEvent",
Expand Down
34 changes: 20 additions & 14 deletions src/components/views/context_menus/ThreadListContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import { WidgetLayoutStore } from "../../../stores/widgets/WidgetLayoutStore";
import { MatrixClientPeg } from "../../../MatrixClientPeg";
import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload";

interface IProps {
export interface ThreadListContextMenuProps {
mxEvent: MatrixEvent;
permalinkCreator: RoomPermalinkCreator;
permalinkCreator?: RoomPermalinkCreator;
onMenuToggle?: (open: boolean) => void;
}

Expand All @@ -43,7 +43,7 @@ const contextMenuBelow = (elementRect: DOMRect) => {
return { left, top, chevronFace };
};

const ThreadListContextMenu: React.FC<IProps> = ({
const ThreadListContextMenu: React.FC<ThreadListContextMenuProps> = ({
mxEvent,
permalinkCreator,
onMenuToggle,
Expand All @@ -64,12 +64,14 @@ const ThreadListContextMenu: React.FC<IProps> = ({
closeThreadOptions();
}, [mxEvent, closeThreadOptions]);

const copyLinkToThread = useCallback(async (evt: ButtonEvent) => {
evt.preventDefault();
evt.stopPropagation();
const matrixToUrl = permalinkCreator.forEvent(mxEvent.getId());
await copyPlaintext(matrixToUrl);
closeThreadOptions();
const copyLinkToThread = useCallback(async (evt: ButtonEvent | undefined) => {
if (permalinkCreator) {
evt?.preventDefault();
evt?.stopPropagation();
const matrixToUrl = permalinkCreator.forEvent(mxEvent.getId());
await copyPlaintext(matrixToUrl);
closeThreadOptions();
}
}, [mxEvent, closeThreadOptions, permalinkCreator]);

useEffect(() => {
Expand All @@ -87,6 +89,7 @@ const ThreadListContextMenu: React.FC<IProps> = ({
title={_t("Thread options")}
isExpanded={menuDisplayed}
inputRef={button}
data-testid="threadlist-dropdown-button"
/>
{ menuDisplayed && (<IconizedContextMenu
onFinished={closeThreadOptions}
Expand All @@ -102,11 +105,14 @@ const ThreadListContextMenu: React.FC<IProps> = ({
label={_t("View in room")}
iconClassName="mx_ThreadPanel_viewInRoom"
/> }
<IconizedContextMenuOption
onClick={(e) => copyLinkToThread(e)}
label={_t("Copy link to thread")}
iconClassName="mx_ThreadPanel_copyLinkToThread"
/>
{ permalinkCreator &&
<IconizedContextMenuOption
data-testid="copy-thread-link"
onClick={(e) => copyLinkToThread(e)}
label={_t("Copy link to thread")}
iconClassName="mx_ThreadPanel_copyLinkToThread"
/>
}
</IconizedContextMenuOptionList>
</IconizedContextMenu>) }
</React.Fragment>;
Expand Down
1 change: 1 addition & 0 deletions src/components/views/elements/Spinner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export default class Spinner extends React.PureComponent<IProps> {
style={{ width: w, height: h }}
aria-label={_t("Loading...")}
role="progressbar"
data-testid="spinner"
/>
</div>
);
Expand Down
6 changes: 3 additions & 3 deletions src/components/views/elements/Tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type State = Partial<Pick<CSSProperties, "display" | "right" | "top" | "transfor

export default class Tooltip extends React.PureComponent<ITooltipProps, State> {
private static container: HTMLElement;
private parent: Element;
private parent: Element | null = null;

// XXX: This is because some components (Field) are unable to `import` the Tooltip class,
// so we expose the Alignment options off of us statically.
Expand Down Expand Up @@ -87,7 +87,7 @@ export default class Tooltip extends React.PureComponent<ITooltipProps, State> {
capture: true,
});

this.parent = ReactDOM.findDOMNode(this).parentNode as Element;
this.parent = ReactDOM.findDOMNode(this)?.parentNode as Element ?? null;

this.updatePosition();
}
Expand All @@ -109,7 +109,7 @@ export default class Tooltip extends React.PureComponent<ITooltipProps, State> {
// positioned, also taking into account any window zoom
private updatePosition = (): void => {
// When the tooltip is hidden, no need to thrash the DOM with `style` attribute updates (performance)
if (!this.props.visible) return;
if (!this.props.visible || !this.parent) return;

const parentBox = this.parent.getBoundingClientRect();
const width = UIStore.instance.windowWidth;
Expand Down
1 change: 1 addition & 0 deletions src/components/views/rooms/BasicMessageComposer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,7 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
aria-activedescendant={activeDescendant}
dir="auto"
aria-disabled={this.props.disabled}
data-testid="basicmessagecomposer"
/>
</div>);
}
Expand Down
1 change: 1 addition & 0 deletions src/components/views/rooms/MessageComposer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ function SendButton(props: ISendButtonProps) {
className="mx_MessageComposer_sendMessage"
onClick={props.onClick}
title={props.title ?? _t('Send message')}
data-testid="sendmessagebtn"
/>
);
}
Expand Down
Loading