From 10e17ef290e07550e2b96c94f5c2a20576fd9862 Mon Sep 17 00:00:00 2001
From: Jason Henriquez <jasonhenriquez@protonmail.com>
Date: Thu, 19 Dec 2024 20:59:07 -0600
Subject: [PATCH] Implement fixes from code review

Fixes clear text button to appear and stay visible when the dropdown options are visible in an ft-input. Fixes updateVisibleDataList not using trim(), thus incorrectly causing filtering of search history when space bar was used.
---
 src/renderer/components/ft-input/ft-input.css | 13 ++++++-------
 src/renderer/components/ft-input/ft-input.js  |  6 +++++-
 src/renderer/components/ft-input/ft-input.vue |  7 ++++---
 src/renderer/components/top-nav/top-nav.js    | 17 +++++++++++------
 src/renderer/store/modules/search-history.js  |  4 ++--
 5 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/src/renderer/components/ft-input/ft-input.css b/src/renderer/components/ft-input/ft-input.css
index 4db1aa301d293..799dab09e0153 100644
--- a/src/renderer/components/ft-input/ft-input.css
+++ b/src/renderer/components/ft-input/ft-input.css
@@ -31,12 +31,15 @@ body[dir='rtl'] .ft-input-component.search.showClearTextButton:focus-within .inp
   padding-inline-start: 46px;
 }
 
-.ft-input-component:focus-within .clearInputTextButton {
+.ft-input-component:focus-within .clearInputTextButton,
+.ft-input-component.showOptions .clearInputTextButton {
   opacity: 0.5;
 }
 
-.clearTextButtonVisible .clearInputTextButton.visible,
-.ft-input-component:focus-within .clearInputTextButton.visible {
+
+.ft-input-component.inputDataPresent .clearInputTextButton.visible,
+.clearTextButtonVisible:not(.showOptions) .clearInputTextButton.visible,
+.ft-input-component:focus-within:not(.showOptions) .clearInputTextButton.visible {
   cursor: pointer;
   opacity: 1;
 }
@@ -209,10 +212,6 @@ body[dir='rtl'] .ft-input-component.search.showClearTextButton:focus-within .inp
   white-space: nowrap;
 }
 
-.bookmarkStarIcon {
-  color: var(--favorite-icon-color);
-}
-
 .searchResultIcon {
   opacity: 0.6;
   padding-inline-end: 10px;
diff --git a/src/renderer/components/ft-input/ft-input.js b/src/renderer/components/ft-input/ft-input.js
index b249a35b128dc..e68bb944954ae 100644
--- a/src/renderer/components/ft-input/ft-input.js
+++ b/src/renderer/components/ft-input/ft-input.js
@@ -96,6 +96,10 @@ export default defineComponent({
     }
   },
   computed: {
+    showOptions: function () {
+      return (this.inputData !== '' || this.showDataWhenEmpty) && this.visibleDataList.length > 0 && this.searchState.showOptions
+    },
+
     barColor: function () {
       return this.$store.getters.getBarColor
     },
@@ -313,7 +317,7 @@ export default defineComponent({
       // Reset selected option before it's updated
       this.searchState.selectedOption = -1
       this.searchState.keyboardSelectedOptionIndex = -1
-      if (this.inputData === '') {
+      if (this.inputData.trim() === '') {
         this.visibleDataList = this.dataList
         return
       }
diff --git a/src/renderer/components/ft-input/ft-input.vue b/src/renderer/components/ft-input/ft-input.vue
index 2482b697d0de5..d58de72a61313 100644
--- a/src/renderer/components/ft-input/ft-input.vue
+++ b/src/renderer/components/ft-input/ft-input.vue
@@ -7,7 +7,8 @@
       forceTextColor: forceTextColor,
       showActionButton: showActionButton,
       showClearTextButton: showClearTextButton,
-      clearTextButtonVisible: inputDataPresent,
+      clearTextButtonVisible: inputDataPresent || showOptions,
+      showOptions: showOptions,
       disabled: disabled
     }"
   >
@@ -29,7 +30,7 @@
       :icon="['fas', 'times-circle']"
       class="clearInputTextButton"
       :class="{
-        visible: inputDataPresent
+        visible: inputDataPresent || showOptions
       }"
       tabindex="0"
       role="button"
@@ -68,7 +69,7 @@
     </span>
     <div class="options">
       <ul
-        v-if="(inputData !== '' || showDataWhenEmpty) && visibleDataList.length > 0 && searchState.showOptions"
+        v-if="showOptions"
         class="list"
         @mouseenter="searchState.isPointerInList = true"
         @mouseleave="searchState.isPointerInList = false"
diff --git a/src/renderer/components/top-nav/top-nav.js b/src/renderer/components/top-nav/top-nav.js
index 22d4f837a9d6b..3907d4237bd90 100644
--- a/src/renderer/components/top-nav/top-nav.js
+++ b/src/renderer/components/top-nav/top-nav.js
@@ -127,7 +127,7 @@ export default defineComponent({
       if (!this.enableSearchSuggestions) {
         return
       }
-      return this.lastSuggestionQuery === '' ? this.$store.getters.getLatestUniqueSearchHistoryEntries(this.$router.currentRoute.fullPath) : this.searchSuggestionsDataList
+      return this.lastSuggestionQuery === '' ? this.$store.getters.getLatestUniqueSearchHistoryEntries : this.searchSuggestionsDataList
     },
   },
   watch: {
@@ -180,11 +180,16 @@ export default defineComponent({
 
       if (isFreeTubeInternalQuery) {
         const adjustedQuery = queryText.substring(3)
-        openInternalPath({
-          path: adjustedQuery,
-          adjustedQuery,
-          doCreateNewWindow
-        })
+        if (this.$router.currentRoute.fullPath !== adjustedQuery) {
+          openInternalPath({
+            path: adjustedQuery,
+            adjustedQuery,
+            doCreateNewWindow
+          })
+        }
+
+        // update in-use search query to the selected search history entry name
+        this.lastSuggestionQuery = this.$store.getters.getSearchHistoryEntryWithRoute(adjustedQuery)?.name ?? ''
         return
       }
 
diff --git a/src/renderer/store/modules/search-history.js b/src/renderer/store/modules/search-history.js
index 870c738095661..d931a3fcd6575 100644
--- a/src/renderer/store/modules/search-history.js
+++ b/src/renderer/store/modules/search-history.js
@@ -12,10 +12,10 @@ const getters = {
     return state.searchHistoryEntries
   },
 
-  getLatestUniqueSearchHistoryEntries: (state) => (routeToExclude) => {
+  getLatestUniqueSearchHistoryEntries: (state) => {
     const nameSet = new Set()
     return state.searchHistoryEntries.filter((entry) => {
-      if (nameSet.has(entry.name) || routeToExclude === entry.route) {
+      if (nameSet.has(entry.name)) {
         return false
       }