From d626f71fdd9d1b5d8fad40dba51a6f9adda0f155 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 18 Nov 2022 16:40:22 +0000 Subject: [PATCH] Extract Search handling from RoomView into its own Component (#9574) * Extract Search handling from RoomView into its own Component * Iterate * Fix types * Add tests * Increase coverage * Simplify test * Improve coverage --- src/Searching.ts | 39 ++- src/components/structures/RoomSearchView.tsx | 258 +++++++++++++++ src/components/structures/RoomView.tsx | 309 +++-------------- src/components/structures/ScrollPanel.tsx | 2 +- src/components/views/rooms/EventTile.tsx | 17 +- src/components/views/rooms/RoomHeader.tsx | 21 +- src/i18n/strings/en_EN.json | 6 +- .../structures/SearchRoomView-test.tsx | 313 ++++++++++++++++++ .../views/rooms/RoomHeader-test.tsx | 17 +- 9 files changed, 689 insertions(+), 293 deletions(-) create mode 100644 src/components/structures/RoomSearchView.tsx create mode 100644 test/components/structures/SearchRoomView-test.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..0617d0136a8 --- /dev/null +++ b/src/components/structures/RoomSearchView.tsx @@ -0,0 +1,258 @@ +/* +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. +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) {}; + +/* istanbul ignore next */ +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; +} + +// XXX: todo: merge overlapping results somehow? +// XXX: why doesn't searching on name work? +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.includes(term)) { + 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 }); // copy to force a refresh + }, (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?.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 = async (backwards: boolean): Promise => { + if (!backwards) { + return false; + } + + if (!results.next_batch) { + debuglog("no more search results"); + return false; + } + + debuglog("requesting more search results"); + const searchPromise = searchPagination(results); + return handleSearchResult(searchPromise); + }; + + 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..a73082129b7 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -17,14 +17,10 @@ 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"; -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 +33,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 +44,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 +63,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 +79,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 +96,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,12 +109,15 @@ 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) {}; 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); @@ -168,11 +163,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 +359,6 @@ export class RoomView extends React.Component { private unmounted = false; private permalinkCreators: Record = {}; - private searchId: number; private roomView = createRef(); private searchResultsPanel = createRef(); @@ -389,7 +379,6 @@ export class RoomView extends React.Component { shouldPeek: true, membersLoaded: !llMembers, numUnreadMessages: 0, - searchResults: null, callState: null, activeCall: null, canPeek: false, @@ -1025,7 +1014,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 +1129,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 +1397,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 +1480,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 +1601,7 @@ export class RoomView extends React.Component { return new Promise(resolve => { this.setState({ timelineRenderingType: TimelineRenderingType.Room, - searchResults: null, + search: null, }, resolve); }); }; @@ -1890,9 +1711,7 @@ export class RoomView extends React.Component { panel = this.messagePanel; } - if (panel) { - panel.handleScrollKey(ev); - } + panel?.handleScrollKey(ev); }; /** @@ -2116,16 +1935,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 +2066,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 +2120,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 +2254,7 @@ export class RoomView 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/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index df2a5f448ef..2b3cc82d105 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -457,9 +457,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) { @@ -468,12 +472,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(prevProps: Readonly) { diff --git a/src/components/views/rooms/RoomHeader.tsx b/src/components/views/rooms/RoomHeader.tsx index 38746271c9c..87ed74198c3 100644 --- a/src/components/views/rooms/RoomHeader.tsx +++ b/src/components/views/rooms/RoomHeader.tsx @@ -20,6 +20,7 @@ import classNames from 'classnames'; import { throttle } from 'lodash'; import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; import { CallType } from "matrix-js-sdk/src/webrtc/call"; +import { ISearchResults } from 'matrix-js-sdk/src/@types/search'; import type { MatrixEvent } from "matrix-js-sdk/src/models/event"; import type { Room } from "matrix-js-sdk/src/models/room"; @@ -393,9 +394,15 @@ const CallLayoutSelector: FC = ({ 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 { @@ -408,7 +415,7 @@ export interface IProps { onAppsClick: (() => void) | null; e2eStatus: E2EStatus; appsShown: boolean; - searchInfo: ISearchInfo; + searchInfo?: ISearchInfo; excludedRightPanelPhaseButtons?: Array; showButtons?: boolean; enableRoomOptionsMenu?: boolean; @@ -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 (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 }) }
    ; } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index af177e147e7..62be5c8f788 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.", diff --git a/test/components/structures/SearchRoomView-test.tsx b/test/components/structures/SearchRoomView-test.tsx new file mode 100644 index 00000000000..c3b07a148c8 --- /dev/null +++ b/test/components/structures/SearchRoomView-test.tsx @@ -0,0 +1,313 @@ +/* +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 } 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"; +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: MatrixClient; + let room: Room; + let permalinkCreator: RoomPermalinkCreator; + + beforeEach(async () => { + stubClient(); + client = MatrixClientPeg.get(); + client.supportsExperimentalThreads = jest.fn().mockReturnValue(true); + room = new Room("!room:server", client, client.getUserId()); + mocked(client.getRoom).mockReturnValue(room); + permalinkCreator = new RoomPermalinkCreator(room, room.roomId); + + jest.spyOn(Element.prototype, "clientHeight", "get").mockReturnValue(100); + }); + + 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"); + }); + + 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, + }); + + render(( + + + + )); + + await screen.findByRole("progressbar"); + 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"); + }); +}); 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}