fix(FilePicker): Fix selecting rows using the keyboard

Signed-off-by: Ferdinand Thiessen <[email protected]>

lib/components/FilePicker/FileListRow.spec.ts

@@ -0,0 +1,101 @@
+/**
+ * @copyright Copyright (c) 2023 Ferdinand Thiessen <[email protected]>
+ *
+ * @author Ferdinand Thiessen <[email protected]>
+ *
+ * @license AGPL-3.0-or-later
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import { mount } from '@vue/test-utils'
+import { File } from '@nextcloud/files'
+
+import FileListRow from './FileListRow.vue'
+
+// Mock OC.MimeType
+window.OC = {
+	MimeType: {
+		getIconUrl: (mime: string) => `/icon/${mime}`,
+	},
+} as never
+
+describe('FilePicker: FileListRow', () => {
+	const node = new File({
+		owner: null,
+		mtime: new Date(),
+		mime: 'text/plain',
+		source: 'https://example.com/dav/a.txt',
+		root: '/',
+	})
+
+	afterEach(() => {
+		vi.restoreAllMocks()
+	})
+
+	it('is mountable', () => {
+		const consoleWarn = vi.spyOn(console, 'warn')
+		const consoleError = vi.spyOn(console, 'error')
+
+		const wrapper = mount(FileListRow, {
+			propsData: {
+				allowPickDirectory: false,
+				selected: false,
+				canPick: true,
+				node,
+			},
+		})
+
+		// No console errors
+		expect(consoleWarn).not.toBeCalled()
+		expect(consoleError).not.toBeCalled()
+		// mounted
+		expect(wrapper.isEmpty()).toBe(false)
+		expect(wrapper.element.tagName.toLowerCase()).toBe('tr')
+		expect(wrapper.element.classList.contains('file-picker__row')).toBe(true)
+	})
+
+	it('Click triggers select', async () => {
+		const wrapper = mount(FileListRow, {
+			propsData: {
+				allowPickDirectory: false,
+				selected: false,
+				canPick: true,
+				node,
+			},
+		})
+
+		await wrapper.find('.row-checkbox *').trigger('click')
+
+		// one event with payload `true` is expected
+		expect(wrapper.emitted('update:selected')).toEqual([[true]])
+	})
+
+	it('Enter triggers select', async () => {
+		const wrapper = mount(FileListRow, {
+			propsData: {
+				allowPickDirectory: false,
+				selected: false,
+				canPick: true,
+				node,
+			},
+		})
+
+		await wrapper.trigger('keydown', { key: 'Enter', bubbles: true })
+
+		expect(wrapper.emitted('update:selected')).toEqual([[true]])
+	})
+})

lib/components/FilePicker/FileListRow.vue

@@ -1,16 +1,22 @@
 <template>
-	<tr tabindex="0"
+	<tr :tabindex="showCheckbox && !isDirectory ? undefined : 0"
+		:aria-selected="!isPickable ? undefined : selected"
 		:class="['file-picker__row', {
 			'file-picker__row--selected': selected && !showCheckbox
 		}]"
-		@key-down="handleKeyDown">
+		v-on="{
+			// same as tabindex -> if we hide the checkbox or this is a directory we need keyboard access to enter the directory or select the node
+			keydown: showCheckbox && !isDirectory ? null : handleKeyDown,
+			click: handleClick
+		}">
 		<td v-if="showCheckbox" class="row-checkbox">
 			<NcCheckboxRadioSwitch :disabled="!isPickable"
 				:checked="selected"
 				:aria-label="t('Select the row for {nodename}', { nodename: displayName })"
+				@click.stop="/* just stop the click event */"
 				@update:checked="toggleSelected" />
 		</td>
-		<td class="row-name" @click="handleClick">
+		<td class="row-name">
 			<div class="file-picker__name-container">
 				<div class="file-picker__file-icon" :style="{ backgroundImage }" />
 				<div class="file-picker__file-name" :title="displayName" v-text="displayName" />
@@ -26,7 +32,7 @@
 	</tr>
 </template>
 <script setup lang="ts">
-import { type Node, formatFileSize } from '@nextcloud/files'
+import { type Node, formatFileSize, FileType } from '@nextcloud/files'
 import { NcCheckboxRadioSwitch, NcDatetime } from '@nextcloud/vue'
 import { computed } from 'vue'
 import { t } from '../../utils/l10n'
@@ -61,10 +67,15 @@ const displayName = computed(() => props.node.attributes?.displayName || props.n
  */
 const fileExtension = computed(() => props.node.extension)
 
+/**
+ * Check if the node is a directory
+ */
+const isDirectory = computed(() => props.node.type === FileType.Folder)
+
 /**
  * If this node can be picked, basically just check if picking a directory is allowed
  */
-const isPickable = computed(() => props.canPick && (props.allowPickDirectory || props.node.mime !== 'httpd/unix-directory'))
+const isPickable = computed(() => props.canPick && (props.allowPickDirectory || !isDirectory.value))
 
 /**
  * Background image url for the given nodes mime type
@@ -82,7 +93,7 @@ function toggleSelected() {
  * Handle clicking the table row, if it is a directory it is opened, else selected
  */
 function handleClick() {
-	if (props.node.mime === 'httpd/unix-directory') {
+	if (isDirectory.value) {
 		emit('enter-directory', props.node)
 	} else {
 		toggleSelected()
@@ -94,7 +105,7 @@ function handleClick() {
  * @param event The Keydown event
  */
 function handleKeyDown(event: KeyboardEvent) {
-	if (event.key === 'enter') {
+	if (event.key === 'Enter') {
 		handleClick()
 	}
 }

vitest.config.ts

@@ -4,6 +4,7 @@ import config from './vite.config'
 export default defineConfig(async (env) => {
 	const viteConfig = (await config(env))
 	delete viteConfig.define
+
 	return {
 		...viteConfig,
 		test: {
@@ -13,6 +14,12 @@ export default defineConfig(async (env) => {
 				include: ['lib/**/*.ts', 'lib/*.ts'],
 				exclude: ['lib/**/*.spec.ts'],
 			},
+			// Fix unresolvable .css extension for ssr
+			server: {
+				deps: {
+					inline: [/@nextcloud\/vue/],
+				},
+			},
 		},
 	}
 })