imterah/element-portable
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix threads fallback incorrectly targets root event (#9229)

Browse source 
* Use RelationType enum instead of hardcoded value

* Fix threads replies fallback to target last reply

* Only unsubscribe from threads events if needed

* fix strict null check

* fix strict null checks

* strict null checks

* fix typing

* Unsubscribe listeners if new thread is `null`

Co-authored-by: Faye Duxovni <fayed@element.io>

* Update strict null checks

* Type HTMLElement as nullable

* Add thread fallback integration test

* lint fix

* Update snapshots

* Add test after changing thread

* Remove test comment

* update snapshot

* fix room context test utility

* Add ThreadListContextMenu test

* lint fix

* fix thread rendering

Co-authored-by: Faye Duxovni <fayed@element.io>
Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
This commit is contained in:
Germain 2022-10-20 12:48:25 +01:00 • committed by GitHub
parent d898af820b
commit be281fd735
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 453 additions and 103 deletions
Download patch file Download diff file Expand all files Collapse all files

18
src/components/structures/FileDropTarget.tsx
View file

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

111
src/components/structures/ThreadView.tsx
View file

@ -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';
@ -70,6 +70,7 @@ interface IProps {
interface IState {
thread?: Thread;
lastReply?: MatrixEvent | null;
layout: Layout;
editState?: EditorStateTransfer;
replyToEvent?: MatrixEvent;
@ -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]) =>
@ -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);
@ -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()) {
@ -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;
}
@ -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 => {
@ -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",

34
src/components/views/context_menus/ThreadListContextMenu.tsx
View file

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

1
src/components/views/elements/Spinner.tsx
View file

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

6
src/components/views/elements/Tooltip.tsx
View file

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

1
src/components/views/rooms/BasicMessageComposer.tsx
View file

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

1
src/components/views/rooms/MessageComposer.tsx
View file

@ -73,6 +73,7 @@ function SendButton(props: ISendButtonProps) {
className="mx_MessageComposer_sendMessage"
onClick={props.onClick}
title={props.title ?? _t('Send message')}
data-testid="sendmessagebtn"
/>
);
}