Skip to content

Commit

Permalink
fix: Make file list only sortable by one property at the time
Browse files Browse the repository at this point in the history
* Also add a default sorting (by name ascending)
* Directories are always sorted first
* Sort by display name if available

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
  • Loading branch information
susnux committed Aug 25, 2023
1 parent bebd5c6 commit afd33fc
Show file tree
Hide file tree
Showing 5 changed files with 251 additions and 18 deletions.
6 changes: 3 additions & 3 deletions l10n/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ msgstr ""
msgid "Files and folders you recently modified will show up here."
msgstr ""

#: lib/components/FilePicker/FileList.vue:39
#: lib/components/FilePicker/FileList.vue:47
msgid "Modified"
msgstr ""

Expand All @@ -71,7 +71,7 @@ msgstr ""
msgid "Move to {target}"
msgstr ""

#: lib/components/FilePicker/FileList.vue:19
#: lib/components/FilePicker/FileList.vue:27
msgid "Name"
msgstr ""

Expand All @@ -94,7 +94,7 @@ msgstr ""
msgid "Select entry"
msgstr ""

#: lib/components/FilePicker/FileList.vue:29
#: lib/components/FilePicker/FileList.vue:37
msgid "Size"
msgstr ""

Expand Down
198 changes: 198 additions & 0 deletions lib/components/FilePicker/FileList.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
/**
* @copyright Copyright (c) 2023 Ferdinand Thiessen <opensource@fthiessen.de>
*
* @author Ferdinand Thiessen <opensource@fthiessen.de>
*
* @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 { mount } from '@vue/test-utils'
import { beforeAll, describe, expect, it, vi } from 'vitest'

import FileList from './FileList.vue'
import { File, Folder } from '@nextcloud/files'

// mock OC.MimeType
window.OC = {
MimeType: {
getIconUrl: (mime: string) => `icon/${mime}`,
},
} as never

const exampleNodes = [
new File({
owner: null,
source: 'http://example.com/dav/a-file.txt',
mime: 'text/plain',
mtime: new Date(),
root: '/',
size: 200,
}),
new File({
owner: null,
source: 'http://example.com/dav/favorite.txt',
mime: 'text/plain',
mtime: new Date(),
root: '/',
size: 321,
attributes: {
favorite: true,
},
}),
new Folder({
owner: null,
source: 'http://example.com/dav/directory',
mtime: new Date(),
root: '/',
size: 0,
}),
new File({
owner: null,
source: 'http://example.com/dav/b-file.txt',
mime: 'text/plain',
mtime: new Date(),
root: '/',
size: 100,
}),
]

describe('FilePicker FileList', () => {
beforeAll(() => {
vi.useFakeTimers()
})

it('is mountable', () => {
const consoleError = vi.spyOn(console, 'error')
const consoleWarning = vi.spyOn(console, 'warn')

const wrapper = mount(FileList, {
propsData: {
multiselect: false,
allowPickDirectory: false,
loading: false,
files: [],
selectedFiles: [],
path: '/',
},
})
expect(wrapper.html()).toBeTruthy()
// No errors or warnings
expect(consoleError).not.toBeCalled()
expect(consoleWarning).not.toBeCalled()
})

it('header checkbox is not shown if multiselect is `false`', () => {
const wrapper = mount(FileList, {
propsData: {
multiselect: false,
allowPickDirectory: false,
loading: false,
files: [],
selectedFiles: [],
path: '/',
},
})
expect(wrapper.find('th.row-checkbox').exists()).toBe(false)
})

it('header checkbox is shown if multiselect is `true`', () => {
const wrapper = mount(FileList, {
propsData: {
multiselect: true,
allowPickDirectory: false,
loading: false,
files: [],
selectedFiles: [],
path: '/',
},
})
expect(wrapper.find('th.row-checkbox').exists()).toBe(true)
// there is an aria label
expect(wrapper.find('[data-test="file-picker_select-all"]').attributes('aria-label')).toBeTruthy()
// no checked
expect(wrapper.find('[data-test="file-picker_select-all"]').props('checked')).toBe(false)
})

it('header checkbox is checked when all nodes are selected', async () => {
const nodes = [...exampleNodes]
const wrapper = mount(FileList, {
propsData: {
multiselect: true,
allowPickDirectory: false,
loading: false,
files: nodes,
selectedFiles: nodes,
path: '/',
},
})

const selectAll = wrapper.find('[data-test="file-picker_select-all"]')
expect(selectAll.props('checked')).toBe(true)
})

describe('file list sorting', () => {
it('is sorted initially by name', async () => {
const nodes = [...exampleNodes]
const wrapper = mount(FileList, {
propsData: {
multiselect: true,
allowPickDirectory: false,
loading: false,
files: nodes,
selectedFiles: [],
path: '/',
},
})

const rows = wrapper.findAll('.file-picker__row')
// all nodes are shown
expect(rows.length).toBe(nodes.length)
// folder are sorted first
expect(rows.at(0).attributes('data-file')).toBe('directory')
// other files are ascending
expect(rows.at(1).attributes('data-file')).toBe('a-file.txt')
expect(rows.at(2).attributes('data-file')).toBe('b-file.txt')
expect(rows.at(3).attributes('data-file')).toBe('favorite.txt')
})

it('can sort descending by name', async () => {
const nodes = [...exampleNodes]
const wrapper = mount(FileList, {
propsData: {
multiselect: true,
allowPickDirectory: false,
loading: false,
files: nodes,
selectedFiles: [],
path: '/',
},
})

await wrapper.find('[data-test="file-picker_sort-name"]').trigger('click')

const rows = wrapper.findAll('.file-picker__row')
// all nodes are shown
expect(rows.length).toBe(nodes.length)
// folder are sorted first
expect(rows.at(0).attributes('data-file')).toBe('directory')
// other files are descending
expect(rows.at(1).attributes('data-file')).toBe('favorite.txt')
expect(rows.at(2).attributes('data-file')).toBe('b-file.txt')
expect(rows.at(3).attributes('data-file')).toBe('a-file.txt')
})
})
})
41 changes: 26 additions & 15 deletions lib/components/FilePicker/FileList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,18 @@
<span class="hidden-visually">
{{ t('Select entry') }}
</span>
<NcCheckboxRadioSwitch v-if="multiselect" :aria-label="t('Select all entries')" :checked="allSelected" @update:checked="onSelectAll" />
<NcCheckboxRadioSwitch v-if="multiselect"
:aria-label="t('Select all entries')"
:checked="allSelected"
data-test="file-picker_select-all"
@update:checked="onSelectAll" />
</th>
<th :aria-sort="sortByName" class="row-name">
<NcButton :wide="true" type="tertiary" @click="toggleSortByName">
<NcButton
:wide="true"
type="tertiary"
data-test="file-picker_sort-name"
@click="toggleSortByName">
<template #icon>
<IconSortAscending v-if="sortByName === 'ascending'" :size="20" />
<IconSortDescending v-else-if="sortByName === 'descending'" :size="20" />
Expand Down Expand Up @@ -62,7 +70,7 @@
</template>

<script setup lang="ts">
import type { Node } from '@nextcloud/files'
import { FileType, type Node } from '@nextcloud/files'
import { getCanonicalLocale } from '@nextcloud/l10n'
import { NcButton, NcCheckboxRadioSwitch } from '@nextcloud/vue'
Expand Down Expand Up @@ -91,7 +99,7 @@ const emit = defineEmits<{
type ISortingOptions = 'ascending' | 'descending' | undefined
const sortByName = ref<ISortingOptions>(undefined)
const sortByName = ref<ISortingOptions>('ascending')
const sortBySize = ref<ISortingOptions>(undefined)
const sortByModified = ref<ISortingOptions>(undefined)
Expand All @@ -101,15 +109,17 @@ const ordering = {
none: <T>(a: T, b: T, fn: (a: T, b: T) => number) => 0,
}
const byName = (a: Node, b: Node) => b.basename.localeCompare(a.basename, getCanonicalLocale())
const byName = (a: Node, b: Node) => (a.attributes?.displayName || a.basename).localeCompare(b.attributes?.displayName || b.basename, getCanonicalLocale())
const bySize = (a: Node, b: Node) => (b.size || 0) - (a.size || 0)
const byDate = (a: Node, b: Node) => (a.mtime?.getTime() || 0) - (b.mtime?.getTime() || 0)
const toggleSorting = (variable: Ref<ISortingOptions>) => {
if (variable.value === 'ascending') {
const old = variable.value
// reset
sortByModified.value = sortBySize.value = sortByName.value = undefined
if (old === 'ascending') {
variable.value = 'descending'
} else if (variable.value === 'descending') {
variable.value = undefined
} else {
variable.value = 'ascending'
}
Expand All @@ -122,27 +132,28 @@ const toggleSortByModified = () => toggleSorting(sortByModified)
/**
* Files sorted by columns
*/
const sortedFiles = computed(() => {
const s = props.files.sort(
const sortedFiles = computed(() => [...props.files].sort(
(a, b) =>
// Folders always come above the files
(b.type === FileType.Folder ? 1 : 0) - (a.type === FileType.Folder ? 1 : 0) ||
// Favorites above other files
// (b.attributes?.favorite || false) - (a.attributes?.favorite || false) ||
// then sort by name / size / modified
ordering[sortByName.value || 'none'](a, b, byName) ||
ordering[sortBySize.value || 'none'](a, b, bySize) ||
ordering[sortByModified.value || 'none'](a, b, byDate)
)
console.warn('files sorted')
return s
}
)
/**
* Contains the selectable files, filtering out directories if `allowPickDirectory` is not set
*/
const selectableFiles = computed(() => props.files.filter((file) => props.allowPickDirectory || file.mime !== 'https/unix-directory'))
const selectableFiles = computed(() => props.files.filter((file) => props.allowPickDirectory || file.type !== FileType.Folder))
/**
* Whether all selectable files are currently selected
*/
const allSelected = computed(() => !props.loading && props.selectedFiles.length >= selectableFiles.value.length)
const allSelected = computed(() => !props.loading && props.selectedFiles.length > 0 && props.selectedFiles.length >= selectableFiles.value.length)
/**
* Handle the "select all" checkbox
Expand Down
1 change: 1 addition & 0 deletions lib/components/FilePicker/FileListRow.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
:class="['file-picker__row', {
'file-picker__row--selected': selected && !showCheckbox
}]"
:data-file="node.basename"
@key-down="handleKeyDown">
<td v-if="showCheckbox" class="row-checkbox">
<NcCheckboxRadioSwitch :disabled="!isPickable"
Expand Down
23 changes: 23 additions & 0 deletions vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { defineConfig } from 'vitest/config'
import config from './vite.config'

export default defineConfig(async (env) => {
const viteConfig = (await config(env))
delete viteConfig.define
return {
...viteConfig,
test: {
environment: 'happy-dom',
coverage: {
provider: 'istanbul',
include: ['lib/**/*.ts', 'lib/*.ts'],
exclude: ['lib/**/*.spec.ts'],
},
server: {
deps: {
inline: [/@nextcloud\/vue/],
},
},
},
}
})

0 comments on commit afd33fc

Please sign in to comment.