From 6deb6a7bfeb88897167ef8795802644eab3be9d0 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 14 Nov 2022 15:24:11 +0000 Subject: [PATCH 1/7] Extract Search handling from RoomView into its own Component --- src/Searching.ts | 39 ++- src/components/structures/RoomSearchView.tsx | 260 ++++++++++++++++ src/components/structures/RoomView.tsx | 305 ++++--------------- src/components/views/rooms/RoomHeader.tsx | 19 +- 4 files changed, 353 insertions(+), 270 deletions(-) create mode 100644 src/components/structures/RoomSearchView.tsx diff --git a/src/Searching.ts b/src/Searching.ts index fcbc7d00075..4652797bfa4 100644 --- a/src/Searching.ts +++ b/src/Searching.ts @@ -35,6 +35,7 @@ const SEARCH_LIMIT = 10; async function serverSideSearch( term: string, roomId: string = undefined, + abortSignal?: AbortSignal, ): Promise<{ response: ISearchResponse, query: ISearchRequestBody }> { const client = MatrixClientPeg.get(); @@ -59,19 +60,24 @@ async function serverSideSearch( }, }; - const response = await client.search({ body: body }); + const response = await client.search({ body: body }, abortSignal); return { response, query: body }; } -async function serverSideSearchProcess(term: string, roomId: string = undefined): Promise { +async function serverSideSearchProcess( + term: string, + roomId: string = undefined, + abortSignal?: AbortSignal, +): Promise { const client = MatrixClientPeg.get(); - const result = await serverSideSearch(term, roomId); + const result = await serverSideSearch(term, roomId, abortSignal); // The js-sdk method backPaginateRoomEventsSearch() uses _query internally // so we're reusing the concept here since we want to delegate the // pagination back to backPaginateRoomEventsSearch() in some cases. const searchResults: ISearchResults = { + abortSignal, _query: result.query, results: [], highlights: [], @@ -90,12 +96,12 @@ function compareEvents(a: ISearchResult, b: ISearchResult): number { return 0; } -async function combinedSearch(searchTerm: string): Promise { +async function combinedSearch(searchTerm: string, abortSignal?: AbortSignal): Promise { const client = MatrixClientPeg.get(); // Create two promises, one for the local search, one for the // server-side search. - const serverSidePromise = serverSideSearch(searchTerm); + const serverSidePromise = serverSideSearch(searchTerm, undefined, abortSignal); const localPromise = localSearch(searchTerm); // Wait for both promises to resolve. @@ -575,7 +581,11 @@ async function combinedPagination(searchResult: ISeshatSearchResults): Promise { +function eventIndexSearch( + term: string, + roomId: string = undefined, + abortSignal?: AbortSignal, +): Promise { let searchPromise: Promise; if (roomId !== undefined) { @@ -586,12 +596,12 @@ function eventIndexSearch(term: string, roomId: string = undefined): Promise { +export default function eventSearch( + term: string, + roomId: string = undefined, + abortSignal?: AbortSignal, +): Promise { const eventIndex = EventIndexPeg.get(); - if (eventIndex === null) return serverSideSearchProcess(term, roomId); - else return eventIndexSearch(term, roomId); + if (eventIndex === null) { + return serverSideSearchProcess(term, roomId, abortSignal); + } else { + return eventIndexSearch(term, roomId, abortSignal); + } } diff --git a/src/components/structures/RoomSearchView.tsx b/src/components/structures/RoomSearchView.tsx new file mode 100644 index 00000000000..a0e6c215fe9 --- /dev/null +++ b/src/components/structures/RoomSearchView.tsx @@ -0,0 +1,260 @@ +/* +Copyright 2015, 2016 OpenMarket Ltd +Copyright 2017 Vector Creations Ltd +Copyright 2018, 2019 New Vector Ltd +Copyright 2019 - 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React, { forwardRef, RefObject, useCallback, useContext, useEffect, useRef, useState } from 'react'; +import { ISearchResults } from 'matrix-js-sdk/src/@types/search'; +import { IThreadBundledRelationship } from 'matrix-js-sdk/src/models/event'; +import { THREAD_RELATION_TYPE } from 'matrix-js-sdk/src/models/thread'; +import { logger } from "matrix-js-sdk/src/logger"; + +import ScrollPanel from "./ScrollPanel"; +import { SearchScope } from "../views/rooms/SearchBar"; +import Spinner from "../views/elements/Spinner"; +import { _t } from "../../languageHandler"; +import { haveRendererForEvent } from "../../events/EventTileFactory"; +import SearchResultTile from "../views/rooms/SearchResultTile"; +import { searchPagination } from "../../Searching"; +import Modal from "../../Modal"; +import ErrorDialog from "../views/dialogs/ErrorDialog"; +import ResizeNotifier from "../../utils/ResizeNotifier"; +import MatrixClientContext from "../../contexts/MatrixClientContext"; +import { RoomPermalinkCreator } from "../../utils/permalinks/Permalinks"; +import RoomContext from "../../contexts/RoomContext"; + +const DEBUG = false; +let debuglog = function(msg: string) {}; + +if (DEBUG) { + // using bind means that we get to keep useful line numbers in the console + debuglog = logger.log.bind(console); +} + +interface Props { + term: string; + scope: SearchScope; + promise: Promise; + abortController?: AbortController; + resizeNotifier: ResizeNotifier; + permalinkCreator: RoomPermalinkCreator; + className: string; + onUpdate(inProgress: boolean, results: ISearchResults | null): void; +} + +export const RoomSearchView = forwardRef(({ + term, + scope, + promise, + abortController, + resizeNotifier, + permalinkCreator, + className, + onUpdate, +}: Props, ref: RefObject) => { + const client = useContext(MatrixClientContext); + const roomContext = useContext(RoomContext); + const [inProgress, setInProgress] = useState(true); + const [highlights, setHighlights] = useState(null); + const [results, setResults] = useState(null); + const aborted = useRef(false); + + const handleSearchResult = useCallback((searchPromise: Promise): Promise => { + setInProgress(true); + + return searchPromise.then(async (results) => { + debuglog("search complete"); + if (aborted.current) { + logger.error("Discarding stale search results"); + return false; + } + + // postgres on synapse returns us precise details of the strings + // which actually got matched for highlighting. + // + // In either case, we want to highlight the literal search term + // whether it was used by the search engine or not. + + let highlights = results.highlights; + if (highlights.indexOf(term) < 0) { + highlights = highlights.concat(term); + } + + // For overlapping highlights, + // favour longer (more specific) terms first + highlights = highlights.sort(function(a, b) { + return b.length - a.length; + }); + + if (client.supportsExperimentalThreads()) { + // Process all thread roots returned in this batch of search results + // XXX: This won't work for results coming from Seshat which won't include the bundled relationship + for (const result of results.results) { + for (const event of result.context.getTimeline()) { + const bundledRelationship = event + .getServerAggregatedRelation(THREAD_RELATION_TYPE.name); + if (!bundledRelationship || event.getThread()) continue; + const room = client.getRoom(event.getRoomId()); + const thread = room.findThreadForEvent(event); + if (thread) { + event.setThread(thread); + } else { + room.createThread(event.getId(), event, [], true); + } + } + } + } + + setHighlights(highlights); + setResults(results); + }, (error) => { + if (aborted.current) { + logger.error("Discarding stale search results"); + return false; + } + logger.error("Search failed", error); + Modal.createDialog(ErrorDialog, { + title: _t("Search failed"), + description: ((error && error.message) ? error.message : + _t("Server may be unavailable, overloaded, or search timed out :(")), + }); + return false; + }).finally(() => { + setInProgress(false); + }); + }, [client, term]); + + // Mount & unmount effect + useEffect(() => { + aborted.current = false; + handleSearchResult(promise); + return () => { + aborted.current = true; + abortController?.abort(); + }; + }, []); // eslint-disable-line react-hooks/exhaustive-deps + + // show searching spinner + if (results?.count === undefined) { + return ( +
+ ); + } + + const onSearchResultsFillRequest = (backwards: boolean): Promise => { + if (!backwards) { + return Promise.resolve(false); + } + + if (results.next_batch) { + debuglog("requesting more search results"); + const searchPromise = searchPagination(results); + return handleSearchResult(searchPromise); + } else { + debuglog("no more search results"); + return Promise.resolve(false); + } + }; + + // XXX: todo: merge overlapping results somehow? + // XXX: why doesn't searching on name work? + + const ret: JSX.Element[] = []; + + if (inProgress) { + ret.push(
  • + +
    • ); + } + + if (!results.next_batch) { + if (!results?.results?.length) { + ret.push(
  • +

    { _t("No results") }

    +
    • , + ); + } else { + ret.push(
  • +

    { _t("No more results") }

    +
    • , + ); + } + } + + // once dynamic content in the search results load, make the scrollPanel check + // the scroll offsets. + const onHeightChanged = () => { + const scrollPanel = ref.current; + scrollPanel?.checkScroll(); + }; + + let lastRoomId: string; + + for (let i = (results?.results?.length || 0) - 1; i >= 0; i--) { + const result = results.results[i]; + + const mxEv = result.context.getEvent(); + const roomId = mxEv.getRoomId(); + const room = client.getRoom(roomId); + if (!room) { + // if we do not have the room in js-sdk stores then hide it as we cannot easily show it + // As per the spec, an all rooms search can create this condition, + // it happens with Seshat but not Synapse. + // It will make the result count not match the displayed count. + logger.log("Hiding search result from an unknown room", roomId); + continue; + } + + if (!haveRendererForEvent(mxEv, roomContext.showHiddenEvents)) { + // XXX: can this ever happen? It will make the result count + // not match the displayed count. + continue; + } + + if (scope === SearchScope.All) { + if (roomId !== lastRoomId) { + ret.push(
  • +

    { _t("Room") }: { room.name }

    +
    • ); + lastRoomId = roomId; + } + } + + const resultLink = "#/room/"+roomId+"/"+mxEv.getId(); + + ret.push(); + } + + return ( + +
  • + { ret } + + ); +}); diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index 426912c5664..7f155010502 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -23,8 +23,7 @@ limitations under the License. import React, { createRef, ReactElement, ReactNode, RefObject, useContext } from 'react'; import classNames from 'classnames'; import { IRecommendedVersion, NotificationCountType, Room, RoomEvent } from "matrix-js-sdk/src/models/room"; -import { IThreadBundledRelationship, MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/models/event"; -import { ISearchResults } from 'matrix-js-sdk/src/@types/search'; +import { MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/models/event"; import { logger } from "matrix-js-sdk/src/logger"; import { EventTimeline } from 'matrix-js-sdk/src/models/event-timeline'; import { EventType } from 'matrix-js-sdk/src/@types/event'; @@ -37,6 +36,7 @@ import { ClientEvent } from "matrix-js-sdk/src/client"; import { CryptoEvent } from "matrix-js-sdk/src/crypto"; import { THREAD_RELATION_TYPE } from 'matrix-js-sdk/src/models/thread'; import { HistoryVisibility } from 'matrix-js-sdk/src/@types/partials'; +import { ISearchResults } from 'matrix-js-sdk/src/@types/search'; import shouldHideEvent from '../../shouldHideEvent'; import { _t } from '../../languageHandler'; @@ -47,7 +47,6 @@ import Modal from '../../Modal'; import { LegacyCallHandlerEvent } from '../../LegacyCallHandler'; import dis, { defaultDispatcher } from '../../dispatcher/dispatcher'; import * as Rooms from '../../Rooms'; -import eventSearch, { searchPagination } from '../../Searching'; import MainSplit from './MainSplit'; import RightPanel from './RightPanel'; import RoomScrollStateStore, { ScrollState } from '../../stores/RoomScrollStateStore'; @@ -67,8 +66,7 @@ import RoomPreviewCard from "../views/rooms/RoomPreviewCard"; import SearchBar, { SearchScope } from "../views/rooms/SearchBar"; import RoomUpgradeWarningBar from "../views/rooms/RoomUpgradeWarningBar"; import AuxPanel from "../views/rooms/AuxPanel"; -import RoomHeader from "../views/rooms/RoomHeader"; -import { XOR } from "../../@types/common"; +import RoomHeader, { ISearchInfo } from "../views/rooms/RoomHeader"; import { IOOBData, IThreepidInvite } from "../../stores/ThreepidInviteStore"; import EffectsOverlay from "../views/elements/EffectsOverlay"; import { containsEmoji } from '../../effects/utils'; @@ -84,8 +82,6 @@ import SpaceRoomView from "./SpaceRoomView"; import { IOpts } from "../../createRoom"; import EditorStateTransfer from "../../utils/EditorStateTransfer"; import ErrorDialog from '../views/dialogs/ErrorDialog'; -import SearchResultTile from '../views/rooms/SearchResultTile'; -import Spinner from "../views/elements/Spinner"; import UploadBar from './UploadBar'; import RoomStatusBar from "./RoomStatusBar"; import MessageComposer from '../views/rooms/MessageComposer'; @@ -103,7 +99,6 @@ import { DoAfterSyncPreparedPayload } from '../../dispatcher/payloads/DoAfterSyn import FileDropTarget from './FileDropTarget'; import Measured from '../views/elements/Measured'; import { FocusComposerPayload } from '../../dispatcher/payloads/FocusComposerPayload'; -import { haveRendererForEvent } from "../../events/EventTileFactory"; import { LocalRoom, LocalRoomState } from '../../models/LocalRoom'; import { createRoomFromLocalRoom } from '../../utils/direct-messages'; import NewRoomIntro from '../views/rooms/NewRoomIntro'; @@ -117,6 +112,8 @@ import { isVideoRoom } from '../../utils/video-rooms'; import { SDKContext } from '../../contexts/SDKContext'; import { CallStore, CallStoreEvent } from "../../stores/CallStore"; import { Call } from "../../models/Call"; +import { RoomSearchView } from './RoomSearchView'; +import eventSearch from "../../Searching"; const DEBUG = false; let debuglog = function(msg: string) {}; @@ -168,11 +165,7 @@ export interface IRoomState { initialEventScrollIntoView?: boolean; replyToEvent?: MatrixEvent; numUnreadMessages: number; - searchTerm?: string; - searchScope?: SearchScope; - searchResults?: XOR<{}, ISearchResults>; - searchHighlights?: string[]; - searchInProgress?: boolean; + search?: ISearchInfo; callState?: CallState; activeCall: Call | null; canPeek: boolean; @@ -368,7 +361,6 @@ export class RoomView extends React.Component { private unmounted = false; private permalinkCreators: Record = {}; - private searchId: number; private roomView = createRef(); private searchResultsPanel = createRef(); @@ -389,7 +381,6 @@ export class RoomView extends React.Component { shouldPeek: true, membersLoaded: !llMembers, numUnreadMessages: 0, - searchResults: null, callState: null, activeCall: null, canPeek: false, @@ -1025,7 +1016,7 @@ export class RoomView extends React.Component { break; case 'reply_to_event': if (!this.unmounted && - this.state.searchResults && + this.state.search && payload.event?.getRoomId() === this.state.roomId && payload.context === TimelineRenderingType.Search ) { @@ -1140,10 +1131,10 @@ export class RoomView extends React.Component { if (ev.getSender() !== this.context.client.credentials.userId) { // update unread count when scrolled up - if (!this.state.searchResults && this.state.atEndOfLiveTimeline) { + if (!this.state.search && this.state.atEndOfLiveTimeline) { // no change } else if (!shouldHideEvent(ev, this.state)) { - this.setState((state, props) => { + this.setState((state) => { return { numUnreadMessages: state.numUnreadMessages + 1 }; }); } @@ -1408,21 +1399,6 @@ export class RoomView extends React.Component { } } - private onSearchResultsFillRequest = (backwards: boolean): Promise => { - if (!backwards) { - return Promise.resolve(false); - } - - if (this.state.searchResults.next_batch) { - debuglog("requesting more search results"); - const searchPromise = searchPagination(this.state.searchResults as ISearchResults); - return this.handleSearchResult(searchPromise); - } else { - debuglog("no more search results"); - return Promise.resolve(false); - } - }; - private onInviteClick = () => { // open the room inviter dis.dispatch({ @@ -1506,187 +1482,34 @@ export class RoomView extends React.Component { } private onSearch = (term: string, scope: SearchScope) => { - this.setState({ - searchTerm: term, - searchScope: scope, - searchResults: {}, - searchHighlights: [], - }); - - // if we already have a search panel, we need to tell it to forget - // about its scroll state. - if (this.searchResultsPanel.current) { - this.searchResultsPanel.current.resetScrollState(); - } - - // make sure that we don't end up showing results from - // an aborted search by keeping a unique id. - // - // todo: should cancel any previous search requests. - this.searchId = new Date().getTime(); - - let roomId; - if (scope === SearchScope.Room) roomId = this.state.room.roomId; - + const roomId = scope === SearchScope.Room ? this.state.room.roomId : undefined; debuglog("sending search request"); - const searchPromise = eventSearch(term, roomId); - this.handleSearchResult(searchPromise); - }; - - private handleSearchResult(searchPromise: Promise): Promise { - // keep a record of the current search id, so that if the search terms - // change before we get a response, we can ignore the results. - const localSearchId = this.searchId; + const abortController = new AbortController(); + const promise = eventSearch(term, roomId, abortController.signal); this.setState({ - searchInProgress: true, + search: { + // make sure that we don't end up showing results from + // an aborted search by keeping a unique id. + searchId: new Date().getTime(), + roomId, + term, + scope, + promise, + abortController, + }, }); + }; - return searchPromise.then(async (results) => { - debuglog("search complete"); - if (this.unmounted || - this.state.timelineRenderingType !== TimelineRenderingType.Search || - this.searchId != localSearchId - ) { - logger.error("Discarding stale search results"); - return false; - } - - // postgres on synapse returns us precise details of the strings - // which actually got matched for highlighting. - // - // In either case, we want to highlight the literal search term - // whether it was used by the search engine or not. - - let highlights = results.highlights; - if (highlights.indexOf(this.state.searchTerm) < 0) { - highlights = highlights.concat(this.state.searchTerm); - } - - // For overlapping highlights, - // favour longer (more specific) terms first - highlights = highlights.sort(function(a, b) { - return b.length - a.length; - }); - - if (this.context.client.supportsExperimentalThreads()) { - // Process all thread roots returned in this batch of search results - // XXX: This won't work for results coming from Seshat which won't include the bundled relationship - for (const result of results.results) { - for (const event of result.context.getTimeline()) { - const bundledRelationship = event - .getServerAggregatedRelation(THREAD_RELATION_TYPE.name); - if (!bundledRelationship || event.getThread()) continue; - const room = this.context.client.getRoom(event.getRoomId()); - const thread = room.findThreadForEvent(event); - if (thread) { - event.setThread(thread); - } else { - room.createThread(event.getId(), event, [], true); - } - } - } - } - - this.setState({ - searchHighlights: highlights, - searchResults: results, - }); - }, (error) => { - logger.error("Search failed", error); - Modal.createDialog(ErrorDialog, { - title: _t("Search failed"), - description: ((error && error.message) ? error.message : - _t("Server may be unavailable, overloaded, or search timed out :(")), - }); - return false; - }).finally(() => { - this.setState({ - searchInProgress: false, - }); + private onSearchUpdate = (inProgress: boolean, searchResults: ISearchResults | null): void => { + this.setState({ + search: { + ...this.state.search, + count: searchResults?.count, + inProgress, + }, }); - } - - private getSearchResultTiles() { - // XXX: todo: merge overlapping results somehow? - // XXX: why doesn't searching on name work? - - const ret = []; - - if (this.state.searchInProgress) { - ret.push(
  • - -
    • ); - } - - if (!this.state.searchResults.next_batch) { - if (!this.state.searchResults?.results?.length) { - ret.push(
  • -

    { _t("No results") }

    -
    • , - ); - } else { - ret.push(
  • -

    { _t("No more results") }

    -
    • , - ); - } - } - - // once dynamic content in the search results load, make the scrollPanel check - // the scroll offsets. - const onHeightChanged = () => { - const scrollPanel = this.searchResultsPanel.current; - if (scrollPanel) { - scrollPanel.checkScroll(); - } - }; - - let lastRoomId; - - for (let i = (this.state.searchResults?.results?.length || 0) - 1; i >= 0; i--) { - const result = this.state.searchResults.results[i]; - - const mxEv = result.context.getEvent(); - const roomId = mxEv.getRoomId(); - const room = this.context.client.getRoom(roomId); - if (!room) { - // if we do not have the room in js-sdk stores then hide it as we cannot easily show it - // As per the spec, an all rooms search can create this condition, - // it happens with Seshat but not Synapse. - // It will make the result count not match the displayed count. - logger.log("Hiding search result from an unknown room", roomId); - continue; - } - - if (!haveRendererForEvent(mxEv, this.state.showHiddenEvents)) { - // XXX: can this ever happen? It will make the result count - // not match the displayed count. - continue; - } - - if (this.state.searchScope === 'All') { - if (roomId !== lastRoomId) { - ret.push(
  • -

    { _t("Room") }: { room.name }

    -
    • ); - lastRoomId = roomId; - } - } - - const resultLink = "#/room/"+roomId+"/"+mxEv.getId(); - - ret.push(); - } - return ret; - } + }; private onAppsClick = () => { dis.dispatch({ @@ -1780,7 +1603,7 @@ export class RoomView extends React.Component { return new Promise(resolve => { this.setState({ timelineRenderingType: TimelineRenderingType.Room, - searchResults: null, + search: null, }, resolve); }); }; @@ -1890,9 +1713,7 @@ export class RoomView extends React.Component { panel = this.messagePanel; } - if (panel) { - panel.handleScrollKey(ev); - } + panel?.handleScrollKey(ev); }; /** @@ -2116,16 +1937,12 @@ export class RoomView extends React.Component { } } - const scrollheaderClasses = classNames({ - mx_RoomView_scrollheader: true, - }); - let statusBar; let isStatusAreaExpanded = true; if (ContentMessages.sharedInstance().getCurrentUploads().length > 0) { statusBar = ; - } else if (!this.state.searchResults) { + } else if (!this.state.search) { isStatusAreaExpanded = this.state.statusBarVisible; statusBar = { let previewBar; if (this.state.timelineRenderingType === TimelineRenderingType.Search) { aux = { ); - let messageComposer; let searchInfo; + let messageComposer; const showComposer = ( // joined and not showing search results - myMembership === 'join' && !this.state.searchResults + myMembership === 'join' && !this.state.search ); if (showComposer) { messageComposer = @@ -2251,40 +2068,24 @@ export class RoomView extends React.Component { />; } - // TODO: Why aren't we storing the term/scope/count in this format - // in this.state if this is what RoomHeader desires? - if (this.state.searchResults) { - searchInfo = { - searchTerm: this.state.searchTerm, - searchScope: this.state.searchScope, - searchCount: this.state.searchResults.count, - }; - } - // if we have search results, we keep the messagepanel (so that it preserves its // scroll state), but hide it. let searchResultsPanel; let hideMessagePanel = false; - if (this.state.searchResults) { - // show searching spinner - if (this.state.searchResults.count === undefined) { - searchResultsPanel = ( -
    - ); - } else { - searchResultsPanel = ( - -
  • - { this.getSearchResultTiles() } - - ); - } + if (this.state.search) { + searchResultsPanel = ; hideMessagePanel = true; } @@ -2321,14 +2122,14 @@ export class RoomView extends React.Component { let topUnreadMessagesBar = null; // Do not show TopUnreadMessagesBar if we have search results showing, it makes no sense - if (this.state.showTopUnreadMessagesBar && !this.state.searchResults) { + if (this.state.showTopUnreadMessagesBar && !this.state.search) { topUnreadMessagesBar = ( ); } let jumpToBottom; // Do not show JumpToBottomButton if we have search results showing, it makes no sense - if (this.state.atEndOfLiveTimeline === false && !this.state.searchResults) { + if (this.state.atEndOfLiveTimeline === false && !this.state.search) { jumpToBottom = ( 0} numUnreadMessages={this.state.numUnreadMessages} @@ -2455,7 +2256,7 @@ export class RoomView extends React.Component { = ({ call }) => { }; export interface ISearchInfo { - searchTerm: string; - searchScope: SearchScope; - searchCount: number; + searchId: number; + roomId?: string; + term: string; + scope: SearchScope; + promise: Promise; + abortController?: AbortController; + + inProgress?: boolean; + count?: number; } export interface IProps { @@ -692,11 +699,9 @@ export default class RoomHeader extends React.Component { // don't display the search count until the search completes and // gives us a valid (possibly zero) searchCount. - if (this.props.searchInfo && - this.props.searchInfo.searchCount !== undefined && - this.props.searchInfo.searchCount !== null) { + if (this.props.searchInfo && typeof this.props.searchInfo.count === "number") { searchStatus =
      - { _t("(~%(count)s results)", { count: this.props.searchInfo.searchCount }) } + { _t("(~%(count)s results)", { count: this.props.searchInfo.count }) }
    ; } From 6e9d69a6f499b95c786afe5da78388842fcc5fef Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 14 Nov 2022 16:00:42 +0000 Subject: [PATCH 2/7] Iterate --- src/components/structures/RoomSearchView.tsx | 10 +++------- src/components/structures/RoomView.tsx | 3 --- src/i18n/strings/en_EN.json | 6 +++--- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/components/structures/RoomSearchView.tsx b/src/components/structures/RoomSearchView.tsx index a0e6c215fe9..b9d09d32db6 100644 --- a/src/components/structures/RoomSearchView.tsx +++ b/src/components/structures/RoomSearchView.tsx @@ -1,8 +1,5 @@ /* -Copyright 2015, 2016 OpenMarket Ltd -Copyright 2017 Vector Creations Ltd -Copyright 2018, 2019 New Vector Ltd -Copyright 2019 - 2022 The Matrix.org Foundation C.I.C. +Copyright 2015 - 2022 The Matrix.org Foundation C.I.C. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -56,6 +53,8 @@ interface Props { onUpdate(inProgress: boolean, results: ISearchResults | null): void; } +// XXX: todo: merge overlapping results somehow? +// XXX: why doesn't searching on name work? export const RoomSearchView = forwardRef(({ term, scope, @@ -170,9 +169,6 @@ export const RoomSearchView = forwardRef(({ } }; - // XXX: todo: merge overlapping results somehow? - // XXX: why doesn't searching on name work? - const ret: JSX.Element[] = []; if (inProgress) { diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index 7f155010502..66735250528 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -17,9 +17,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -// TODO: This component is enormous! There's several things which could stand-alone: -// - Search results component - import React, { createRef, ReactElement, ReactNode, RefObject, useContext } from 'react'; import classNames from 'classnames'; import { IRecommendedVersion, NotificationCountType, Room, RoomEvent } from "matrix-js-sdk/src/models/room"; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index e383e4ff265..1d88e42399e 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -3322,6 +3322,9 @@ "Find a room…": "Find a room…", "Find a room… (e.g. %(exampleRoom)s)": "Find a room… (e.g. %(exampleRoom)s)", "If you can't find the room you're looking for, ask for an invite or create a new room.": "If you can't find the room you're looking for, ask for an invite or create a new room.", + "Search failed": "Search failed", + "Server may be unavailable, overloaded, or search timed out :(": "Server may be unavailable, overloaded, or search timed out :(", + "No more results": "No more results", "You can't send any messages until you review and agree to our terms and conditions.": "You can't send any messages until you review and agree to our terms and conditions.", "Your message wasn't sent because this homeserver has hit its Monthly Active User Limit. Please contact your service administrator to continue using the service.": "Your message wasn't sent because this homeserver has hit its Monthly Active User Limit. Please contact your service administrator to continue using the service.", "Your message wasn't sent because this homeserver has been blocked by its administrator. Please contact your service administrator to continue using the service.": "Your message wasn't sent because this homeserver has been blocked by its administrator. Please contact your service administrator to continue using the service.", @@ -3335,9 +3338,6 @@ "We're creating a room with %(names)s": "We're creating a room with %(names)s", "You seem to be uploading files, are you sure you want to quit?": "You seem to be uploading files, are you sure you want to quit?", "You seem to be in a call, are you sure you want to quit?": "You seem to be in a call, are you sure you want to quit?", - "Search failed": "Search failed", - "Server may be unavailable, overloaded, or search timed out :(": "Server may be unavailable, overloaded, or search timed out :(", - "No more results": "No more results", "Failed to reject invite": "Failed to reject invite", "You have %(count)s unread notifications in a prior version of this room.|other": "You have %(count)s unread notifications in a prior version of this room.", "You have %(count)s unread notifications in a prior version of this room.|one": "You have %(count)s unread notification in a prior version of this room.", From dd067f660cdc6e618ceee0b2f30df565820cf50c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 14 Nov 2022 16:15:56 +0000 Subject: [PATCH 3/7] Fix types --- src/components/views/rooms/RoomHeader.tsx | 4 ++-- test/components/views/rooms/RoomHeader-test.tsx | 17 +++++++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/components/views/rooms/RoomHeader.tsx b/src/components/views/rooms/RoomHeader.tsx index 4b75cbddfcd..87ed74198c3 100644 --- a/src/components/views/rooms/RoomHeader.tsx +++ b/src/components/views/rooms/RoomHeader.tsx @@ -415,7 +415,7 @@ export interface IProps { onAppsClick: (() => void) | null; e2eStatus: E2EStatus; appsShown: boolean; - searchInfo: ISearchInfo; + searchInfo?: ISearchInfo; excludedRightPanelPhaseButtons?: Array; showButtons?: boolean; enableRoomOptionsMenu?: boolean; @@ -699,7 +699,7 @@ export default class RoomHeader extends React.Component { // don't display the search count until the search completes and // gives us a valid (possibly zero) searchCount. - if (this.props.searchInfo && typeof this.props.searchInfo.count === "number") { + if (typeof this.props.searchInfo?.count === "number") { searchStatus =
      { _t("(~%(count)s results)", { count: this.props.searchInfo.count }) }
    ; diff --git a/test/components/views/rooms/RoomHeader-test.tsx b/test/components/views/rooms/RoomHeader-test.tsx index 1d978778f39..887a58fba3f 100644 --- a/test/components/views/rooms/RoomHeader-test.tsx +++ b/test/components/views/rooms/RoomHeader-test.tsx @@ -26,6 +26,7 @@ import { PendingEventOrdering } from "matrix-js-sdk/src/client"; import { CallType } from "matrix-js-sdk/src/webrtc/call"; import { ClientWidgetApi, Widget } from "matrix-widget-api"; import EventEmitter from "events"; +import { ISearchResults } from 'matrix-js-sdk/src/@types/search'; import type { MatrixClient } from "matrix-js-sdk/src/client"; import type { MatrixEvent } from "matrix-js-sdk/src/models/event"; @@ -253,9 +254,11 @@ function mountHeader(room: Room, propsOverride = {}, roomContext?: Partial(() => {}), + term: "", + scope: SearchScope.Room, + count: 0, }, viewingCall: false, activeCall: null, @@ -472,9 +475,11 @@ describe("RoomHeader (React Testing Library)", () => { e2eStatus={E2EStatus.Normal} appsShown={true} searchInfo={{ - searchTerm: "", - searchScope: SearchScope.Room, - searchCount: 0, + searchId: Math.random(), + promise: new Promise(() => {}), + term: "", + scope: SearchScope.Room, + count: 0, }} viewingCall={false} activeCall={null} From ae569d445d40d1a183b4d477690a2fddab3d3cd5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 15 Nov 2022 12:07:12 +0000 Subject: [PATCH 4/7] Add tests --- src/components/structures/RoomSearchView.tsx | 5 +- src/components/views/rooms/EventTile.tsx | 17 +- .../structures/SearchRoomView-test.tsx | 167 ++++++++++++++++++ 3 files changed, 179 insertions(+), 10 deletions(-) create mode 100644 test/components/structures/SearchRoomView-test.tsx diff --git a/src/components/structures/RoomSearchView.tsx b/src/components/structures/RoomSearchView.tsx index b9d09d32db6..803fdbe1e63 100644 --- a/src/components/structures/RoomSearchView.tsx +++ b/src/components/structures/RoomSearchView.tsx @@ -150,7 +150,10 @@ export const RoomSearchView = forwardRef(({ // show searching spinner if (results?.count === undefined) { return ( -
    +
    ); } diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index 4a3b1ebf8d6..2fdf914ab9c 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -471,9 +471,13 @@ export class UnwrappedEventTile extends React.Component componentWillUnmount() { const client = MatrixClientPeg.get(); - client.removeListener(CryptoEvent.DeviceVerificationChanged, this.onDeviceVerificationChanged); - client.removeListener(CryptoEvent.UserTrustStatusChanged, this.onUserVerificationChanged); - client.removeListener(RoomEvent.Receipt, this.onRoomReceipt); + if (client) { + client.removeListener(CryptoEvent.DeviceVerificationChanged, this.onDeviceVerificationChanged); + client.removeListener(CryptoEvent.UserTrustStatusChanged, this.onUserVerificationChanged); + client.removeListener(RoomEvent.Receipt, this.onRoomReceipt); + const room = client.getRoom(this.props.mxEvent.getRoomId()); + room?.off(ThreadEvent.New, this.onNewThread); + } this.isListeningForReceipts = false; this.props.mxEvent.removeListener(MatrixEventEvent.Decrypted, this.onDecrypted); if (this.props.showReactions) { @@ -482,12 +486,7 @@ export class UnwrappedEventTile extends React.Component if (SettingsStore.getValue("feature_thread")) { this.props.mxEvent.off(ThreadEvent.Update, this.updateThread); } - - const room = MatrixClientPeg.get().getRoom(this.props.mxEvent.getRoomId()); - room?.off(ThreadEvent.New, this.onNewThread); - if (this.threadState) { - this.threadState.off(NotificationStateEvents.Update, this.onThreadStateUpdate); - } + this.threadState?.off(NotificationStateEvents.Update, this.onThreadStateUpdate); } componentDidUpdate() { diff --git a/test/components/structures/SearchRoomView-test.tsx b/test/components/structures/SearchRoomView-test.tsx new file mode 100644 index 00000000000..0c2b7fc8c01 --- /dev/null +++ b/test/components/structures/SearchRoomView-test.tsx @@ -0,0 +1,167 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from "react"; +import { mocked, MockedObject } from "jest-mock"; +import { render, screen } from "@testing-library/react"; +import { Room } from "matrix-js-sdk/src/models/room"; +import { ISearchResults } from "matrix-js-sdk/src/@types/search"; +import { defer } from "matrix-js-sdk/src/utils"; +import { SearchResult } from "matrix-js-sdk/src/models/search-result"; +import { IEvent, MatrixEvent } from "matrix-js-sdk/src/models/event"; +import { EventType } from "matrix-js-sdk/src/@types/event"; +import { MatrixClient } from "matrix-js-sdk/src/matrix"; + +import { RoomSearchView } from "../../../src/components/structures/RoomSearchView"; +import { SearchScope } from "../../../src/components/views/rooms/SearchBar"; +import ResizeNotifier from "../../../src/utils/ResizeNotifier"; +import { RoomPermalinkCreator } from "../../../src/utils/permalinks/Permalinks"; +import { stubClient } from "../../test-utils"; +import MatrixClientContext from "../../../src/contexts/MatrixClientContext"; +import { MatrixClientPeg } from "../../../src/MatrixClientPeg"; + +describe("", () => { + const eventMapper = (obj: Partial) => new MatrixEvent(obj); + const resizeNotifier = new ResizeNotifier(); + let client: MockedObject; + let room: Room; + let permalinkCreator: RoomPermalinkCreator; + + beforeEach(async () => { + stubClient(); + client = mocked(MatrixClientPeg.get()); + room = new Room("!room:server", client, client.getUserId()); + client.getRoom.mockReturnValue(room); + permalinkCreator = new RoomPermalinkCreator(room, room.roomId); + }); + + afterEach(async () => { + jest.restoreAllMocks(); + }); + + it("should show a spinner before the promise resolves", async () => { + const deferred = defer(); + + render(( + + )); + + await screen.findByTestId("messagePanelSearchSpinner"); + }); + + it("should render results when the promise resolves", async () => { + render(( + + ({ + results: [ + SearchResult.fromJson({ + rank: 1, + result: { + room_id: room.roomId, + event_id: "$2", + sender: client.getUserId(), + origin_server_ts: 1, + content: { body: "Foo Test Bar", msgtype: "m.text" }, + type: EventType.RoomMessage, + }, + context: { + profile_info: {}, + events_before: [{ + room_id: room.roomId, + event_id: "$1", + sender: client.getUserId(), + origin_server_ts: 1, + content: { body: "Before", msgtype: "m.text" }, + type: EventType.RoomMessage, + }], + events_after: [{ + room_id: room.roomId, + event_id: "$3", + sender: client.getUserId(), + origin_server_ts: 1, + content: { body: "After", msgtype: "m.text" }, + type: EventType.RoomMessage, + }], + }, + }, eventMapper), + ], + highlights: [], + count: 1, + })} + resizeNotifier={resizeNotifier} + permalinkCreator={permalinkCreator} + className="someClass" + onUpdate={jest.fn()} + /> + + )); + + await screen.findByText("Before"); + await screen.findByText("Foo Test Bar"); + await screen.findByText("After"); + }); + + it("should highlight words correctly", async () => { + render(( + + ({ + results: [ + SearchResult.fromJson({ + rank: 1, + result: { + room_id: room.roomId, + event_id: "$2", + sender: client.getUserId(), + origin_server_ts: 1, + content: { body: "Foo Test Bar", msgtype: "m.text" }, + type: EventType.RoomMessage, + }, + context: { + profile_info: {}, + events_before: [], + events_after: [], + }, + }, eventMapper), + ], + highlights: ["test"], + count: 1, + })} + resizeNotifier={resizeNotifier} + permalinkCreator={permalinkCreator} + className="someClass" + onUpdate={jest.fn()} + /> + + )); + + const text = await screen.findByText("Test"); + expect(text).toHaveClass("mx_EventTile_searchHighlight"); + }); +}); From a00781f4c1dbf5d7806b1872c9704ffb415a1f12 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 15 Nov 2022 15:47:30 +0000 Subject: [PATCH 5/7] Increase coverage --- src/components/structures/RoomSearchView.tsx | 2 +- src/components/structures/ScrollPanel.tsx | 2 +- .../structures/SearchRoomView-test.tsx | 92 +++++++++++++++++-- 3 files changed, 88 insertions(+), 8 deletions(-) diff --git a/src/components/structures/RoomSearchView.tsx b/src/components/structures/RoomSearchView.tsx index 803fdbe1e63..70e26ca3cf5 100644 --- a/src/components/structures/RoomSearchView.tsx +++ b/src/components/structures/RoomSearchView.tsx @@ -119,7 +119,7 @@ export const RoomSearchView = forwardRef(({ } setHighlights(highlights); - setResults(results); + setResults({ ...results }); // copy to force a refresh }, (error) => { if (aborted.current) { logger.error("Discarding stale search results"); diff --git a/src/components/structures/ScrollPanel.tsx b/src/components/structures/ScrollPanel.tsx index 5db5e6daad9..202966434f7 100644 --- a/src/components/structures/ScrollPanel.tsx +++ b/src/components/structures/ScrollPanel.tsx @@ -376,7 +376,7 @@ export default class ScrollPanel extends React.Component { const itemlist = this.itemlist.current; const firstTile = itemlist && itemlist.firstElementChild as HTMLElement; const contentTop = firstTile && firstTile.offsetTop; - const fillPromises = []; + const fillPromises: Promise[] = []; // if scrollTop gets to 1 screen from the top of the first tile, // try backward filling diff --git a/test/components/structures/SearchRoomView-test.tsx b/test/components/structures/SearchRoomView-test.tsx index 0c2b7fc8c01..d0dc7041451 100644 --- a/test/components/structures/SearchRoomView-test.tsx +++ b/test/components/structures/SearchRoomView-test.tsx @@ -15,8 +15,8 @@ limitations under the License. */ import React from "react"; -import { mocked, MockedObject } from "jest-mock"; -import { render, screen } from "@testing-library/react"; +import { mocked } from "jest-mock"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { Room } from "matrix-js-sdk/src/models/room"; import { ISearchResults } from "matrix-js-sdk/src/@types/search"; import { defer } from "matrix-js-sdk/src/utils"; @@ -32,20 +32,28 @@ import { RoomPermalinkCreator } from "../../../src/utils/permalinks/Permalinks"; import { stubClient } from "../../test-utils"; import MatrixClientContext from "../../../src/contexts/MatrixClientContext"; import { MatrixClientPeg } from "../../../src/MatrixClientPeg"; +import { searchPagination } from "../../../src/Searching"; + +jest.mock("../../../src/Searching", () => ({ + searchPagination: jest.fn(), +})); describe("", () => { const eventMapper = (obj: Partial) => new MatrixEvent(obj); const resizeNotifier = new ResizeNotifier(); - let client: MockedObject; + let client: MatrixClient; let room: Room; let permalinkCreator: RoomPermalinkCreator; beforeEach(async () => { stubClient(); - client = mocked(MatrixClientPeg.get()); + client = MatrixClientPeg.get(); + client.supportsExperimentalThreads = jest.fn().mockReturnValue(true); room = new Room("!room:server", client, client.getUserId()); - client.getRoom.mockReturnValue(room); + mocked(client.getRoom).mockReturnValue(room); permalinkCreator = new RoomPermalinkCreator(room, room.roomId); + + jest.spyOn(Element.prototype, "clientHeight", "get").mockReturnValue(100); }); afterEach(async () => { @@ -130,7 +138,7 @@ describe("", () => { ({ results: [ SearchResult.fromJson({ @@ -164,4 +172,76 @@ describe("", () => { const text = await screen.findByText("Test"); expect(text).toHaveClass("mx_EventTile_searchHighlight"); }); + + it("should show spinner above results when backpaginating", async () => { + const searchResults: ISearchResults = { + results: [ + SearchResult.fromJson({ + rank: 1, + result: { + room_id: room.roomId, + event_id: "$2", + sender: client.getUserId(), + origin_server_ts: 1, + content: { body: "Foo Test Bar", msgtype: "m.text" }, + type: EventType.RoomMessage, + }, + context: { + profile_info: {}, + events_before: [], + events_after: [], + }, + }, eventMapper), + ], + highlights: ["test"], + next_batch: "next_batch", + count: 2, + }; + + mocked(searchPagination).mockResolvedValue({ + ...searchResults, + results: [ + ...searchResults.results, + SearchResult.fromJson({ + rank: 1, + result: { + room_id: room.roomId, + event_id: "$4", + sender: client.getUserId(), + origin_server_ts: 4, + content: { body: "Potato", msgtype: "m.text" }, + type: EventType.RoomMessage, + }, + context: { + profile_info: {}, + events_before: [], + events_after: [], + }, + }, eventMapper), + ], + next_batch: undefined, + }); + + const { container } = render(( + + + + )); + + await waitFor(() => expect(container.querySelector(".mx_AutoHideScrollbar")).toBeTruthy()); + const scrollable = container.querySelector(".mx_AutoHideScrollbar"); + fireEvent.scroll(scrollable, {}); + + await screen.findByRole("progressbar"); + await screen.findByText("Potato"); + expect(screen.queryByRole("progressbar")).toBeFalsy(); + }); }); From 146bcf62895129e6a40312cb6d8dbbb88d837857 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 15 Nov 2022 16:16:36 +0000 Subject: [PATCH 6/7] Simplify test --- test/components/structures/SearchRoomView-test.tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/components/structures/SearchRoomView-test.tsx b/test/components/structures/SearchRoomView-test.tsx index d0dc7041451..ee8745e5dd6 100644 --- a/test/components/structures/SearchRoomView-test.tsx +++ b/test/components/structures/SearchRoomView-test.tsx @@ -16,7 +16,7 @@ limitations under the License. import React from "react"; import { mocked } from "jest-mock"; -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { render, screen } from "@testing-library/react"; import { Room } from "matrix-js-sdk/src/models/room"; import { ISearchResults } from "matrix-js-sdk/src/@types/search"; import { defer } from "matrix-js-sdk/src/utils"; @@ -222,7 +222,7 @@ describe("", () => { next_batch: undefined, }); - const { container } = render(( + render(( ", () => { )); - await waitFor(() => expect(container.querySelector(".mx_AutoHideScrollbar")).toBeTruthy()); - const scrollable = container.querySelector(".mx_AutoHideScrollbar"); - fireEvent.scroll(scrollable, {}); - await screen.findByRole("progressbar"); await screen.findByText("Potato"); expect(screen.queryByRole("progressbar")).toBeFalsy(); From c8b6488a04ea936b358e2f191d50141d836b7fd8 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 15 Nov 2022 17:03:54 +0000 Subject: [PATCH 7/7] Improve coverage --- src/components/structures/RoomSearchView.tsx | 29 ++++---- src/components/structures/RoomView.tsx | 1 + .../structures/SearchRoomView-test.tsx | 70 +++++++++++++++++++ 3 files changed, 85 insertions(+), 15 deletions(-) diff --git a/src/components/structures/RoomSearchView.tsx b/src/components/structures/RoomSearchView.tsx index 70e26ca3cf5..0617d0136a8 100644 --- a/src/components/structures/RoomSearchView.tsx +++ b/src/components/structures/RoomSearchView.tsx @@ -37,6 +37,7 @@ import RoomContext from "../../contexts/RoomContext"; const DEBUG = false; let debuglog = function(msg: string) {}; +/* istanbul ignore next */ if (DEBUG) { // using bind means that we get to keep useful line numbers in the console debuglog = logger.log.bind(console); @@ -89,7 +90,7 @@ export const RoomSearchView = forwardRef(({ // whether it was used by the search engine or not. let highlights = results.highlights; - if (highlights.indexOf(term) < 0) { + if (!highlights.includes(term)) { highlights = highlights.concat(term); } @@ -128,8 +129,8 @@ export const RoomSearchView = forwardRef(({ logger.error("Search failed", error); Modal.createDialog(ErrorDialog, { title: _t("Search failed"), - description: ((error && error.message) ? error.message : - _t("Server may be unavailable, overloaded, or search timed out :(")), + description: error?.message + ?? _t("Server may be unavailable, overloaded, or search timed out :("), }); return false; }).finally(() => { @@ -157,19 +158,19 @@ export const RoomSearchView = forwardRef(({ ); } - const onSearchResultsFillRequest = (backwards: boolean): Promise => { + const onSearchResultsFillRequest = async (backwards: boolean): Promise => { if (!backwards) { - return Promise.resolve(false); + return false; } - if (results.next_batch) { - debuglog("requesting more search results"); - const searchPromise = searchPagination(results); - return handleSearchResult(searchPromise); - } else { + if (!results.next_batch) { debuglog("no more search results"); - return Promise.resolve(false); + return false; } + + debuglog("requesting more search results"); + const searchPromise = searchPagination(results); + return handleSearchResult(searchPromise); }; const ret: JSX.Element[] = []; @@ -184,13 +185,11 @@ export const RoomSearchView = forwardRef(({ if (!results?.results?.length) { ret.push(

  • { _t("No results") }

    -
    • , - ); + ); } else { ret.push(

  • { _t("No more results") }

    -
    • , - ); + ); } } diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index 66735250528..a73082129b7 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -117,6 +117,7 @@ let debuglog = function(msg: string) {}; const BROWSER_SUPPORTS_SANDBOX = 'sandbox' in document.createElement('iframe'); +/* istanbul ignore next */ if (DEBUG) { // using bind means that we get to keep useful line numbers in the console debuglog = logger.log.bind(console); diff --git a/test/components/structures/SearchRoomView-test.tsx b/test/components/structures/SearchRoomView-test.tsx index ee8745e5dd6..c3b07a148c8 100644 --- a/test/components/structures/SearchRoomView-test.tsx +++ b/test/components/structures/SearchRoomView-test.tsx @@ -240,4 +240,74 @@ describe("", () => { await screen.findByText("Potato"); expect(screen.queryByRole("progressbar")).toBeFalsy(); }); + + it("should handle resolutions after unmounting sanely", async () => { + const deferred = defer(); + + const { unmount } = render(( + + + + )); + + unmount(); + deferred.resolve({ + results: [], + highlights: [], + }); + }); + + it("should handle rejections after unmounting sanely", async () => { + const deferred = defer(); + + const { unmount } = render(( + + + + )); + + unmount(); + deferred.reject({ + results: [], + highlights: [], + }); + }); + + it("should show modal if error is encountered", async () => { + const deferred = defer(); + + render(( + + + + )); + deferred.reject(new Error("Some error")); + + await screen.findByText("Search failed"); + await screen.findByText("Some error"); + }); });