From 07299828c9e0d9050e6a8c63ed129b16a4d4bf0d Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 9 Apr 2019 00:06:13 +0100 Subject: [PATCH 1/3] Pressing next forces search to select --- src/devtools/views/Components/TreeContext.js | 21 +++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/devtools/views/Components/TreeContext.js b/src/devtools/views/Components/TreeContext.js index b83078f653..41dd0eb518 100644 --- a/src/devtools/views/Components/TreeContext.js +++ b/src/devtools/views/Components/TreeContext.js @@ -202,17 +202,25 @@ function reduceSearchState(store: Store, state: State, action: Action): State { const prevSearchText = searchText; const numPrevSearchResults = searchResults.length; + // We track explicitly whether search was requested because + // we might want to search even if search index didn't change. + // For example, if you press "next result" on a search with a single + // result but a different current selection, we'll set this to true. + let didRequestSearch = false; + // Search isn't supported when the owner's tree is active. if (ownerStack.length === 0) { switch (type) { case 'GO_TO_NEXT_SEARCH_RESULT': if (numPrevSearchResults > 0) { + didRequestSearch = true; searchIndex = searchIndex + 1 < numPrevSearchResults ? searchIndex + 1 : 0; } break; case 'GO_TO_PREVIOUS_SEARCH_RESULT': if (numPrevSearchResults > 0) { + didRequestSearch = true; searchIndex = ((searchIndex: any): number) > 0 ? ((searchIndex: any): number) - 1 @@ -313,10 +321,17 @@ function reduceSearchState(store: Store, state: State, action: Action): State { } // Changes in search index or typing should override the selected element. - const didAddToSearchText = + if (searchIndex !== prevSearchIndex) { + didRequestSearch = true; + } + if ( + // Did the user type more? searchText.length > prevSearchText.length && - searchText.indexOf(prevSearchText) === 0; - if (searchIndex !== prevSearchIndex || didAddToSearchText) { + searchText.indexOf(prevSearchText) === 0 + ) { + didRequestSearch = true; + } + if (didRequestSearch) { if (searchIndex === null) { selectedElementIndex = null; selectedElementID = null; From 3eca0bbe6144dcb2d542766e323bf3c6f005cd08 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 10 Apr 2019 15:08:13 +0100 Subject: [PATCH 2/3] Use heuristic suggested by @sophiebits --- src/devtools/views/Components/TreeContext.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/devtools/views/Components/TreeContext.js b/src/devtools/views/Components/TreeContext.js index 41dd0eb518..92b59770cf 100644 --- a/src/devtools/views/Components/TreeContext.js +++ b/src/devtools/views/Components/TreeContext.js @@ -320,16 +320,20 @@ function reduceSearchState(store: Store, state: State, action: Action): State { } } - // Changes in search index or typing should override the selected element. if (searchIndex !== prevSearchIndex) { + // The user intentionally navigated between search results didRequestSearch = true; } - if ( - // Did the user type more? - searchText.length > prevSearchText.length && - searchText.indexOf(prevSearchText) === 0 - ) { - didRequestSearch = true; + if (searchText !== prevSearchText) { + if (searchResults.indexOf(selectedElementID) === -1) { + // Only move the selection if the new query + // doesn't match the current selection anymore. + didRequestSearch = true; + } else { + // Selected item still matches the new search query. + // Adjust the index to reflect its position in new results. + searchIndex = searchResults.indexOf(selectedElementID); + } } if (didRequestSearch) { if (searchIndex === null) { From 1102fc5c1190b08ad5032333975ec57d9a0b5254 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 10 Apr 2019 15:25:03 +0100 Subject: [PATCH 3/3] Refactor: extract a variable --- src/devtools/views/Components/TreeContext.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/devtools/views/Components/TreeContext.js b/src/devtools/views/Components/TreeContext.js index 92b59770cf..8e264f826f 100644 --- a/src/devtools/views/Components/TreeContext.js +++ b/src/devtools/views/Components/TreeContext.js @@ -325,14 +325,15 @@ function reduceSearchState(store: Store, state: State, action: Action): State { didRequestSearch = true; } if (searchText !== prevSearchText) { - if (searchResults.indexOf(selectedElementID) === -1) { + const newSearchIndex = searchResults.indexOf(selectedElementID); + if (newSearchIndex === -1) { // Only move the selection if the new query // doesn't match the current selection anymore. didRequestSearch = true; } else { // Selected item still matches the new search query. // Adjust the index to reflect its position in new results. - searchIndex = searchResults.indexOf(selectedElementID); + searchIndex = newSearchIndex; } } if (didRequestSearch) {