From 1d3fe4bbf32a08212e124238aadff9fd25cf5d40 Mon Sep 17 00:00:00 2001 From: Ken Powers Date: Wed, 4 Sep 2019 10:46:28 -0400 Subject: [PATCH] Split search actions between discussions and messages --- stylesheets/_modules.scss | 21 ++-- stylesheets/_theme_dark.scss | 2 +- ts/components/MainHeader.tsx | 21 +++- ts/components/SearchResults.tsx | 42 +++++-- ts/state/ducks/search.ts | 188 ++++++++++++++++++++++---------- ts/state/selectors/search.ts | 26 ++++- ts/util/lint/exceptions.json | 6 +- 7 files changed, 213 insertions(+), 93 deletions(-) diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index a53a347083..23f28feb5e 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -3219,20 +3219,6 @@ padding-right: 1em; width: 100%; text-align: center; - - animation: delayed-fade-in 2s; -} - -@keyframes delayed-fade-in { - 0% { - opacity: 0; - } - 50% { - opacity: 0; - } - 100% { - opacity: 1; - } } .module-search-results__contacts-header { @@ -3257,6 +3243,13 @@ letter-spacing: 0; } +.module-search-results__spinner-container { + width: 100%; + padding: 10px; + + text-align: center; +} + // Module: Message Search Result .module-message-search-result { diff --git a/stylesheets/_theme_dark.scss b/stylesheets/_theme_dark.scss index 9fa78df7d4..f7890daef4 100644 --- a/stylesheets/_theme_dark.scss +++ b/stylesheets/_theme_dark.scss @@ -1396,7 +1396,7 @@ body.dark-theme { } &:focus { - border: solid 1px blue; + border: solid 1px $color-signal-blue; outline: none; } } diff --git a/ts/components/MainHeader.tsx b/ts/components/MainHeader.tsx index fc7a8d4c53..bff57b17a9 100644 --- a/ts/components/MainHeader.tsx +++ b/ts/components/MainHeader.tsx @@ -25,11 +25,16 @@ export interface PropsType { i18n: LocalizerType; updateSearchTerm: (searchTerm: string) => void; - search: ( + searchMessages: ( query: string, options: { searchConversationId?: string; regionCode: string; + } + ) => void; + searchDiscussions: ( + query: string, + options: { ourNumber: string; noteToSelf: string; } @@ -66,15 +71,21 @@ export class MainHeader extends React.Component { i18n, ourNumber, regionCode, - search, + searchDiscussions, + searchMessages, searchConversationId, } = this.props; - if (search) { - search(searchTerm, { - searchConversationId, + if (searchDiscussions) { + searchDiscussions(searchTerm, { noteToSelf: i18n('noteToSelf').toLowerCase(), ourNumber, + }); + } + + if (searchMessages) { + searchMessages(searchTerm, { + searchConversationId, regionCode, }); } diff --git a/ts/components/SearchResults.tsx b/ts/components/SearchResults.tsx index d7ca520956..9239540279 100644 --- a/ts/components/SearchResults.tsx +++ b/ts/components/SearchResults.tsx @@ -8,6 +8,7 @@ import { import { Intl } from './Intl'; import { Emojify } from './conversation/Emojify'; +import { Spinner } from './Spinner'; import { ConversationListItem, PropsData as ConversationListItemPropsType, @@ -17,11 +18,13 @@ import { StartNewConversation } from './StartNewConversation'; import { LocalizerType } from '../types/Util'; export type PropsDataType = { + discussionsLoading: boolean; items: Array; + messagesLoading: boolean; noResults: boolean; regionCode: string; - searchTerm: string; searchConversationName?: string; + searchTerm: string; }; type StartNewConversationType = { @@ -52,6 +55,10 @@ type MessageType = { type: 'message'; data: string; }; +type SpinnerType = { + type: 'spinner'; + data: undefined; +}; export type SearchResultRowType = | StartNewConversationType @@ -60,7 +67,8 @@ export type SearchResultRowType = | MessagesHeaderType | ConversationType | ContactsType - | MessageType; + | MessageType + | SpinnerType; type PropsHousekeepingType = { i18n: LocalizerType; @@ -89,7 +97,7 @@ export class SearchResults extends React.Component { public mostRecentWidth = 0; public mostRecentHeight = 0; public cellSizeCache = new CellMeasurerCache({ - defaultHeight: 36, + defaultHeight: 80, fixedWidth: true, }); public listRef = React.createRef(); @@ -160,6 +168,12 @@ export class SearchResults extends React.Component { const { data } = row; return renderMessageSearchResult(data); + } else if (row.type === 'spinner') { + return ( +
+ +
+ ); } else { throw new Error( 'SearchResults.renderRowContents: Encountered unknown row type' @@ -194,14 +208,24 @@ export class SearchResults extends React.Component { }; public componentDidUpdate(prevProps: PropsType) { - const { items } = this.props; + const { + items, + searchTerm, + discussionsLoading, + messagesLoading, + } = this.props; - if ( + if (searchTerm !== prevProps.searchTerm) { + this.resizeAll(); + } else if ( + discussionsLoading !== prevProps.discussionsLoading || + messagesLoading !== prevProps.messagesLoading + ) { + this.resizeAll(); + } else if ( items && - items.length > 0 && prevProps.items && - prevProps.items.length > 0 && - items !== prevProps.items + prevProps.items.length !== items.length ) { this.resizeAll(); } @@ -270,7 +294,7 @@ export class SearchResults extends React.Component { } return ( -
+
{({ height, width }) => { this.mostRecentWidth = width; diff --git a/ts/state/ducks/search.ts b/ts/state/ducks/search.ts index 9b8e4ca659..d65388f7f1 100644 --- a/ts/state/ducks/search.ts +++ b/ts/state/ducks/search.ts @@ -4,8 +4,8 @@ import { normalize } from '../../types/PhoneNumber'; import { trigger } from '../../shims/events'; import { cleanSearchTerm } from '../../util/cleanSearchTerm'; import { - searchConversations, - searchMessages, + searchConversations as dataSearchConversations, + searchMessages as dataSearchMessages, searchMessagesInConversation, } from '../../../js/modules/data'; import { makeLookup } from '../../util/makeLookup'; @@ -40,25 +40,48 @@ export type SearchStateType = { // We do store message data to pass through the selector messageLookup: MessageSearchResultLookupType; selectedMessage?: string; + // Loading state + discussionsLoading: boolean; + messagesLoading: boolean; }; // Actions -type SearchResultsPayloadType = { +type SearchResultsBaseType = { query: string; normalizedPhoneNumber?: string; +}; +type SearchMessagesResultsPayloadType = SearchResultsBaseType & { messages: Array; +}; +type SearchDiscussionsResultsPayloadType = SearchResultsBaseType & { conversations: Array; contacts: Array; }; - -type SearchResultsKickoffActionType = { - type: 'SEARCH_RESULTS'; - payload: Promise; +type SearchMessagesResultsKickoffActionType = { + type: 'SEARCH_MESSAGES_RESULTS'; + payload: Promise; }; -type SearchResultsFulfilledActionType = { - type: 'SEARCH_RESULTS_FULFILLED'; - payload: SearchResultsPayloadType; +type SearchDiscussionsResultsKickoffActionType = { + type: 'SEARCH_DISCUSSIONS_RESULTS'; + payload: Promise; +}; + +type SearchMessagesResultsPendingActionType = { + type: 'SEARCH_MESSAGES_RESULTS_PENDING'; + payload: SearchMessagesResultsPayloadType; +}; +type SearchDiscussionsResultsPendingActionType = { + type: 'SEARCH_DISCUSSIONS_RESULTS_PENDING'; + payload: SearchDiscussionsResultsPayloadType; +}; +type SearchMessagesResultsFulfilledActionType = { + type: 'SEARCH_MESSAGES_RESULTS_FULFILLED'; + payload: SearchMessagesResultsPayloadType; +}; +type SearchDiscussionsResultsFulfilledActionType = { + type: 'SEARCH_DISCUSSIONS_RESULTS_FULFILLED'; + payload: SearchDiscussionsResultsPayloadType; }; type UpdateSearchTermActionType = { type: 'SEARCH_UPDATE'; @@ -83,7 +106,12 @@ type SearchInConversationActionType = { }; export type SEARCH_TYPES = - | SearchResultsFulfilledActionType + | SearchMessagesResultsKickoffActionType + | SearchDiscussionsResultsKickoffActionType + | SearchMessagesResultsPendingActionType + | SearchDiscussionsResultsPendingActionType + | SearchMessagesResultsFulfilledActionType + | SearchDiscussionsResultsFulfilledActionType | UpdateSearchTermActionType | ClearSearchActionType | ClearConversationSearchActionType @@ -95,7 +123,8 @@ export type SEARCH_TYPES = // Action Creators export const actions = { - search, + searchMessages, + searchDiscussions, clearSearch, clearConversationSearch, searchInConversation, @@ -103,59 +132,73 @@ export const actions = { startNewConversation, }; -function search( +function searchMessages( query: string, options: { - searchConversationId?: string; regionCode: string; - ourNumber: string; - noteToSelf: string; } -): SearchResultsKickoffActionType { +): SearchMessagesResultsKickoffActionType { return { - type: 'SEARCH_RESULTS', - payload: doSearch(query, options), + type: 'SEARCH_MESSAGES_RESULTS', + payload: doSearchMessages(query, options), }; } -async function doSearch( +function searchDiscussions( + query: string, + options: { + ourNumber: string; + noteToSelf: string; + } +): SearchDiscussionsResultsKickoffActionType { + return { + type: 'SEARCH_DISCUSSIONS_RESULTS', + payload: doSearchDiscussions(query, options), + }; +} + +async function doSearchMessages( query: string, options: { searchConversationId?: string; regionCode: string; + } +): Promise { + const { regionCode, searchConversationId } = options; + const normalizedPhoneNumber = normalize(query, { regionCode }); + + const messages = await queryMessages(query, searchConversationId); + + return { + messages, + normalizedPhoneNumber, + query, + }; +} + +async function doSearchDiscussions( + query: string, + options: { ourNumber: string; noteToSelf: string; } -): Promise { - const { regionCode, ourNumber, noteToSelf, searchConversationId } = options; - const normalizedPhoneNumber = normalize(query, { regionCode }); +): Promise { + const { ourNumber, noteToSelf } = options; + const { conversations, contacts } = await queryConversationsAndContacts( + query, + { + ourNumber, + noteToSelf, + } + ); - if (searchConversationId) { - const messages = await queryMessages(query, searchConversationId); - - return { - contacts: [], - conversations: [], - messages, - normalizedPhoneNumber, - query, - }; - } else { - const [discussions, messages] = await Promise.all([ - queryConversationsAndContacts(query, { ourNumber, noteToSelf }), - queryMessages(query), - ]); - const { conversations, contacts } = discussions; - - return { - contacts, - conversations, - messages, - normalizedPhoneNumber, - query, - }; - } + return { + conversations, + contacts, + query, + }; } + function clearSearch(): ClearSearchActionType { return { type: 'SEARCH_CLEAR', @@ -214,7 +257,7 @@ async function queryMessages(query: string, searchConversationId?: string) { return searchMessagesInConversation(normalized, searchConversationId); } - return searchMessages(normalized); + return dataSearchMessages(normalized); } catch (e) { return []; } @@ -227,7 +270,7 @@ async function queryConversationsAndContacts( const { ourNumber, noteToSelf } = options; const query = providedQuery.replace(/[+-.()]*/g, ''); - const searchResults: Array = await searchConversations( + const searchResults: Array = await dataSearchConversations( query ); @@ -266,6 +309,8 @@ function getEmptyState(): SearchStateType { messageLookup: {}, conversations: [], contacts: [], + discussionsLoading: false, + messagesLoading: false, }; } @@ -312,15 +357,27 @@ export function reducer( }; } - if (action.type === 'SEARCH_RESULTS_FULFILLED') { + if (action.type === 'SEARCH_MESSAGES_RESULTS_PENDING') { + return { + ...state, + messageIds: [], + messageLookup: {}, + messagesLoading: true, + }; + } + + if (action.type === 'SEARCH_DISCUSSIONS_RESULTS_PENDING') { + return { + ...state, + contacts: [], + conversations: [], + discussionsLoading: true, + }; + } + + if (action.type === 'SEARCH_MESSAGES_RESULTS_FULFILLED') { const { payload } = action; - const { - contacts, - conversations, - messages, - normalizedPhoneNumber, - query, - } = payload; + const { messages, normalizedPhoneNumber, query } = payload; // Reject if the associated query is not the most recent user-provided query if (state.query !== query) { @@ -331,12 +388,23 @@ export function reducer( return { ...state, - contacts, - conversations, normalizedPhoneNumber, query, messageIds, messageLookup: makeLookup(messages, 'id'), + messagesLoading: false, + }; + } + + if (action.type === 'SEARCH_DISCUSSIONS_RESULTS_FULFILLED') { + const { payload } = action; + const { contacts, conversations } = payload; + + return { + ...state, + contacts, + conversations, + discussionsLoading: false, }; } diff --git a/ts/state/selectors/search.ts b/ts/state/selectors/search.ts index a288694409..f66a0a2060 100644 --- a/ts/state/selectors/search.ts +++ b/ts/state/selectors/search.ts @@ -63,7 +63,6 @@ export const getMessageSearchResultLookup = createSelector( getSearch, (state: SearchStateType) => state.messageLookup ); - export const getSearchResults = createSelector( [getSearch, getRegionCode, getConversationLookup, getSelectedConversation], ( @@ -71,11 +70,14 @@ export const getSearchResults = createSelector( regionCode: string, lookup: ConversationLookupType, selectedConversation?: string + // tslint:disable-next-line max-func-body-length ): SearchResultsPropsType | undefined => { const { contacts, conversations, + discussionsLoading, messageIds, + messagesLoading, searchConversationName, } = state; @@ -86,6 +88,8 @@ export const getSearchResults = createSelector( const haveContacts = contacts && contacts.length; const haveMessages = messageIds && messageIds.length; const noResults = + !discussionsLoading && + !messagesLoading && !showStartNewConversation && !haveConversations && !haveContacts && @@ -115,6 +119,15 @@ export const getSearchResults = createSelector( }, }); }); + } else if (discussionsLoading) { + items.push({ + type: 'conversations-header', + data: undefined, + }); + items.push({ + type: 'spinner', + data: undefined, + }); } if (haveContacts) { @@ -145,10 +158,21 @@ export const getSearchResults = createSelector( data: messageId, }); }); + } else if (messagesLoading) { + items.push({ + type: 'messages-header', + data: undefined, + }); + items.push({ + type: 'spinner', + data: undefined, + }); } return { + discussionsLoading, items, + messagesLoading, noResults, regionCode: regionCode, searchConversationName, diff --git a/ts/util/lint/exceptions.json b/ts/util/lint/exceptions.json index 3b1ed5c63e..381796bf8b 100644 --- a/ts/util/lint/exceptions.json +++ b/ts/util/lint/exceptions.json @@ -7502,7 +7502,7 @@ "rule": "React-createRef", "path": "ts/components/MainHeader.js", "line": " this.inputRef = react_1.default.createRef();", - "lineNumber": 83, + "lineNumber": 87, "reasonCategory": "usageTrusted", "updated": "2019-08-09T21:17:57.798Z", "reasonDetail": "Used only to set focus" @@ -7511,7 +7511,7 @@ "rule": "React-createRef", "path": "ts/components/MainHeader.tsx", "line": " this.inputRef = React.createRef();", - "lineNumber": 48, + "lineNumber": 53, "reasonCategory": "usageTrusted", "updated": "2019-08-09T21:17:57.798Z", "reasonDetail": "Used only to set focus" @@ -7520,7 +7520,7 @@ "rule": "React-createRef", "path": "ts/components/SearchResults.js", "line": " this.listRef = react_1.default.createRef();", - "lineNumber": 21, + "lineNumber": 22, "reasonCategory": "usageTrusted", "updated": "2019-08-09T00:44:31.008Z", "reasonDetail": "SearchResults needs to interact with its child List directly"