Added file previews #3187

PVince81 · 2020-03-16T16:50:32Z

Description

Related Issue

Part of #276

Motivation and Context

How Has This Been Tested?

manual test, scroll wildly
preview in "all files list"
previews in public link files list
previews in shared file list
acceptance tests

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

Open tasks:

update ODS Added iconUrl to oc-file element owncloud-design-system#678
changelog entry
acceptance tests

update-docs · 2020-03-16T16:50:36Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

PVince81 · 2020-03-17T11:13:11Z

seems blob URL doesn't work for the icon component:

PVince81 · 2020-03-17T15:25:14Z

Previews do work when I scroll over the same element at least once.
I suspect that mounted() from oc-icon is not being called as "iconUrl" is undefined. In error cases it would be an empty string instead of undefined.
Likely related to the way how recycle scroller works.

PVince81 · 2020-03-17T17:00:53Z

requires ODS update to fix preview loading issue: Reload icon image when its url prop changed owncloud-design-system#681

PVince81 · 2020-03-18T16:53:55Z

added tests but discovered some new issues:

preview not loading directly after upload ?
scrolling down then back up quicky sometimes removes the already loaded previews: maybe the elements are re-created or something and we need to requery the URL ? (svg appears there for icon type instead of the expected img from the previous load, makes acceptance tests fail)

PVince81 · 2020-03-19T14:58:30Z

and now the lorem file is locked.
the failures are totally random... I'll try rebasing just in case...

PVince81 · 2020-03-19T14:58:54Z

rebased

PVince81 · 2020-03-19T17:04:13Z

Seems master is all green #3202

So there is something I changed here that is causing new side effects 😢

PVince81 · 2020-03-20T08:50:14Z

seems there's a locking issue.

waiting for previews to always load is not a good idea as we need to use a delay, it will slow down the tests.

idea: add a hidden loading indicator, maybe "data-loading" on the preview while the media source is running, so that the tests know that they need to wait longer.

PVince81 · 2020-03-20T08:56:16Z

or use waitForOutstandingAjaxCalls

PVince81 · 2020-03-20T09:59:32Z

Added waitForOutstandingAjaxCalls to attempt to wait for previews to be loaded by mediaSource() before continuing. This is to prevent running into situations where the file itself might be locked.

apps/files/src/components/FileItem.vue

LukasHirt · 2020-03-22T20:38:32Z

Pls post a screenshot into description

PVince81 · 2020-03-23T13:13:17Z

adjusted and rebased

PVince81 · 2020-03-23T14:58:00Z

while trying to debug the tests I found more issues which are fixed now:

path names with special characters work now (try loading preview for "strängé folder name (duplicate Support apps/plugins #2 &)/strängé filename (duplicate Support apps/plugins #2 &).txt")
added limit to concurrent downloads of previews

However there are still these issues:

for some strange reason, the preview loading doesn't kick in when entering folder in CI environment
due to concurrency, I suspect that the "wait for outstanding ajax calls" would think that the last ajax call was done when maybe next tick the next ajax call will start (in a p-queue env), so need to add a delay there.
preview detection still feels unreliable, maybe need to use a data attribute instead with a loading state there...

apps/files/src/components/FileItem.vue

PVince81 · 2020-03-25T10:40:23Z

Seems that waitForAllThumbnailsLoaded is not working:

The code is in the right place: https://github.com/owncloud/phoenix/blob/filelist-previews/tests/acceptance/stepDefinitions/filesContext.js#L114

I've now added an extra 1 second delay after waiting, just in case the file locks need more time to release themselves...

PVince81 · 2020-03-25T10:44:03Z

now debugging waitForAllThumbnailsLoaded, seems the selector is not working because the attribute is missing when "previewLoaded" is false ?!

PVince81 · 2020-03-25T11:02:53Z

okay, seems Vue.js doesn't render HTML attributes if their value is boolean false. Had to set it to the string "false".
also had to fix the selector for waitForAllThumbnailsLoaded to use xpath.

let's see...

PVince81 · 2020-03-25T12:02:03Z

I had an explicit check for a file entry in the file list with "upload overwrite" but doesn't cover all cases. We anyway don't need it any more due to the global waiting. So I've added "user browses to files page" before all the overwrite cases.

Squashed

ownclouders · 2020-03-25T12:48:34Z

💥 Acceptance tests webUIOCIS failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/9034/

Added new component FileItem wrapped around oc-file to better encapsulated the formatting methods as computed properties instead of using the mixin. Added preview support in file list loaded using mediaSource.

PVince81 · 2020-03-25T13:36:56Z

added a few more fixes.
the trashbin name was not rendered properly as trashbin must display the original path as prefix

ownclouders · 2020-03-25T14:24:54Z

💥 Acceptance tests webUIOCIS failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/9036/

When displaying the file list or scrolling down to formerly invisible resources, wait for ajax calls to make sure the mediaSource and preview loading call has ended before performing any operations. This is to avoid running into file lock issues while loading the previews.

PVince81 · 2020-03-25T14:29:40Z

almost all green, I've pushed one last fix.

the OCIS tests fail, especially the ones related to thumbnail. Not sure if the thumbnailer service is already in place ? if not, we should skip them. do you mind having a look @dpakach ?

ownclouders · 2020-03-25T15:20:08Z

💥 Acceptance tests webUIOCIS failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/9037/

dpakach · 2020-03-26T03:20:55Z

@PVince81, I think we can skip the tests on ocis until we properly implement thumbnail service in CI.

dpakach · 2020-03-26T03:24:44Z

tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature

@@ -69,6 +69,7 @@ Feature: Sharing files and folders with internal groups
    When the user renames file "lorem.txt" to "new-lorem.txt" using the webUI
    And the user shares file "new-lorem.txt" with group "grp1" as "Editor" using the webUI
    And the user re-logs in as "user1" using the webUI
+    And the user browses to the files page


can we call waitTillThumbnailsLoaded function in the user has logged in or user has re-logged in step.
That way we can avoid having user has browsed to the files page everywhere after logging in.

dpakach · 2020-03-26T09:41:33Z

The CI has passed but I restarted just to make sure it is stable

LukasHirt

Code looks good. If you're okay with tests @individual-it @dpakach feel free to merge 😉

C0rby · 2020-03-26T15:27:58Z

apps/files/src/components/FileItem.vue

+      if (this.item.etag) {
+        // add etag for URL based caching
+        // strip double quotes from etag
+        query.c = this.item.etag.substr(2, this.item.etag.length - 2)


@LukasHirt, oh no here is a bug..
the substr should be substr(1, this.item.etag.length - 2) or else it will cut of the first character. :/

Thanks for catching that! I'll open a PR with fix and ping you for review 😉

PVince81 self-assigned this Mar 16, 2020

PVince81 force-pushed the filelist-previews branch from 24dfe94 to 00d97cf Compare March 17, 2020 11:07

PVince81 force-pushed the filelist-previews branch from 2fbdb54 to 445ec41 Compare March 17, 2020 17:00

PVince81 force-pushed the filelist-previews branch from 445ec41 to ba359d1 Compare March 18, 2020 16:51

PVince81 force-pushed the filelist-previews branch from ba359d1 to 1c419d9 Compare March 18, 2020 21:23

PVince81 requested a review from LukasHirt March 18, 2020 21:24

PVince81 marked this pull request as ready for review March 18, 2020 21:24

PVince81 requested a review from DeepDiver1975 as a code owner March 18, 2020 21:24

PVince81 force-pushed the filelist-previews branch from 1c419d9 to e530fbd Compare March 19, 2020 14:58

PVince81 mentioned this pull request Mar 19, 2020

[NOMERGE] Test if CI also fails randomly from master #3202

Closed

LukasHirt reviewed Mar 22, 2020

View reviewed changes

apps/files/src/components/FileItem.vue Outdated Show resolved Hide resolved

LukasHirt reviewed Mar 22, 2020

View reviewed changes

apps/files/src/components/FileItem.vue Show resolved Hide resolved

PVince81 force-pushed the filelist-previews branch from 5d14937 to 05107ac Compare March 23, 2020 11:39

PVince81 force-pushed the filelist-previews branch 2 times, most recently from 4f950c4 to cc77caf Compare March 23, 2020 14:55

C0rby reviewed Mar 23, 2020

View reviewed changes

apps/files/src/components/FileItem.vue Outdated Show resolved Hide resolved

PVince81 force-pushed the filelist-previews branch from e905d33 to 09aeb74 Compare March 25, 2020 10:39

PVince81 force-pushed the filelist-previews branch from 09aeb74 to d38cfa2 Compare March 25, 2020 11:01

PVince81 force-pushed the filelist-previews branch from d38cfa2 to 7ebf071 Compare March 25, 2020 12:01

Added file preview in file list

e380514

Added new component FileItem wrapped around oc-file to better encapsulated the formatting methods as computed properties instead of using the mixin. Added preview support in file list loaded using mediaSource.

PVince81 force-pushed the filelist-previews branch from 7ebf071 to 59b1fd2 Compare March 25, 2020 13:36

Vincent Petry added 3 commits March 25, 2020 15:28

Add fetch API to ajax counters in tests code

fb8eef6

Still wait for ajax calls in case there's another one

6518128

PVince81 force-pushed the filelist-previews branch from 59b1fd2 to 6518128 Compare March 25, 2020 14:29

dpakach reviewed Mar 26, 2020

View reviewed changes

dpakach added 2 commits March 26, 2020 14:16

[Tests-only] Skip thumbnail tests in ocis

27f643f

[Tests-only] Wait for thumbnails to load after loggin in

8eeae76

LukasHirt approved these changes Mar 26, 2020

View reviewed changes

individual-it approved these changes Mar 26, 2020

View reviewed changes

individual-it merged commit 1a34c63 into master Mar 26, 2020

delete-merged-branch bot deleted the filelist-previews branch March 26, 2020 15:00

C0rby reviewed Mar 26, 2020

View reviewed changes

LukasHirt mentioned this pull request Mar 27, 2020

Do not remove first char in etag #3274

Merged

PVince81 mentioned this pull request Apr 14, 2020

Display previews #276

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added file previews #3187

Added file previews #3187

PVince81 commented Mar 16, 2020 •

edited

update-docs bot commented Mar 16, 2020

PVince81 commented Mar 17, 2020

PVince81 commented Mar 17, 2020

PVince81 commented Mar 17, 2020 •

edited

PVince81 commented Mar 18, 2020 •

edited

PVince81 commented Mar 19, 2020

PVince81 commented Mar 19, 2020

PVince81 commented Mar 19, 2020

PVince81 commented Mar 20, 2020

PVince81 commented Mar 20, 2020

PVince81 commented Mar 20, 2020

LukasHirt commented Mar 22, 2020

PVince81 commented Mar 23, 2020

PVince81 commented Mar 23, 2020

PVince81 commented Mar 25, 2020

PVince81 commented Mar 25, 2020

PVince81 commented Mar 25, 2020

PVince81 commented Mar 25, 2020

ownclouders commented Mar 25, 2020

PVince81 commented Mar 25, 2020

ownclouders commented Mar 25, 2020

PVince81 commented Mar 25, 2020

ownclouders commented Mar 25, 2020

dpakach commented Mar 26, 2020

dpakach Mar 26, 2020

dpakach commented Mar 26, 2020

LukasHirt left a comment

C0rby Mar 26, 2020

LukasHirt Mar 27, 2020

Added file previews #3187

Added file previews #3187

Conversation

PVince81 commented Mar 16, 2020 • edited

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Open tasks:

update-docs bot commented Mar 16, 2020

PVince81 commented Mar 17, 2020

PVince81 commented Mar 17, 2020

PVince81 commented Mar 17, 2020 • edited

PVince81 commented Mar 18, 2020 • edited

PVince81 commented Mar 19, 2020

PVince81 commented Mar 19, 2020

PVince81 commented Mar 19, 2020

PVince81 commented Mar 20, 2020

PVince81 commented Mar 20, 2020

PVince81 commented Mar 20, 2020

LukasHirt commented Mar 22, 2020

PVince81 commented Mar 23, 2020

PVince81 commented Mar 23, 2020

PVince81 commented Mar 25, 2020

PVince81 commented Mar 25, 2020

PVince81 commented Mar 25, 2020

PVince81 commented Mar 25, 2020

ownclouders commented Mar 25, 2020

PVince81 commented Mar 25, 2020

ownclouders commented Mar 25, 2020

PVince81 commented Mar 25, 2020

ownclouders commented Mar 25, 2020

dpakach commented Mar 26, 2020

dpakach Mar 26, 2020

Choose a reason for hiding this comment

dpakach commented Mar 26, 2020

LukasHirt left a comment

Choose a reason for hiding this comment

C0rby Mar 26, 2020

Choose a reason for hiding this comment

LukasHirt Mar 27, 2020

Choose a reason for hiding this comment

PVince81 commented Mar 16, 2020 •

edited

PVince81 commented Mar 17, 2020 •

edited

PVince81 commented Mar 18, 2020 •

edited