fix(files): File type filter UI sync with filter state

When changing the folder the filter will be re-mounted by the file list, so we need to pass the current state of the filter to the filter UI. Signed-off-by: Ferdinand Thiessen <[email protected]>
nextcloud · Nov 17, 2024 · 7be3fd3 · 7be3fd3
1 parent 714fc63
commit 7be3fd3
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 5 deletions.
diff --git a/apps/files/src/components/FileListFilter/FileListFilterType.vue b/apps/files/src/components/FileListFilter/FileListFilterType.vue
@@ -45,6 +45,10 @@ export default defineComponent({
 	},
 
 	props: {
+		presets: {
+			type: Array as PropType<ITypePreset[]>,
+			default: () => [],
+		},
 		typePresets: {
 			type: Array as PropType<ITypePreset[]>,
 			required: true,
@@ -71,6 +75,10 @@ export default defineComponent({
 	},
 
 	watch: {
+		/** Reset selected options if property is changed */
+		preset() {
+			this.selectedOptions = this.presets ?? []
+		},
 		selectedOptions(newValue, oldValue) {
 			if (this.selectedOptions.length === 0) {
 				if (oldValue.length !== 0) {
@@ -82,6 +90,10 @@ export default defineComponent({
 		},
 	},
 
+	mounted() {
+		this.selectedOptions = this.presets ?? []
+	},
+
 	methods: {
 		resetFilter() {
 			this.selectedOptions = []

diff --git a/apps/files/src/filters/TypeFilter.ts b/apps/files/src/filters/TypeFilter.ts
@@ -4,7 +4,6 @@
  */
 import type { IFileListFilterChip, INode } from '@nextcloud/files'
 
-import { subscribe } from '@nextcloud/event-bus'
 import { FileListFilter, registerFileListFilter } from '@nextcloud/files'
 import { t } from '@nextcloud/l10n'
 import Vue from 'vue'
@@ -89,12 +88,12 @@ const getTypePresets = async () => [
 class TypeFilter extends FileListFilter {
 
 	private currentInstance?: Vue
-	private currentPresets?: ITypePreset[]
+	private currentPresets: ITypePreset[]
 	private allPresets?: ITypePreset[]
 
 	constructor() {
 		super('files:type', 10)
-		subscribe('files:navigation:changed', () => this.setPreset())
+		this.currentPresets = []
 	}
 
 	public async mount(el: HTMLElement) {
@@ -103,13 +102,16 @@ class TypeFilter extends FileListFilter {
 			this.allPresets = await getTypePresets()
 		}
 
+		// Already mounted
 		if (this.currentInstance) {
 			this.currentInstance.$destroy()
+			delete this.currentInstance
 		}
 
 		const View = Vue.extend(FileListFilterType as never)
 		this.currentInstance = new View({
 			propsData: {
+				presets: this.currentPresets,
 				typePresets: this.allPresets!,
 			},
 			el,
@@ -142,7 +144,8 @@ class TypeFilter extends FileListFilter {
 	}
 
 	public setPreset(presets?: ITypePreset[]) {
-		this.currentPresets = presets
+		this.currentPresets = presets ?? []
+		this.currentInstance!.$props.preset = presets
 		this.filterUpdated()
 
 		const chips: IFileListFilterChip[] = []
@@ -151,7 +154,7 @@ class TypeFilter extends FileListFilter {
 				chips.push({
 					icon: preset.icon,
 					text: preset.label,
-					onclick: () => this.setPreset(presets.filter(({ id }) => id !== preset.id)),
+					onclick: () => this.removeFilterPreset(preset.id),
 				})
 			}
 		} else {
@@ -160,6 +163,16 @@ class TypeFilter extends FileListFilter {
 		this.updateChips(chips)
 	}
 
+	/**
+	 * Helper callback that removed a preset from selected.
+	 * This is used when clicking on "remove" on a filter-chip.
+	 * @param presetId Id of preset to remove
+	 */
+	private removeFilterPreset(presetId: string) {
+		const filtered = this.currentPresets.filter(({ id }) => id !== presetId)
+		this.setPreset(filtered)
+	}
+
 }
 
 /**

diff --git a/cypress/e2e/files/files-filtering.cy.ts b/cypress/e2e/files/files-filtering.cy.ts
@@ -201,6 +201,55 @@ describe('files: Filter in files list', { testIsolation: true }, () => {
 		getRowForFile('text.txt').should('not.exist')
 	})
 
+	/** Regression test of https://github.com/nextcloud/server/issues/47251 */
+	it.only('keeps filter state when changing the directory', () => {
+		// files are visible
+		getRowForFile('folder').should('be.visible')
+		getRowForFile('file.txt').should('be.visible')
+
+		// enable type filter for folders
+		filesFilters.filterContainter()
+			.findByRole('button', { name: 'Type' })
+			.should('be.visible')
+			.click()
+		cy.findByRole('menuitemcheckbox', { name: 'Folders' })
+			.should('be.visible')
+			.click()
+		// assert the button is checked
+		cy.findByRole('menuitemcheckbox', { name: 'Folders' })
+			.should('have.attr', 'aria-checked', 'true')
+		// close the menu
+		filesFilters.filterContainter()
+			.findByRole('button', { name: 'Type' })
+			.click()
+
+		// See the chips are active
+		filesFilters.activeFilters()
+			.should('have.length', 1)
+			.contains(/Folder/).should('be.visible')
+
+		// See that folder is visible but file not
+		getRowForFile('folder').should('be.visible')
+		getRowForFile('file.txt').should('not.exist')
+
+		// Change the directory
+		navigateToFolder('folder')
+		getRowForFile('folder').should('not.exist')
+
+		// See that the chip is still 
+		filesFilters.activeFilters()
+			.should('have.length', 1)
+			.contains(/Folder/).should('be.visible')
+		// And also the button should be active
+		filesFilters.filterContainter()
+			.findByRole('button', { name: 'Type' })
+			.should('be.visible')
+			.click()
+		cy.findByRole('menuitemcheckbox', { name: 'Folders' })
+			.should('be.visible')
+			.and('have.attr', 'aria-checked', 'true')
+	})
+
 	it('resets filter when changing the view', () => {
 		// All are visible by default
 		getRowForFile('folder').should('be.visible')