From e3e7d41d5cbaa1e9a8cda81e61be063d71f85fe2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sat, 10 Jul 2021 19:41:50 +0100 Subject: [PATCH 1/2] only consider valid & loaded url previews for show N more prompt --- .../views/rooms/LinkPreviewGroup.tsx | 34 ++++++++++++----- .../views/rooms/LinkPreviewWidget.tsx | 37 +++---------------- 2 files changed, 29 insertions(+), 42 deletions(-) diff --git a/src/components/views/rooms/LinkPreviewGroup.tsx b/src/components/views/rooms/LinkPreviewGroup.tsx index ff6fd4afd2..2541b2e375 100644 --- a/src/components/views/rooms/LinkPreviewGroup.tsx +++ b/src/components/views/rooms/LinkPreviewGroup.tsx @@ -14,43 +14,57 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useEffect } from "react"; +import React, { useContext, useEffect } from "react"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; +import { IPreviewUrlResponse } from "matrix-js-sdk/src/client"; import { useStateToggle } from "../../../hooks/useStateToggle"; import LinkPreviewWidget from "./LinkPreviewWidget"; import AccessibleButton from "../elements/AccessibleButton"; import { _t } from "../../../languageHandler"; +import MatrixClientContext from "../../../contexts/MatrixClientContext"; +import { useAsyncMemo } from "../../../hooks/useAsyncMemo"; const INITIAL_NUM_PREVIEWS = 2; interface IProps { links: string[]; // the URLs to be previewed mxEvent: MatrixEvent; // the Event associated with the preview - onCancelClick?(): void; // called when the preview's cancel ('hide') button is clicked - onHeightChanged?(): void; // called when the preview's contents has loaded + onCancelClick(): void; // called when the preview's cancel ('hide') button is clicked + onHeightChanged(): void; // called when the preview's contents has loaded } const LinkPreviewGroup: React.FC = ({ links, mxEvent, onCancelClick, onHeightChanged }) => { + const cli = useContext(MatrixClientContext); const [expanded, toggleExpanded] = useStateToggle(); + + const ts = mxEvent.getTs(); + const previews = useAsyncMemo<[string, IPreviewUrlResponse][]>(async () => { + return Promise.all<[string, IPreviewUrlResponse] | void>(links.map(link => { + return cli.getUrlPreview(link, ts).then(preview => [link, preview], error => { + console.error("Failed to get URL preview: " + error); + }); + })).then(a => a.filter(Boolean)) as Promise<[string, IPreviewUrlResponse][]>; + }, [links, ts], []); + useEffect(() => { onHeightChanged(); - }, [onHeightChanged, expanded]); + }, [onHeightChanged, expanded, previews]); - const shownLinks = expanded ? links : links.slice(0, INITIAL_NUM_PREVIEWS); + const showPreviews = expanded ? previews : previews.slice(0, INITIAL_NUM_PREVIEWS); - let toggleButton; - if (links.length > INITIAL_NUM_PREVIEWS) { + let toggleButton: JSX.Element; + if (previews.length > INITIAL_NUM_PREVIEWS) { toggleButton = { expanded ? _t("Collapse") - : _t("Show %(count)s other previews", { count: links.length - shownLinks.length }) } + : _t("Show %(count)s other previews", { count: previews.length - showPreviews.length }) } ; } return
- { shownLinks.map((link, i) => ( - + { showPreviews.map(([link, preview], i) => ( + { i === 0 ? ( { - private unmounted = false; +export default class LinkPreviewWidget extends React.Component { private readonly description = createRef(); - constructor(props) { - super(props); - - this.state = { - preview: null, - }; - - MatrixClientPeg.get().getUrlPreview(this.props.link, this.props.mxEvent.getTs()).then((preview) => { - if (this.unmounted) { - return; - } - this.setState({ preview }, this.props.onHeightChanged); - }, (error) => { - console.error("Failed to get URL preview: " + error); - }); - } - componentDidMount() { if (this.description.current) { linkifyElement(this.description.current); @@ -72,12 +49,8 @@ export default class LinkPreviewWidget extends React.Component { } } - componentWillUnmount() { - this.unmounted = true; - } - private onImageClick = ev => { - const p = this.state.preview; + const p = this.props.preview; if (ev.button != 0 || ev.metaKey) return; ev.preventDefault(); @@ -99,7 +72,7 @@ export default class LinkPreviewWidget extends React.Component { }; render() { - const p = this.state.preview; + const p = this.props.preview; if (!p || Object.keys(p).length === 0) { return
; } From f5f4be88f020eca02a59122758518fe978c5e30d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 12 Jul 2021 08:34:26 +0100 Subject: [PATCH 2/2] Update tests to expect LinkPreviewGroup behaviour --- .../views/messages/TextualBody-test.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/test/components/views/messages/TextualBody-test.js b/test/components/views/messages/TextualBody-test.js index c6a3f3c779..fd11a9d46b 100644 --- a/test/components/views/messages/TextualBody-test.js +++ b/test/components/views/messages/TextualBody-test.js @@ -22,8 +22,10 @@ import sdk from "../../../skinned-sdk"; import { mkEvent, mkStubRoom } from "../../../test-utils"; import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; import * as languageHandler from "../../../../src/languageHandler"; +import * as TestUtils from "../../../test-utils"; -const TextualBody = sdk.getComponent("views.messages.TextualBody"); +const _TextualBody = sdk.getComponent("views.messages.TextualBody"); +const TextualBody = TestUtils.wrapInMatrixClientContext(_TextualBody); configure({ adapter: new Adapter() }); @@ -305,10 +307,9 @@ describe("", () => { const wrapper = mount( {}} />); expect(wrapper.text()).toBe(ev.getContent().body); - let widgets = wrapper.find("LinkPreviewWidget"); - // at this point we should have exactly one widget - expect(widgets.length).toBe(1); - expect(widgets.at(0).prop("link")).toBe("https://matrix.org/"); + let widgets = wrapper.find("LinkPreviewGroup"); + // at this point we should have exactly one link + expect(widgets.at(0).prop("links")).toEqual(["https://matrix.org/"]); // simulate an event edit and check the transition from the old URL preview to the new one const ev2 = mkEvent({ @@ -333,11 +334,9 @@ describe("", () => { // XXX: this is to give TextualBody enough time for state to settle wrapper.setState({}, () => { - widgets = wrapper.find("LinkPreviewWidget"); - // at this point we should have exactly two widgets (not the matrix.org one anymore) - expect(widgets.length).toBe(2); - expect(widgets.at(0).prop("link")).toBe("https://vector.im/"); - expect(widgets.at(1).prop("link")).toBe("https://riot.im/"); + widgets = wrapper.find("LinkPreviewGroup"); + // at this point we should have exactly two links (not the matrix.org one anymore) + expect(widgets.at(0).prop("links")).toEqual(["https://vector.im/", "https://riot.im/"]); }); }); });