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 <louis@chmn.me>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
  • Loading branch information
artonge committed Feb 28, 2023
1 parent a2a2242 commit 4105b48
Show file tree
Hide file tree
Showing 23 changed files with 193 additions and 64 deletions.
4 changes: 2 additions & 2 deletions js/photos-main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/photos-main.js.map

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/photos-public.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/photos-public.js.map

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,25 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

/**
* @copyright Copyright (c) 2023 Louis Chemineau <louis@chmn.me>
*
* @author Louis Chemineau <louis@chmn.me>
*
* @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/>.
*
*/

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/photos-src_views_AlbumContent_vue.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/photos-src_views_AlbumContent_vue.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/photos-src_views_Timeline_vue.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/photos-src_views_Timeline_vue.js.map

Large diffs are not rendered by default.

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 <louis@chmn.me>
*
* @author Louis Chemineau <louis@chmn.me>
*
* @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 4105b48

Please sign in to comment.