Skip to content

Commit

Permalink
perf: simplify table selection-checkbox model
Browse files Browse the repository at this point in the history
The checkbox for selection in the ResourceTable had the selected
resources as model, which lead to the virtual dom of all checkboxes
being invalidated if the selection got changed at all (adding / removing
one item to/from the selection). Changed the checkbox model to only
reflect the selection state of the one resource it is about. With this
only the virtual dom of the checkboxes gets invlidated that were added
to or removed from the selection.
  • Loading branch information
kulmann committed Jul 28, 2022
1 parent cecf399 commit b53f23d
Show file tree
Hide file tree
Showing 15 changed files with 83 additions and 64 deletions.
7 changes: 0 additions & 7 deletions changelog/unreleased/bugfix-table-lazy-loading-performance

This file was deleted.

8 changes: 8 additions & 0 deletions changelog/unreleased/bugfix-table-render-performance
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Bugfix: File list render performance

We've drastically increased the initial render performance of the files list by removing the lazy loading delay and by moving the loading visualization from the OcTd to the OcTr component. For the selection of files there also has been a slight improvement in render speed.

https://github.com/owncloud/web/issues/7038
https://github.com/owncloud/web/pull/7298
https://github.com/owncloud/web/pull/7312
https://github.com/owncloud/web/pull/7367
55 changes: 30 additions & 25 deletions packages/web-app-files/src/components/FilesList/ResourceTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
:drag-drop="dragDrop"
:hover="hover"
:item-dom-selector="resourceDomSelector"
:selection="selection"
:selection="selectedResources"
:sort-by="sortBy"
:sort-dir="sortDir"
:lazy="lazyLoading"
Expand Down Expand Up @@ -40,10 +40,9 @@
:label="getResourceCheckboxLabel(item)"
:hide-label="true"
size="large"
:value="selection"
:option="item"
:value="isResourceSelected(item)"
:outline="isLatestSelectedItem(item)"
@input="emitSelect"
@input="setSelection($event, item)"
@click.native.stop
/>
</template>
Expand Down Expand Up @@ -200,7 +199,7 @@ const mapResourceFields = (resource: Resource, mapping = {}) => {
export default defineComponent({
mixins: [Rename],
model: {
prop: 'selection',
prop: 'selectedIds',
event: 'select'
},
props: {
Expand Down Expand Up @@ -250,7 +249,7 @@ export default defineComponent({
/**
* V-model for the selection
*/
selection: {
selectedIds: {
type: Array,
default: () => []
},
Expand Down Expand Up @@ -540,10 +539,10 @@ export default defineComponent({
return this.configuration?.options?.displayResourcesLazy
},
areAllResourcesSelected() {
return this.selection.length === this.resources.length
return this.selectedIds.length === this.resources.length
},
selectedIds() {
return this.selection.map((r) => r.id)
selectedResources() {
return this.resources.filter((resource) => this.selectedIds.includes(resource.id))
},
allResourcesCheckboxLabel() {
return this.$gettext('Select all resources')
Expand All @@ -560,6 +559,9 @@ export default defineComponent({
},
methods: {
...mapActions('Files/sidebar', ['openWithPanel']),
isResourceSelected(item) {
return this.selectedIds.includes(item.id)
},
isLatestSelectedItem(item) {
return item.id === this.latestSelectedId
},
Expand Down Expand Up @@ -625,27 +627,27 @@ export default defineComponent({
this.$emit('sort', opts)
},
addSelectedResource(file) {
const isSelected = this.selection.some((e) => e.id === file.id)
if (!isSelected) {
this.$emit('select', this.selection.concat([file]))
const isSelected = this.isResourceSelected(file)
if (isSelected) {
this.$emit('select', this.selectedIds)
} else {
this.$emit('select', this.selection)
this.$emit('select', this.selectedIds.concat([file]))
}
},
resetDropPosition(id, event, item) {
const instance = this.$refs[id].tippy
if (instance === undefined) return
if (!this.selection.includes(item)) {
this.emitSelect([item])
if (!this.isResourceSelected(item)) {
this.emitSelect([item.id])
}
this.displayPositionedDropdown(instance, event)
},
showContextMenu(row, event, item) {
event.preventDefault()
const instance = row.$el.getElementsByClassName('resource-table-btn-action-dropdown')[0]
if (instance === undefined) return
if (!this.selection.includes(item)) {
this.emitSelect([item])
if (!this.isResourceSelected(item)) {
this.emitSelect([item.id])
}
this.displayPositionedDropdown(instance._tippy, event)
},
Expand Down Expand Up @@ -691,7 +693,7 @@ export default defineComponent({
if (eventData && eventData.shiftKey) {
return bus.publish('app.files.list.clicked.shift', resource)
}
return this.emitSelect([resource])
return this.emitSelect([resource.id])
},
formatDate(date) {
return DateTime.fromJSDate(new Date(date))
Expand All @@ -701,19 +703,22 @@ export default defineComponent({
formatDateRelative(date) {
return DateTime.fromJSDate(new Date(date)).setLocale(this.currentLanguage).toRelative()
},
emitSelect(resources) {
/**
* Triggered when a checkbox for selecting a resource or the checkbox for selecting all resources is clicked
* @property {array} resources The selected resources
*/
setSelection(selected, resource) {
if (selected) {
this.emitSelect([...this.selectedIds, resource.id])
} else {
this.emitSelect(this.selectedIds.filter((id) => id !== resource.id))
}
},
emitSelect(selectedIds) {
bus.publish('app.files.list.clicked')
this.$emit('select', resources)
this.$emit('select', selectedIds)
},
toggleSelectionAll() {
if (this.areAllResourcesSelected) {
return this.emitSelect([])
}
this.emitSelect(this.resources)
this.emitSelect(this.resources.map((resource) => resource.id))
},
emitFileClick(resource) {
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
</no-content-message>
<resource-table
v-else
v-model="itemsSelected"
v-model="selectedResourcesIds"
:data-test-share-status="shareStatus"
class="files-table"
:class="{ 'files-table-squashed': !sidebarClosed }"
Expand Down Expand Up @@ -58,7 +58,7 @@
</div>
</template>
<template #contextMenu>
<context-actions :items="itemsSelected" />
<context-actions :items="selectedResources" />
</template>
<template #footer>
<div v-if="showMoreToggle && hasMore" class="oc-width-1-1 oc-text-center oc-mt">
Expand Down Expand Up @@ -171,7 +171,11 @@ export default defineComponent({
}
},
setup() {
const { fileListHeaderY } = useResourcesViewDefaults<Resource, any, any[]>()
const { fileListHeaderY, selectedResourcesIds, selectedResources } = useResourcesViewDefaults<
Resource,
any,
any[]
>()
const store = useStore()
const hasShareJail = useCapabilityShareJailEnabled()
Expand All @@ -193,7 +197,9 @@ export default defineComponent({
resourceTargetLocation,
resourceTargetParamMapping,
resourceTargetQueryMapping,
fileListHeaderY
fileListHeaderY,
selectedResources,
selectedResourcesIds
}
},
Expand All @@ -203,7 +209,6 @@ export default defineComponent({
}),
computed: {
...mapGetters('Files', ['selectedFiles']),
...mapGetters(['configuration']),
...mapState('Files/sidebar', { sidebarClosed: 'closed' }),
Expand All @@ -216,14 +221,6 @@ export default defineComponent({
countFolders() {
return this.items.filter((s) => s.type === 'folder').length
},
itemsSelected: {
get() {
return this.selectedFiles
},
set(resources) {
this.SET_FILE_SELECTION(resources.filter((r) => r.status === this.shareStatus))
}
},
toggleMoreLabel() {
return this.showMore ? this.$gettext('Show less') : this.$gettext('Show more')
},
Expand All @@ -242,7 +239,7 @@ export default defineComponent({
},
methods: {
...mapActions('Files', ['loadIndicators', 'loadPreview', 'loadAvatars']),
...mapMutations('Files', ['LOAD_FILES', 'SET_FILE_SELECTION', 'CLEAR_CURRENT_FILES_LIST']),
...mapMutations('Files', ['LOAD_FILES', 'CLEAR_CURRENT_FILES_LIST']),
rowMounted(resource, component) {
const debounced = debounce(({ unobserve }) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/web-app-files/src/components/TrashBin.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<resource-table
v-else
id="files-trashbin-table"
v-model="selectedResources"
v-model="selectedResourcesIds"
class="files-table"
:class="{ 'files-table-squashed': !sidebarClosed }"
:fields-displayed="['name', 'ddate']"
Expand Down Expand Up @@ -97,7 +97,7 @@ export default defineComponent({
computed: {
...mapState('Files', ['files']),
...mapGetters('Files', ['highlightedFile', 'selectedFiles', 'totalFilesCount']),
...mapGetters('Files', ['highlightedFile', 'totalFilesCount']),
...mapState('Files/sidebar', { sidebarClosed: 'closed' }),
isEmpty() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ interface ResourcesViewDefaultsResult<T, TT, TU extends any[]> {
sortDir: ComputedRef<SortDir>

selectedResources: WritableComputedRef<Resource[]>
selectedResourcesIds: WritableComputedRef<(string | number)[]>
isResourceInSelection(resource: Resource): boolean
}

Expand Down Expand Up @@ -79,9 +80,17 @@ export const useResourcesViewDefaults = <T, TT, TU extends any[]>(
store.commit('Files/SET_FILE_SELECTION', resources)
}
})
const selectedResourcesIds: WritableComputedRef<(string | number)[]> = computed({
get(): (string | number)[] {
return store.state.Files.selectedIds
},
set(selectedIds) {
store.commit('Files/SET_SELECTED_IDS', selectedIds)
}
})

const isResourceInSelection = (resource: Resource) => {
return unref(selectedResources).includes(resource)
return unref(selectedResourcesIds).includes(resource.id)
}

return {
Expand All @@ -98,6 +107,7 @@ export const useResourcesViewDefaults = <T, TT, TU extends any[]>(
sortBy,
sortDir,
selectedResources,
selectedResourcesIds,
isResourceInSelection
}
}
18 changes: 13 additions & 5 deletions packages/web-app-files/src/store/mutations.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,19 @@ export default {
SET_LATEST_SELECTED_FILE_ID(state, fileId) {
state.latestSelectedId = fileId
},
SET_FILE_SELECTION(state, files) {
const latestSelected = files.find((i) => !state.selectedIds.some((j) => j === i.id))
const latestSelectedId = latestSelected ? latestSelected.id : state.latestSelectedId
state.latestSelectedId = latestSelectedId
state.selectedIds = files.map((f) => f.id)
SET_FILE_SELECTION(state, selectedFiles) {
const latestSelected = selectedFiles.find((i) => !state.selectedIds.includes(i.id))
if (latestSelected) {
state.latestSelectedId = latestSelected.id
}
state.selectedIds = selectedFiles.map((f) => f.id)
},
SET_SELECTED_IDS(state, selectedIds) {
const latestSelectedId = selectedIds.find((id) => !state.selectedIds.includes(id))
if (latestSelectedId) {
state.latestSelectedId = latestSelectedId
}
state.selectedIds = selectedIds
},
ADD_FILE_SELECTION(state, file) {
const selected = [...state.selectedIds]
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/views/Favorites.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<resource-table
v-else
id="files-favorites-table"
v-model="selectedResources"
v-model="selectedResourcesIds"
class="files-table"
:class="{ 'files-table-squashed': !sidebarClosed }"
:are-paths-displayed="true"
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/views/Personal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<resource-table
v-else
id="files-personal-table"
v-model="selectedResources"
v-model="selectedResourcesIds"
class="files-table"
:class="{ 'files-table-squashed': !sidebarClosed }"
:are-thumbnails-displayed="displayThumbnails"
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/views/PublicFiles.vue
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<resource-table
v-else
id="files-public-files-table"
v-model="selectedResources"
v-model="selectedResourcesIds"
class="files-table"
:class="{ 'files-table-squashed': !sidebarClosed }"
:are-thumbnails-displayed="displayThumbnails"
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/views/shares/SharedResource.vue
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<resource-table
v-else
id="files-shared-resource-table"
v-model="selectedResources"
v-model="selectedResourcesIds"
class="files-table"
:class="{ 'files-table-squashed': !sidebarClosed }"
:are-thumbnails-displayed="displayThumbnails"
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/views/shares/SharedViaLink.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<resource-table
v-else
id="files-shared-via-link-table"
v-model="selectedResources"
v-model="selectedResourcesIds"
class="files-table"
:class="{ 'files-table-squashed': !sidebarClosed }"
:fields-displayed="['name', 'sharedWith', 'sdate']"
Expand Down
4 changes: 1 addition & 3 deletions packages/web-app-files/src/views/shares/SharedWithMe.vue
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
</template>

<script lang="ts">
import { mapGetters, mapState } from 'vuex'
import { mapGetters } from 'vuex'
import { useSort, useResourcesViewDefaults } from '../../composables'
import AppLoadingSpinner from 'web-pkg/src/components/AppLoadingSpinner.vue'
Expand Down Expand Up @@ -148,9 +148,7 @@ export default defineComponent({
}),
computed: {
...mapGetters('Files', ['selectedFiles']),
...mapGetters(['configuration']),
...mapState('Files/sidebar', { sidebarClosed: 'closed' }),
pendingTitle() {
return this.$gettext('Pending shares')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<resource-table
v-else
id="files-shared-with-others-table"
v-model="selectedResources"
v-model="selectedResourcesIds"
class="files-table"
:class="{ 'files-table-squashed': !sidebarClosed }"
:fields-displayed="['name', 'sharedWith', 'sdate']"
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/views/spaces/Project.vue
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
<resource-table
v-else
id="files-spaces-table"
v-model="selectedResources"
v-model="selectedResourcesIds"
class="files-table oc-mt-xl"
:resources="paginatedResources"
:target-route="resourceTargetLocation"
Expand Down

0 comments on commit b53f23d

Please sign in to comment.