Skip to content

Commit

Permalink
Improve overall preview loading
Browse files Browse the repository at this point in the history
This PR improve on the logic to display previews:

1. Append the etag of the file to the file request. This ensure that old cache will not be used if the image is updated
2. Listen to 'files:file:updated' to refetch the file's info and have the new etag
3. Distinguish onload and on error events of the small and large previews to have a finer rendering conditions. Mostly not rendering both previews if the larger one is loaded.
4. Do not delay rendering of files to make the UI snappier

Signed-off-by: Louis Chemineau <[email protected]>
  • Loading branch information
artonge authored and backportbot-nextcloud[bot] committed Feb 27, 2023
1 parent d4fbd9f commit 7f9a3c8
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 37 deletions.
81 changes: 46 additions & 35 deletions src/components/File.vue
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,27 @@
<div class="file__images">
<VideoIcon v-if="file.mime.includes('video')" class="video-icon" :size="64" />

<img v-if="visibility !== 'none' && canLoad && !error"
<img v-if="visibility !== 'none' && canLoad && !errorNear && !loadedVisible"
ref="imgNear"
:key="`${file.basename}-near`"
:src="srcNear"
:alt="file.basename"
:aria-describedby="ariaDescription"
@load="onLoad"
@error="onError">
@load="onLoadNear"
@error="onErrorNear">

<img v-if="visibility === 'visible' && canLoad && !error"
<img v-if="(visibility === 'visible' || (loadedVisible && visibility === 'near')) && canLoad && !errorVisible"
ref="imgVisible"
:key="`${file.basename}-visible`"
:src="srcVisible"
:alt="file.basename"
:aria-describedby="ariaDescription"
@load="onLoad"
@error="onError">
@load="onLoadVisible"
@error="onErrorVisible">
</div>

<!-- image description -->
<p :id="ariaDescription" class="file__hidden-description" :class="{show: error}">{{ file.basename }}</p>
<p :id="ariaDescription" class="file__hidden-description" :class="{show: errorNear && errorVisible}">{{ file.basename }}</p>
</a>

<NcCheckboxRadioSwitch v-if="allowSelection"
Expand Down Expand Up @@ -111,8 +111,10 @@ export default {

data() {
return {
loaded: false,
error: false,
loadedNear: false,
loadedVisible: false,
errorNear: false,
errorVisible: false,
canLoad: false,
semaphoreSymbol: null,
isDestroyed: false,
Expand Down Expand Up @@ -146,25 +148,24 @@ export default {
},
},

mounted() {
// Don't render the component right away as it is useless if the user is only scrolling
setTimeout(async () => {
this.semaphoreSymbol = await this.semaphore.acquire(() => {
switch (this.visibility) {
case 'visible':
return 1
case 'near':
return 2
default:
return 3
}
}, this.file.fileid)

this.canLoad = true
if (this.visibility === 'none' || this.isDestroyed) {
this.releaseSemaphore()
async mounted() {
this.semaphoreSymbol = await this.semaphore.acquire(() => {
switch (this.visibility) {
case 'visible':
return 1
case 'near':
return 2
default:
return 3
}
}, 250)
}, this.file.fileid)

if (this.visibility === 'none' || this.isDestroyed) {
this.releaseSemaphore()
return
}

this.canLoad = true
},

beforeDestroy() {
Expand All @@ -185,14 +186,25 @@ export default {
this.$emit('click', this.file.fileid)
},

/** When the image is fully loaded by browser we remove the placeholder */
onLoad() {
this.loaded = true
/** When the 'near' image is fully loaded by browser we release semaphore */
onLoadNear() {
this.loadedNear = true
this.releaseSemaphore()
},

onError() {
this.error = true
/** When the 'visible' image is fully loaded by browser we release semaphore */
onLoadVisible() {
this.loadedVisible = true
this.releaseSemaphore()
},

onErrorNear() {
this.errorNear = true
this.releaseSemaphore()
},

onErrorVisible() {
this.errorVisible = true
this.releaseSemaphore()
},

Expand All @@ -203,11 +215,10 @@ export default {
getItemURL(size) {
const token = this.$route.params.token
if (token) {
return generateUrl(`/apps/photos/api/v1/publicPreview/${this.file.fileid}?x=${size}&y=${size}&token=${token}`)
return generateUrl(`/apps/photos/api/v1/publicPreview/${this.file.fileid}?etag=${this.decodedEtag}&x=${size}&y=${size}&token=${token}`)
} else {
return generateUrl(`/apps/photos/api/v1/preview/${this.file.fileid}?x=${size}&y=${size}`)
return generateUrl(`/apps/photos/api/v1/preview/${this.file.fileid}?etag=${this.decodedEtag}&x=${size}&y=${size}`)
}

},

releaseSemaphore() {
Expand Down
25 changes: 24 additions & 1 deletion src/components/FilesListViewer.vue
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,15 @@
</div>
</template>
<script>
import { mapGetters } from 'vuex'
import { mapActions, mapGetters } from 'vuex'

import PackageVariant from 'vue-material-design-icons/PackageVariant'

import { NcEmptyContent, NcLoadingIcon } from '@nextcloud/vue'
import { subscribe, unsubscribe } from '@nextcloud/event-bus'

import TiledLayout from '../components/TiledLayout/TiledLayout.vue'
import { fetchFile } from '../services/fileFetcher.js'
import VirtualScrolling from '../components/VirtualScrolling.vue'
import EmptyBox from '../assets/Illustrations/empty.svg'

Expand Down Expand Up @@ -222,7 +224,19 @@ export default {
},
},

mounted() {
subscribe('files:file:updated', this.handleFileUpdated)
},

destroyed() {
unsubscribe('files:file:updated', this.handleFileUpdated)
},

methods: {
...mapActions([
'appendFiles',
]),

// Ask the parent for more content.
needContent() {
this.$emit('need-content')
Expand All @@ -237,6 +251,15 @@ export default {
ratio: file.fileMetadataSizeParsed.width / file.fileMetadataSizeParsed.height,
}
},

/**
* @param {object} data
* @param {string} data.fileid - The file id of the updated file.
*/
async handleFileUpdated({ fileid }) {
const fetchedFile = await fetchFile(this.files[fileid].filename)
this.appendFiles([fetchedFile])
},
},
}
</script>
Expand Down
73 changes: 73 additions & 0 deletions src/services/fileFetcher.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/**
* @copyright Copyright (c) 2023 Louis Chemineau <[email protected]>
*
* @author Louis Chemineau <[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 { genFileInfo } from '../utils/fileUtils.js'
import defaultClient from './DavClient.js'

/**
* @param {string[]} extraProps - Extra properties to add to the DAV request.
* @return {string}
*/
function getCollectionFilesDavRequest(extraProps = []) {
return `<?xml version="1.0"?>
<d:propfind xmlns:d="DAV:"
xmlns:oc="http://owncloud.org/ns"
xmlns:nc="http://nextcloud.org/ns"
xmlns:ocs="http://open-collaboration-services.org/ns">
<d:prop>
<d:getcontentlength />
<d:getcontenttype />
<d:getetag />
<d:getlastmodified />
<d:resourcetype />
<nc:file-metadata-size />
<nc:has-preview />
<oc:favorite />
<oc:fileid />
<oc:permissions />
${extraProps.join('')}
</d:prop>
</d:propfind>`
}

/**
* @param {string} fileName - The full file's name
* @param {import('webdav').StatOptions} options - Options to forward to the webdav client.
* @return {Promise<object>}
*/
export async function fetchFile(fileName, options = {}) {
try {
const response = await defaultClient.stat(fileName, {
data: getCollectionFilesDavRequest(),
details: true,
...options,
})

return genFileInfo(response.data)
} catch (error) {
if (error.code === 'ERR_CANCELED') {
return null
}

throw error
}
}
2 changes: 1 addition & 1 deletion src/utils/fileUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function genFileInfo(obj) {

if (fileInfo.filename) {
// Adding context
fileInfo.source = generateRemoteUrl(rootPath) + '/' + encodeFilePath(fileInfo.filename)
fileInfo.source = generateRemoteUrl(rootPath) + encodeFilePath(fileInfo.filename)
}

return fileInfo
Expand Down

0 comments on commit 7f9a3c8

Please sign in to comment.