From 2154ee4dcd00b4709e38b718ec10cde9ddd5ea11 Mon Sep 17 00:00:00 2001
From: Jonas <jonas@freesources.org>
Date: Thu, 25 Jul 2024 17:56:59 +0200
Subject: [PATCH] fix(search): Address review comments

* Consolidate vuex search module
* Get rid of watcher + duplicated state around `matchAll`/`highlightAll`
* Scroll current search result into view

Signed-off-by: Jonas <jonas@freesources.org>
---
 src/components/PageList.vue     |  2 +-
 src/components/SearchDialog.vue | 51 ++++++++++++++++++++-------------
 src/store/search.js             | 24 ++++------------
 3 files changed, 38 insertions(+), 39 deletions(-)

diff --git a/src/components/PageList.vue b/src/components/PageList.vue
index a7a908556..74ca41043 100644
--- a/src/components/PageList.vue
+++ b/src/components/PageList.vue
@@ -285,7 +285,7 @@ export default {
 		},
 		filterString() {
 			this.getContentFilteredPagesDebounced()
-			this.setSearchQuery({ query: this.filterString })
+			this.setSearchQuery(this.filterString)
 		},
 	},
 
diff --git a/src/components/SearchDialog.vue b/src/components/SearchDialog.vue
index eb1db6620..700055342 100644
--- a/src/components/SearchDialog.vue
+++ b/src/components/SearchDialog.vue
@@ -13,7 +13,7 @@
 
 			<NcButton alignment="center-reverse"
 				:aria-label="t('collectives', 'Find previous match')"
-				@click="previousSearch">
+				@click="previous">
 				<template #icon>
 					<ArrowUp :size="20" />
 				</template>
@@ -22,7 +22,7 @@
 
 			<NcButton alignment="center-reverse"
 				:aria-label="t('collectives', 'Find next match')"
-				@click="nextSearch">
+				@click="next">
 				<template #icon>
 					<ArrowDown :size="20" />
 				</template>
@@ -48,7 +48,7 @@
 		</div>
 
 		<div class="search-dialog__highlight-all">
-			<NcCheckboxRadioSwitch :checked.sync="highlightAll">
+			<NcCheckboxRadioSwitch :checked.sync="isHighlightAllChecked">
 				{{ t('collectives', 'Highlight all matches') }}
 			</NcCheckboxRadioSwitch>
 		</div>
@@ -59,7 +59,7 @@
 import { subscribe } from '@nextcloud/event-bus'
 import { NcButton, NcCheckboxRadioSwitch } from '@nextcloud/vue'
 import { translate as t } from '@nextcloud/l10n'
-import { mapGetters, mapMutations, mapActions } from 'vuex'
+import { mapGetters, mapMutations } from 'vuex'
 import ArrowDown from 'vue-material-design-icons/ArrowDown.vue'
 import ArrowUp from 'vue-material-design-icons/ArrowUp.vue'
 import Close from 'vue-material-design-icons/Close.vue'
@@ -79,7 +79,6 @@ export default {
 		return {
 			totalMatches: null,
 			matchIndex: 0,
-			highlightAll: true,
 		}
 	},
 
@@ -88,18 +87,14 @@ export default {
 			'searchQuery',
 			'matchAll',
 		]),
-	},
 
-	watch: {
-		highlightAll() {
-			if (this.highlightAll !== this.matchAll) {
+		isHighlightAllChecked: {
+			get() {
+				return this.matchAll
+			},
+			set() {
 				this.toggleMatchAll()
-			}
-		},
-		matchAll(value) {
-			if (this.highlightAll !== value) {
-				this.highlightAll = value
-			}
+			},
 		},
 	},
 
@@ -114,13 +109,28 @@ export default {
 		t,
 		...mapMutations([
 			'setSearchQuery',
+			'toggleMatchAll',
 			'nextSearch',
 			'previousSearch',
 		]),
-		...mapActions([
-			'toggleMatchAll',
-			'clearSearch',
-		]),
+
+		previous() {
+			this.previousSearch()
+			this.scrollIntoView()
+		},
+
+		next() {
+			this.nextSearch()
+			this.scrollIntoView()
+		},
+
+		clearSearch() {
+			this.setSearchQuery('')
+		},
+
+		scrollIntoView() {
+			document.querySelector('[data-text-el="search-decoration"]')?.scrollIntoView({ block: 'center' })
+		},
 	},
 }
 </script>
@@ -130,7 +140,6 @@ $button-gap: calc(var(--default-grid-baseline) * 3);
 
 .search-dialog__container {
 	width: 100%;
-	height: 50px;
 	display: flex;
 	position: sticky;
 	align-items: center;
@@ -152,6 +161,8 @@ $button-gap: calc(var(--default-grid-baseline) * 3);
 .search-dialog__highlight-all {
 	margin-left: auto;
 	margin-right: $button-gap;
+	margin-top: $button-gap;
+	margin-bottom: $button-gap;
 	display: flex;
 	align-items: center;
 	column-gap: $button-gap;
diff --git a/src/store/search.js b/src/store/search.js
index 974e6277a..fc23b4226 100644
--- a/src/store/search.js
+++ b/src/store/search.js
@@ -16,9 +16,13 @@ export default {
 	},
 
 	mutations: {
-		setSearchQuery(state, { query, matchAll }) {
+		setSearchQuery(state, query) {
 			state.query = query
-			emit('text:editor:search', { query, matchAll })
+			emit('text:editor:search', { query: state.query, matchAll: state.matchAll })
+		},
+		toggleMatchAll(state) {
+			state.matchAll = !state.matchAll
+			emit('text:editor:search', { query: state.query, matchAll: state.matchAll })
 		},
 		nextSearch(state) {
 			state.matchAll = false
@@ -28,21 +32,5 @@ export default {
 			state.matchAll = false
 			emit('text:editor:search-previous', {})
 		},
-		toggleMatchAll(state) {
-			state.matchAll = !state.matchAll
-		},
-	},
-
-	actions: {
-		toggleMatchAll({ state, commit }) {
-			commit('toggleMatchAll')
-			commit('setSearchQuery', {
-				query: state.query,
-				matchAll: state.matchAll,
-			})
-		},
-		clearSearch({ state, commit }) {
-			commit('setSearchQuery', { query: '' })
-		},
 	},
 }