Merge pull request #2033 from nextcloud-libraries/artonge/feat/add_canpick_option

feat(FilePicker): Improve pick and navigation logic to match user expectation

Author: artonge

lib/components/FilePicker/FileList.vue

@@ -67,7 +67,7 @@
 						:key="file.fileid || file.path"
 						:allow-pick-directory="allowPickDirectory"
 						:show-checkbox="multiselect"
-						:can-pick="multiselect || selectedFiles.length === 0 || selectedFiles.includes(file)"
+						:can-pick="(multiselect || selectedFiles.length === 0 || selectedFiles.includes(file)) && (canPick === undefined || canPick(file))"
 						:selected="selectedFiles.includes(file)"
 						:node="file"
 						:crop-image-previews="cropImagePreviews"
@@ -82,6 +82,7 @@
 <script setup lang="ts">
 import type { FilesSortingMode, INode } from '@nextcloud/files'
 import type { FileListViews } from '../../composables/filesSettings.ts'
+import type { IFilePickerCanPick } from '../types.ts'
 
 import { FileType, sortNodes } from '@nextcloud/files'
 import { computed, nextTick, onMounted, onUnmounted, ref } from 'vue'
@@ -125,6 +126,10 @@ const props = defineProps<{
 	 * Files to show
 	 */
 	files: INode[]
+	/**
+	 * Custom function to decide if a node can be picked
+	 */
+	canPick?: IFilePickerCanPick
 }>()
 
 /// sorting related stuff

lib/components/FilePicker/FileListRow.spec.ts

@@ -3,22 +3,109 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-import { File } from '@nextcloud/files'
+import type { VueWrapper } from '@vue/test-utils'
+import type { ComponentProps } from 'vue-component-type-helpers'
+
+import { File, Folder, Permission } from '@nextcloud/files'
 import { shallowMount } from '@vue/test-utils'
 import { afterEach, describe, expect, it, vi } from 'vitest'
 import { nextTick } from 'vue'
 import FileListRow from './FileListRow.vue'
 
-describe('FilePicker: FileListRow', () => {
-	const node = new File({
-		owner: null,
-		mtime: new Date(),
-		mime: 'text/plain',
-		source: 'https://example.com/dav/a.txt',
-		root: '/',
-		attributes: { displayName: 'test' },
+type SubmitAction = (wrapper: VueWrapper<any>) => Promise<void>
+type ElementEvent = { 'update:selected': boolean | undefined, enterDirectory: Folder | undefined }
+
+async function clickCheckboxAction(wrapper: VueWrapper<any>) {
+	wrapper.find('input[type="checkbox"]').trigger('click')
+}
+
+async function clickElementAction(wrapper: VueWrapper<any>) {
+	wrapper.find('[data-testid="row-name"]').trigger('click')
+}
+
+async function pressEnterAction(wrapper: VueWrapper<any>) {
+	wrapper.element.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Enter' }))
+	await nextTick()
+}
+
+function testSubmitNode(name: string, props: ComponentProps<typeof FileListRow>, eventPayload: ElementEvent, actionCallback: SubmitAction) {
+	it(name, async () => {
+		const wrapper = shallowMount(FileListRow, {
+			props,
+			global: {
+				stubs: {
+					NcCheckboxRadioSwitch: {
+						template: '<label><input type="checkbox" @click="$emit(\'update:model-value\', true)" ></label>',
+					},
+				},
+			},
+		})
+
+		await actionCallback(wrapper)
+
+		for (const [event, payload] of Object.entries(eventPayload)) {
+			if (payload === undefined) {
+				expect(wrapper.emitted(event)).toBeUndefined()
+			} else {
+				expect(wrapper.emitted(event)).toEqual([[payload]])
+			}
+		}
 	})
+}
+
+const node = new File({
+	owner: 'alice',
+	mtime: new Date(),
+	mime: 'text/plain',
+	source: 'https://example.com/remote.php/dav/alice/a.txt',
+	root: '/',
+	attributes: { displayName: 'test' },
+})
 
+const folder = new Folder({
+	owner: 'alice',
+	mtime: new Date(),
+	mime: 'httpd/unix-directory',
+	source: 'https://example.com/remote.php/dav/alice/b',
+	root: '/',
+	permissions: Permission.ALL,
+	attributes: { displayName: 'test folder' },
+})
+
+const folderNonReadable = new Folder({
+	owner: 'alice',
+	mtime: new Date(),
+	mime: 'httpd/unix-directory',
+	source: 'https://example.com/remote.php/dav/alice/b',
+	root: '/',
+	permissions: Permission.ALL & ~Permission.READ,
+	attributes: { displayName: 'test folder' },
+})
+
+const defaultOptions = {
+	selected: false,
+	cropImagePreviews: true,
+	canPick: true,
+	showCheckbox: true,
+	allowPickDirectory: true,
+}
+
+const noEmits = {
+	'update:selected': undefined,
+	enterDirectory: undefined,
+}
+
+const selectNode = {
+	'update:selected': true,
+	enterDirectory: undefined,
+}
+
+const navigateToFolder = {
+	'update:selected': undefined,
+	enterDirectory: folder,
+}
+
+describe('FilePicker: FileListRow', () => {
 	afterEach(() => {
 		vi.restoreAllMocks()
 	})
@@ -64,82 +151,73 @@ describe('FilePicker: FileListRow', () => {
 		expect(wrapper.find('[data-testid="row-checkbox"]').exists()).toBe(false)
 	})
 
-	it('Click checkbox triggers select', async () => {
-		const wrapper = shallowMount(FileListRow, {
-			props: {
-				allowPickDirectory: false,
-				selected: false,
-				showCheckbox: true,
-				canPick: true,
-				node,
-				cropImagePreviews: true,
-			},
-			global: {
-				stubs: {
-					NcCheckboxRadioSwitch: {
-						template: '<label><input type="checkbox" @click="$emit(\'update:model-value\', true)" ></label>',
-					},
-				},
-			},
+	describe('when node is a file', () => {
+		const fileOptions = {
+			...defaultOptions,
+			node,
+		}
+
+		testSubmitNode('Click checkbox triggers select', { ...fileOptions }, selectNode, clickCheckboxAction)
+		testSubmitNode('Click element triggers select', { ...fileOptions }, selectNode, clickElementAction)
+		testSubmitNode('Click element without checkbox triggers select', { ...fileOptions, showCheckbox: false }, selectNode, clickElementAction)
+		testSubmitNode('Enter triggers select', { ...fileOptions, showCheckbox: false }, selectNode, pressEnterAction)
+
+		describe('canPick: false', () => {
+			const options = {
+				...fileOptions,
+				canPick: false,
+			}
+
+			testSubmitNode('Click checkbox does not triggers select', options, noEmits, clickCheckboxAction)
+			testSubmitNode('Click element does not triggers select', options, noEmits, clickElementAction)
+			testSubmitNode('Click element without checkbox does not triggers select', { ...options, showCheckbox: false }, noEmits, clickElementAction)
+			testSubmitNode('Enter does not triggers select', { ...options, showCheckbox: false }, noEmits, pressEnterAction)
 		})
-
-		await wrapper.find('input[type="checkbox"]').trigger('click')
-
-		// one event with payload `true` is expected
-		expect(wrapper.emitted('update:selected')).toEqual([[true]])
 	})
 
-	it('Click element triggers select', async () => {
-		const wrapper = shallowMount(FileListRow, {
-			props: {
-				allowPickDirectory: false,
-				selected: false,
-				showCheckbox: true,
-				canPick: true,
-				node,
-				cropImagePreviews: true,
-			},
+	describe('when node is a folder', () => {
+		const folderOptions = {
+			...defaultOptions,
+			node: folder,
+		}
+
+		testSubmitNode('Click checkbox triggers select', folderOptions, selectNode, clickCheckboxAction)
+		testSubmitNode('Click element navigates to it', folderOptions, navigateToFolder, clickElementAction)
+		testSubmitNode('Click element without checkbox navigates to it', { ...folderOptions, showCheckbox: false }, navigateToFolder, clickElementAction)
+		testSubmitNode('Enter navigates to it', { ...folderOptions, showCheckbox: false }, navigateToFolder, pressEnterAction)
+
+		describe('canPick: false', () => {
+			const options = {
+				...folderOptions,
+				canPick: false,
+			}
+
+			testSubmitNode('Click checkbox does not triggers select', options, noEmits, clickCheckboxAction)
+			testSubmitNode('Click element navigates to it', options, navigateToFolder, clickElementAction)
+			testSubmitNode('Click element without checkbox navigates to it', { ...options, showCheckbox: false }, navigateToFolder, clickElementAction)
+			testSubmitNode('Enter navigates to it', { ...options, showCheckbox: false }, navigateToFolder, pressEnterAction)
 		})
 
-		await wrapper.find('[data-testid="row-name"]').trigger('click')
+		describe('without READ permissions', () => {
+			const options = {
+				...folderOptions,
+				node: folderNonReadable,
+			}
 
-		// one event with payload `true` is expected
-		expect(wrapper.emitted('update:selected')).toEqual([[true]])
-	})
-
-	it('Click element without checkbox triggers select', async () => {
-		const wrapper = shallowMount(FileListRow, {
-			props: {
-				allowPickDirectory: false,
-				selected: false,
-				showCheckbox: false,
-				canPick: true,
-				node,
-				cropImagePreviews: true,
-			},
+			testSubmitNode('Click checkbox triggers select', options, selectNode, clickCheckboxAction)
+			testSubmitNode('Click element does not navigates to it', options, noEmits, clickElementAction)
+			testSubmitNode('Click element without checkbox does not navigates to it', { ...options, showCheckbox: false }, noEmits, clickElementAction)
+			testSubmitNode('Enter does not navigates to it', { ...options, showCheckbox: false }, noEmits, pressEnterAction)
 		})
 
-		await wrapper.find('[data-testid="row-name"]').trigger('click')
-
-		// one event with payload `true` is expected
-		expect(wrapper.emitted('update:selected')).toEqual([[true]])
-	})
-
-	it('Enter triggers select', async () => {
-		const wrapper = shallowMount(FileListRow, {
-			props: {
+		describe('allowPickDirectory: false', () => {
+			const options = {
+				...folderOptions,
+				node: folderNonReadable,
 				allowPickDirectory: false,
-				selected: false,
-				showCheckbox: false,
-				canPick: true,
-				node,
-				cropImagePreviews: true,
-			},
-		})
+			}
 
-		wrapper.element.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Enter' }))
-		await nextTick()
-
-		expect(wrapper.emitted('update:selected')).toEqual([[true]])
+			testSubmitNode('Click checkbox does not triggers select', options, noEmits, clickCheckboxAction)
+		})
 	})
 })

lib/components/FilePicker/FileListRow.vue

@@ -10,6 +10,8 @@
 		:class="[
 			{
 				'file-picker__row--selected': selected && !showCheckbox,
+				'file-picker__row--not-navigatable': isDirectory && !isNavigatable,
+				'file-picker__row--not-pickable': !isPickable,
 			},
 		]"
 		:data-filename="node.basename"
@@ -46,7 +48,7 @@
 <script setup lang="ts">
 import type { INode } from '@nextcloud/files'
 
-import { FileType, formatFileSize } from '@nextcloud/files'
+import { FileType, formatFileSize, Permission } from '@nextcloud/files'
 import { computed } from 'vue'
 import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwitch'
 import NcDateTime from '@nextcloud/vue/components/NcDateTime'
@@ -97,10 +99,19 @@ const isDirectory = computed(() => props.node.type === FileType.Folder)
  */
 const isPickable = computed(() => props.canPick && (props.allowPickDirectory || !isDirectory.value))
 
+/**
+ * If this node is not readable, then we cannot navigate to it.
+ */
+const isNavigatable = computed(() => (props.node.permissions & Permission.READ) === Permission.READ)
+
 /**
  * Toggle the selection state
  */
 function toggleSelected() {
+	if (!isPickable.value) {
+		return
+	}
+
 	emit('update:selected', !props.selected)
 }
 
@@ -109,7 +120,9 @@ function toggleSelected() {
  */
 function handleClick() {
 	if (isDirectory.value) {
-		emit('enterDirectory', props.node)
+		if (isNavigatable.value) {
+			emit('enterDirectory', props.node)
+		}
 	} else {
 		toggleSelected()
 	}
@@ -132,9 +145,27 @@ function handleKeyDown(event: KeyboardEvent) {
 
 .file-picker {
 	&__row {
+		* {
+			cursor: pointer;
+		}
+
 		&--selected {
 			background-color: var(--color-background-dark);
 		}
+
+		&--not-navigatable {
+			filter: saturate(0.7);
+			opacity: 0.7;
+		}
+
+		&--not-navigatable,
+		&--not-pickable {
+
+			* {
+				cursor: default !important;
+			}
+		}
+
 		&:hover {
 			background-color: var(--color-background-hover);
 		}