Implement code review changes

Fix preventDefault being called for arrow left/right in search input. Revert ellipsis changes for overly long search results. Adjust matching search history logic to be non-heuristic.

src/constants.js

@@ -193,9 +193,12 @@ const PLAYLIST_HEIGHT_FORCE_LIST_THRESHOLD = 500
 // YouTube search character limit is 100 characters
 const SEARCH_CHAR_LIMIT = 100
 
-// matches # of results we show for search suggestions
+// max # of results we show for search suggestions
 const SEARCH_RESULTS_DISPLAY_LIMIT = 14
 
+// max # of search history results we show when mixed with YT search suggestions
+const MIXED_SEARCH_HISTORY_ENTRIES_DISPLAY_LIMIT = 4
+
 // Displayed on the about page and used in the main.js file to only allow bitcoin URLs with this wallet address to be opened
 const ABOUT_BITCOIN_ADDRESS = '1Lih7Ho5gnxb1CwPD4o59ss78pwo2T91eS'
 
@@ -209,5 +212,6 @@ export {
   PLAYLIST_HEIGHT_FORCE_LIST_THRESHOLD,
   SEARCH_CHAR_LIMIT,
   SEARCH_RESULTS_DISPLAY_LIMIT,
+  MIXED_SEARCH_HISTORY_ENTRIES_DISPLAY_LIMIT,
   ABOUT_BITCOIN_ADDRESS,
 }

src/renderer/components/ft-input/ft-input.css

@@ -207,9 +207,6 @@ body[dir='rtl'] .ft-input-component.search.showClearTextButton:focus-within .inp
   padding-block: 0;
   line-height: 2rem;
   padding-inline: 15px;
-  text-overflow: ellipsis;
-  overflow-x: hidden;
-  white-space: nowrap;
 }
 
 .searchResultIcon {

src/renderer/components/ft-input/ft-input.js

@@ -295,37 +295,33 @@ export default defineComponent({
 
       this.searchState.showOptions = true
 
-      const isArrowKeyPress = event.key.startsWith('Arrow')
-
-      if (!isArrowKeyPress) {
+      // "select" the Remove button through right arrow navigation, and unselect it with the left arrow
+      if (event.key === 'ArrowRight') {
+        this.removeButtonSelectedIndex = this.searchState.selectedOption
+      } else if (event.key === 'ArrowLeft') {
+        this.removeButtonSelectedIndex = -1
+      } else if (event.key === 'ArrowUp' || event.key === 'ArrowDown') {
+        event.preventDefault()
+        const newIndex = this.searchState.selectedOption + (event.key === 'ArrowDown' ? 1 : -1)
+        this.updateSelectedOptionIndex(newIndex)
+      } else {
         const selectedOptionValue = this.searchStateKeyboardSelectedOptionValue
         // Keyboard selected & is char
         if (!isNullOrEmpty(selectedOptionValue) && isKeyboardEventKeyPrintableChar(event.key)) {
           // Update input based on KB selected suggestion value instead of current input value
           event.preventDefault()
           this.handleInput(`${selectedOptionValue}${event.key}`)
-          return
         }
-        return
-      }
-
-      event.preventDefault()
-      if (event.key === 'ArrowDown' || event.key === 'ArrowUp') {
-        const newIndex = this.searchState.selectedOption + (event.key === 'ArrowDown' ? 1 : -1)
-        this.updateSelectedOptionIndex(newIndex)
-
-        // unset selection of "Remove" button
-        this.removeButtonSelectedIndex = -1
-      } else {
-        // "select" the Remove button through right arrow navigation, and unselect it with the left arrow
-        const newIndex = event.key === 'ArrowRight' ? this.searchState.selectedOption : -1
-        this.removeButtonSelectedIndex = newIndex
       }
     },
 
     // Updates the selected dropdown option index and handles the under/over-flow behavior
     updateSelectedOptionIndex: function (index) {
       this.searchState.selectedOption = index
+
+      // unset selection of "Remove" button
+      this.removeButtonSelectedIndex = -1
+
       // Allow deselecting suggestion
       if (this.searchState.selectedOption < -1) {
         this.searchState.selectedOption = this.visibleDataList.length - 1

src/renderer/components/top-nav/top-nav.js

@@ -5,7 +5,7 @@ import FtProfileSelector from '../ft-profile-selector/ft-profile-selector.vue'
 import FtIconButton from '../ft-icon-button/ft-icon-button.vue'
 import debounce from 'lodash.debounce'
 
-import { IpcChannels, KeyboardShortcuts, MOBILE_WIDTH_THRESHOLD, SEARCH_RESULTS_DISPLAY_LIMIT } from '../../../constants'
+import { IpcChannels, KeyboardShortcuts, MIXED_SEARCH_HISTORY_ENTRIES_DISPLAY_LIMIT, MOBILE_WIDTH_THRESHOLD, SEARCH_RESULTS_DISPLAY_LIMIT } from '../../../constants'
 import { localizeAndAddKeyboardShortcutToActionTitle, openInternalPath } from '../../helpers/utils'
 import { translateWindowTitle } from '../../helpers/strings'
 import { clearLocalSearchSuggestionsSession, getLocalSearchSuggestions } from '../../helpers/api/local'
@@ -127,6 +127,7 @@ export default defineComponent({
 
     latestMatchingSearchHistoryNames: function () {
       return this.$store.getters.getLatestMatchingSearchHistoryNames(this.lastSuggestionQuery)
+        .slice(0, MIXED_SEARCH_HISTORY_ENTRIES_DISPLAY_LIMIT)
     },
 
     latestSearchHistoryNames: function () {

src/renderer/store/modules/search-history.js

@@ -1,9 +1,6 @@
 import { SEARCH_RESULTS_DISPLAY_LIMIT } from '../../../constants'
 import { DBSearchHistoryHandlers } from '../../../datastores/handlers/index'
 
-// maximum number of search history results to display when mixed with regular YT search suggestions
-const MIXED_SEARCH_HISTORY_ENTRIES_DISPLAY_LIMIT = 4
-
 const state = {
   searchHistoryEntries: []
 }
@@ -18,18 +15,11 @@ const getters = {
   },
 
   getLatestMatchingSearchHistoryNames: (state) => (name) => {
-    const matches = []
-    for (const entry of state.searchHistoryEntries) {
-      if (entry.name.startsWith(name)) {
-        matches.push(entry.name)
-        if (matches.length === MIXED_SEARCH_HISTORY_ENTRIES_DISPLAY_LIMIT) {
-          break
-        }
-      }
-    }
+    const matches = state.searchHistoryEntries.filter((entry) => entry.name.startsWith(name))
 
     // prioritize more concise matches
-    return matches.toSorted((a, b) => a.length - b.length)
+    return matches.map((entry) => entry.name)
+      .toSorted((a, b) => a.length - b.length)
   },
 
   getSearchHistoryEntryWithId: (state) => (id) => {

src/renderer/views/Search/Search.js

@@ -50,6 +50,10 @@ export default defineComponent({
     showFamilyFriendlyOnly: function() {
       return this.$store.getters.getShowFamilyFriendlyOnly
     },
+
+    enableSearchSuggestions: function () {
+      return this.$store.getters.getEnableSearchSuggestions
+    },
   },
   watch: {
     $route () {
@@ -145,7 +149,9 @@ export default defineComponent({
         }
       }
 
-      this.updateSearchHistoryEntry()
+      if (this.enableSearchSuggestions) {
+        this.updateSearchHistoryEntry()
+      }
     },
 
     performSearchLocal: async function (payload) {