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 · Jan 17, 2025 · 4a9954f · 4a9954f
1 parent 326120a
commit 4a9954f
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 9 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,17 +75,25 @@ export default defineComponent({
 	},
 
 	watch: {
+		/** Reset selected options if property is changed */
+		presets() {
+			this.selectedOptions = this.presets ?? []
+		},
 		selectedOptions(newValue, oldValue) {
 			if (this.selectedOptions.length === 0) {
 				if (oldValue.length !== 0) {
-					this.$emit('update:preset')
+					this.$emit('update:presets')
 				}
 			} else {
-				this.$emit('update:preset', this.selectedOptions)
+				this.$emit('update:presets', this.selectedOptions)
 			}
 		},
 	},
 
+	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,18 +102,21 @@ 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,
 		})
-			.$on('update:preset', this.setPreset.bind(this))
+			.$on('update:presets', this.setPresets.bind(this))
 			.$mount()
 	}
 
@@ -141,8 +143,9 @@ class TypeFilter extends FileListFilter {
 		})
 	}
 
-	public setPreset(presets?: ITypePreset[]) {
-		this.currentPresets = presets
+	public setPresets(presets?: ITypePreset[]) {
+		this.currentPresets = presets ?? []
+		this.currentInstance!.$props.presets = 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.setPresets(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('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')