From d4cf3881bc1cfe6e4dba48bdb506120ecfe90fd2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 22 Oct 2024 12:58:45 +0100 Subject: [PATCH] Switch away from deprecated ReactDOM findDOMNode (#28259) * Remove unused method getVisibleDecryptionFailures Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Switch away from ReactDOM findDOMNode Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/NodeAnimator.tsx | 30 +++++---- src/components/structures/MessagePanel.tsx | 7 +- src/components/structures/ScrollPanel.tsx | 2 +- src/components/structures/TimelinePanel.tsx | 64 ++++--------------- src/components/views/messages/TextualBody.tsx | 19 ++++-- src/components/views/rooms/Autocomplete.tsx | 11 +++- .../structures/TimelinePanel-test.tsx | 25 ++++++++ 7 files changed, 77 insertions(+), 81 deletions(-) diff --git a/src/NodeAnimator.tsx b/src/NodeAnimator.tsx index b5abcd2440..3ca098311f 100644 --- a/src/NodeAnimator.tsx +++ b/src/NodeAnimator.tsx @@ -6,12 +6,11 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ -import React, { Key, MutableRefObject, ReactElement, ReactInstance } from "react"; -import ReactDom from "react-dom"; +import React, { Key, MutableRefObject, ReactElement, RefCallback } from "react"; interface IChildProps { style: React.CSSProperties; - ref: (node: React.ReactInstance) => void; + ref: RefCallback; } interface IProps { @@ -36,7 +35,7 @@ function isReactElement(c: ReturnType<(typeof React.Children)["toArray"]>[number * automatic positional animation, look at react-shuffle or similar libraries. */ export default class NodeAnimator extends React.Component { - private nodes: Record = {}; + private nodes: Record = {}; private children: { [key: string]: ReactElement } = {}; public static defaultProps: Partial = { startStyles: [], @@ -71,10 +70,10 @@ export default class NodeAnimator extends React.Component { if (!isReactElement(c)) return; if (oldChildren[c.key!]) { const old = oldChildren[c.key!]; - const oldNode = ReactDom.findDOMNode(this.nodes[old.key!]); + const oldNode = this.nodes[old.key!]; - if (oldNode && (oldNode as HTMLElement).style.left !== c.props.style.left) { - this.applyStyles(oldNode as HTMLElement, { left: c.props.style.left }); + if (oldNode && oldNode.style.left !== c.props.style.left) { + this.applyStyles(oldNode, { left: c.props.style.left }); } // clone the old element with the props (and children) of the new element // so prop updates are still received by the children. @@ -98,26 +97,29 @@ export default class NodeAnimator extends React.Component { }); } - private collectNode(k: Key, node: React.ReactInstance, restingStyle: React.CSSProperties): void { + private collectNode(k: Key, domNode: HTMLElement | null, restingStyle: React.CSSProperties): void { const key = typeof k === "bigint" ? Number(k) : k; - if (node && this.nodes[key] === undefined && this.props.startStyles.length > 0) { + if (domNode && this.nodes[key] === undefined && this.props.startStyles.length > 0) { const startStyles = this.props.startStyles; - const domNode = ReactDom.findDOMNode(node); // start from startStyle 1: 0 is the one we gave it // to start with, so now we animate 1 etc. for (let i = 1; i < startStyles.length; ++i) { - this.applyStyles(domNode as HTMLElement, startStyles[i]); + this.applyStyles(domNode, startStyles[i]); } // and then we animate to the resting state window.setTimeout(() => { - this.applyStyles(domNode as HTMLElement, restingStyle); + this.applyStyles(domNode, restingStyle); }, 0); } - this.nodes[key] = node; + if (domNode) { + this.nodes[key] = domNode; + } else { + delete this.nodes[key]; + } if (this.props.innerRef) { - this.props.innerRef.current = node; + this.props.innerRef.current = domNode; } } diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 97b8bc9c02..7383e06f07 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -7,7 +7,6 @@ Please see LICENSE files in the repository root for full details. */ import React, { createRef, ReactNode, TransitionEvent } from "react"; -import ReactDOM from "react-dom"; import classNames from "classnames"; import { Room, MatrixClient, RoomStateEvent, EventStatus, MatrixEvent, EventType } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; @@ -245,7 +244,7 @@ export default class MessagePanel extends React.Component { private readMarkerNode = createRef(); private whoIsTyping = createRef(); - private scrollPanel = createRef(); + public scrollPanel = createRef(); private readonly showTypingNotificationsWatcherRef: string; private eventTiles: Record = {}; @@ -376,13 +375,13 @@ export default class MessagePanel extends React.Component { // +1: read marker is below the window public getReadMarkerPosition(): number | null { const readMarker = this.readMarkerNode.current; - const messageWrapper = this.scrollPanel.current; + const messageWrapper = this.scrollPanel.current?.divScroll; if (!readMarker || !messageWrapper) { return null; } - const wrapperRect = (ReactDOM.findDOMNode(messageWrapper) as HTMLElement).getBoundingClientRect(); + const wrapperRect = messageWrapper.getBoundingClientRect(); const readMarkerRect = readMarker.getBoundingClientRect(); // the read-marker pretends to have zero height when it is actually diff --git a/src/components/structures/ScrollPanel.tsx b/src/components/structures/ScrollPanel.tsx index 6c0da3018f..d072c322ce 100644 --- a/src/components/structures/ScrollPanel.tsx +++ b/src/components/structures/ScrollPanel.tsx @@ -186,7 +186,7 @@ export default class ScrollPanel extends React.Component { private bottomGrowth!: number; private minListHeight!: number; private heightUpdateInProgress = false; - private divScroll: HTMLDivElement | null = null; + public divScroll: HTMLDivElement | null = null; public constructor(props: IProps) { super(props); diff --git a/src/components/structures/TimelinePanel.tsx b/src/components/structures/TimelinePanel.tsx index b55709e8c2..846fc56d17 100644 --- a/src/components/structures/TimelinePanel.tsx +++ b/src/components/structures/TimelinePanel.tsx @@ -7,7 +7,6 @@ Please see LICENSE files in the repository root for full details. */ import React, { createRef, ReactNode } from "react"; -import ReactDOM from "react-dom"; import { Room, RoomEvent, @@ -67,9 +66,6 @@ const READ_RECEIPT_INTERVAL_MS = 500; const READ_MARKER_DEBOUNCE_MS = 100; -// How far off-screen a decryption failure can be for it to still count as "visible" -const VISIBLE_DECRYPTION_FAILURE_MARGIN = 100; - const debuglog = (...args: any[]): void => { if (SettingsStore.getValue("debug_timeline_panel")) { logger.log.call(console, "TimelinePanel debuglog:", ...args); @@ -398,6 +394,10 @@ class TimelinePanel extends React.Component { } } + private get messagePanelDiv(): HTMLDivElement | null { + return this.messagePanel.current?.scrollPanel.current?.divScroll ?? null; + } + /** * Logs out debug info to describe the state of the TimelinePanel and the * events in the room according to the matrix-js-sdk. This is useful when @@ -418,15 +418,12 @@ class TimelinePanel extends React.Component { // And we can suss out any corrupted React `key` problems. let renderedEventIds: string[] | undefined; try { - const messagePanel = this.messagePanel.current; - if (messagePanel) { - const messagePanelNode = ReactDOM.findDOMNode(messagePanel) as Element; - if (messagePanelNode) { - const actuallyRenderedEvents = messagePanelNode.querySelectorAll("[data-event-id]"); - renderedEventIds = [...actuallyRenderedEvents].map((renderedEvent) => { - return renderedEvent.getAttribute("data-event-id")!; - }); - } + const messagePanelNode = this.messagePanelDiv; + if (messagePanelNode) { + const actuallyRenderedEvents = messagePanelNode.querySelectorAll("[data-event-id]"); + renderedEventIds = [...actuallyRenderedEvents].map((renderedEvent) => { + return renderedEvent.getAttribute("data-event-id")!; + }); } } catch (err) { logger.error(`onDumpDebugLogs: Failed to get the actual event ID's in the DOM`, err); @@ -1766,45 +1763,6 @@ class TimelinePanel extends React.Component { return index > -1 ? index : null; } - /** - * Get a list of undecryptable events currently visible on-screen. - * - * @param {boolean} addMargin Whether to add an extra margin beyond the viewport - * where events are still considered "visible" - * - * @returns {MatrixEvent[] | null} A list of undecryptable events, or null if - * the list of events could not be determined. - */ - public getVisibleDecryptionFailures(addMargin?: boolean): MatrixEvent[] | null { - const messagePanel = this.messagePanel.current; - if (!messagePanel) return null; - - const messagePanelNode = ReactDOM.findDOMNode(messagePanel) as Element; - if (!messagePanelNode) return null; // sometimes this happens for fresh rooms/post-sync - const wrapperRect = messagePanelNode.getBoundingClientRect(); - const margin = addMargin ? VISIBLE_DECRYPTION_FAILURE_MARGIN : 0; - const screenTop = wrapperRect.top - margin; - const screenBottom = wrapperRect.bottom + margin; - - const result: MatrixEvent[] = []; - for (const ev of this.state.liveEvents) { - const eventId = ev.getId(); - if (!eventId) continue; - const node = messagePanel.getNodeForEventId(eventId); - if (!node) continue; - - const boundingRect = node.getBoundingClientRect(); - if (boundingRect.top > screenBottom) { - // we have gone past the visible section of timeline - break; - } else if (boundingRect.bottom >= screenTop) { - // the tile for this event is in the visible part of the screen (or just above/below it). - if (ev.isDecryptionFailure()) result.push(ev); - } - } - return result; - } - private getLastDisplayedEventIndex(opts: IEventIndexOpts = {}): number | null { const ignoreOwn = opts.ignoreOwn || false; const allowPartial = opts.allowPartial || false; @@ -1812,7 +1770,7 @@ class TimelinePanel extends React.Component { const messagePanel = this.messagePanel.current; if (!messagePanel) return null; - const messagePanelNode = ReactDOM.findDOMNode(messagePanel) as Element; + const messagePanelNode = this.messagePanelDiv; if (!messagePanelNode) return null; // sometimes this happens for fresh rooms/post-sync const wrapperRect = messagePanelNode.getBoundingClientRect(); const myUserId = MatrixClientPeg.safeGet().credentials.userId; diff --git a/src/components/views/messages/TextualBody.tsx b/src/components/views/messages/TextualBody.tsx index 08b7991807..0e0c29747f 100644 --- a/src/components/views/messages/TextualBody.tsx +++ b/src/components/views/messages/TextualBody.tsx @@ -52,6 +52,8 @@ export default class TextualBody extends React.Component { private tooltips: Element[] = []; private reactRoots: Element[] = []; + private ref = createRef(); + public static contextType = RoomContext; public declare context: React.ContextType; @@ -84,8 +86,8 @@ export default class TextualBody extends React.Component { if (this.props.mxEvent.getContent().format === "org.matrix.custom.html") { // Handle expansion and add buttons - const pres = (ReactDOM.findDOMNode(this) as Element).getElementsByTagName("pre"); - if (pres.length > 0) { + const pres = this.ref.current?.getElementsByTagName("pre"); + if (pres && pres.length > 0) { for (let i = 0; i < pres.length; i++) { // If there already is a div wrapping the codeblock we want to skip this. // This happens after the codeblock was edited. @@ -477,7 +479,12 @@ export default class TextualBody extends React.Component { if (isEmote) { return ( -
+
*  {mxEvent.sender ? mxEvent.sender.name : mxEvent.getSender()} @@ -490,7 +497,7 @@ export default class TextualBody extends React.Component { } if (isNotice) { return ( -
+
{body} {widgets}
@@ -498,14 +505,14 @@ export default class TextualBody extends React.Component { } if (isCaption) { return ( -
+
{body} {widgets}
); } return ( -
+
{body} {widgets}
diff --git a/src/components/views/rooms/Autocomplete.tsx b/src/components/views/rooms/Autocomplete.tsx index db4ac37415..af33fb2d9e 100644 --- a/src/components/views/rooms/Autocomplete.tsx +++ b/src/components/views/rooms/Autocomplete.tsx @@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ -import React, { createRef, KeyboardEvent } from "react"; +import React, { createRef, KeyboardEvent, RefObject } from "react"; import classNames from "classnames"; import { flatMap } from "lodash"; import { Room } from "matrix-js-sdk/src/matrix"; @@ -45,6 +45,7 @@ export default class Autocomplete extends React.PureComponent { public queryRequested?: string; public debounceCompletionsRequest?: number; private containerRef = createRef(); + private completionRefs: Record> = {}; public static contextType = RoomContext; public declare context: React.ContextType; @@ -260,7 +261,7 @@ export default class Autocomplete extends React.PureComponent { public componentDidUpdate(prevProps: IProps): void { this.applyNewProps(prevProps.query, prevProps.room); // this is the selected completion, so scroll it into view if needed - const selectedCompletion = this.refs[`completion${this.state.selectionOffset}`] as HTMLElement; + const selectedCompletion = this.completionRefs[`completion${this.state.selectionOffset}`]?.current; if (selectedCompletion) { selectedCompletion.scrollIntoView({ @@ -286,9 +287,13 @@ export default class Autocomplete extends React.PureComponent { this.onCompletionClicked(componentPosition); }; + const refId = `completion${componentPosition}`; + if (!this.completionRefs[refId]) { + this.completionRefs[refId] = createRef(); + } return React.cloneElement(completion.component, { "key": j, - "ref": `completion${componentPosition}`, + "ref": this.completionRefs[refId], "id": generateCompletionDomId(componentPosition - 1), // 0 index the completion IDs className, onClick, diff --git a/test/unit-tests/components/structures/TimelinePanel-test.tsx b/test/unit-tests/components/structures/TimelinePanel-test.tsx index 7de688bfe6..4a66351779 100644 --- a/test/unit-tests/components/structures/TimelinePanel-test.tsx +++ b/test/unit-tests/components/structures/TimelinePanel-test.tsx @@ -41,6 +41,8 @@ import { mkThread } from "../../../test-utils/threads"; import { createMessageEventContent } from "../../../test-utils/events"; import SettingsStore from "../../../../src/settings/SettingsStore"; import ScrollPanel from "../../../../src/components/structures/ScrollPanel"; +import defaultDispatcher from "../../../../src/dispatcher/dispatcher"; +import { Action } from "../../../../src/dispatcher/actions"; // ScrollPanel calls this, but jsdom doesn't mock it for us HTMLDivElement.prototype.scrollBy = () => {}; @@ -1002,4 +1004,27 @@ describe("TimelinePanel", () => { await waitFor(() => expect(screen.queryByRole("progressbar")).toBeNull()); await waitFor(() => expect(container.querySelector(".mx_RoomView_MessageList")).not.toBeEmptyDOMElement()); }); + + it("should dump debug logs on Action.DumpDebugLogs", async () => { + const spy = jest.spyOn(console, "debug"); + + const [, room, events] = setupTestData(); + const eventsPage2 = events.slice(1, 2); + + // Start with only page 2 of the main events in the window + const [, timelineSet] = mkTimeline(room, eventsPage2); + room.getTimelineSets = jest.fn().mockReturnValue([timelineSet]); + + await withScrollPanelMountSpy(async () => { + const { container } = render(); + + await waitFor(() => expectEvents(container, [events[1]])); + }); + + defaultDispatcher.fire(Action.DumpDebugLogs); + + await waitFor(() => + expect(spy).toHaveBeenCalledWith(expect.stringContaining("TimelinePanel(Room): Debugging info for roomId")), + ); + }); });