Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[F2V] migrate files_trashbin to vue #36534

Merged
merged 32 commits into from Apr 6, 2023
Merged

[F2V] migrate files_trashbin to vue #36534

merged 32 commits into from Apr 6, 2023

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Feb 4, 2023

image

  • Base API
  • Fix and update side libraries
  • Implement store
    • Fix reactivity issue :(
  • Implement initial view
    • Virtual scrolling
    • Table mode
  • Selection handling
  • Very basic action (without API for now ?)
  • Accessibility
  • Testing

Needs:

Follow-up:

  • Improve loading indicator for actions
  • Deepen virtual scrolling performances
  • Implement keyboard arrow/pageUp-Down navigation and focus management
  • Hide some columns on small screens

@skjnldsv skjnldsv added this to the Nextcloud 27 milestone Feb 4, 2023
@skjnldsv skjnldsv added this to In progress in Files to vue via automation Feb 4, 2023
@skjnldsv skjnldsv self-assigned this Feb 4, 2023
@@ -118,7 +119,7 @@
$mimeType = $this->mimeTypeDetector->detectPath($file->getName());
}

$f = $this->previewManager->getPreview($file, $x, $y, true, IPreview::MODE_FILL, $mimeType);
$f = $this->previewManager->getPreview($file, $x, $y, $a, IPreview::MODE_FILL, $mimeType);

Check notice

Code scanning / Psalm

ArgumentTypeCoercion Note

Argument 1 of OCP\IPreview::getPreview expects OCP\Files\File, but parent type OCP\Files\Node provided
@skjnldsv skjnldsv force-pushed the feat/files2vue-trashbin branch 3 times, most recently from c640275 to ce90a9a Compare March 24, 2023 16:17
$this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'file_sorting_direction', $direction);
return new Response();

$userId = $this->userSession->getUser()->getUID();

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method getUID on possibly null value
@skjnldsv skjnldsv force-pushed the feat/files2vue-trashbin branch 3 times, most recently from cf4797b to ae3fd22 Compare March 28, 2023 15:37
@skjnldsv skjnldsv force-pushed the feat/files2vue-trashbin branch 2 times, most recently from 1b76d46 to 47d53c7 Compare April 5, 2023 06:42
@skjnldsv skjnldsv requested review from a team, artonge, Pytal and szaimen and removed request for a team April 5, 2023 07:17
@skjnldsv skjnldsv added 3. to review Waiting for reviews feature: files feature: trashbin and removed 2. developing Work in progress labels Apr 5, 2023
apps/comments/src/services/DavClient.js Show resolved Hide resolved
apps/comments/src/services/GetComments.ts Outdated Show resolved Hide resolved
apps/comments/src/views/Comments.vue Outdated Show resolved Hide resolved
apps/files/js/app.js Show resolved Hide resolved
apps/files/js/filelist.js Show resolved Hide resolved
apps/files/src/components/FilesListHeader.vue Outdated Show resolved Hide resolved
apps/files/src/components/FilesListHeader.vue Outdated Show resolved Hide resolved
apps/files/src/views/FilesList.vue Outdated Show resolved Hide resolved
apps/files/src/views/FilesList.vue Show resolved Hide resolved
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@juliushaertl
Copy link
Member

juliushaertl commented Apr 6, 2023

Looks very nice already :) Just some findings, could also be follow ups

  • Switching back to other file list does not work once entered the trash bin once
  • Previews show a black box
  • Restore/delete permanently a file does not remove it from the list
  • Grid view is gone
  • Breadcrumbs show Foldername.dTIMESTAMP when browsing a deleted folder, in the old implementation we just showed `Foldername
  • Sorting by size/name doesn't work yet

@skjnldsv
Copy link
Member Author

skjnldsv commented Apr 6, 2023

  • Previews show a black box

You mean when it's loading? Because I do see previews :)
image

  • Restore/delete permanently a file does not remove it from the list

It used to... 🤦
I'll investigate in followup

  • Breadcrumbs show Foldername.dTIMESTAMP when browsing a deleted folder, in the old implementation we just showed `Foldername

Yeah, I need to see how I can mitigate this without performance drawback :)

  • Sorting by size/name doesn't work yet

Works here! 🤔

Peek.06-04-2023.16-29.mp4

The rest of your points will be followups 😉

@szaimen
Copy link
Contributor

szaimen commented Apr 6, 2023

Additionally to Julius' findings, I also found something:

  • right-click does not work anymore
  • the mobile view is currently completely broken (but already in the top post as I can see)
  • clicking on a file in the trashbin downloads the file but with incorrect mimetype ending
  • there is at least one warning in the console log when I open the trashbin:
    image

Apart from that looks good to me!

@skjnldsv
Copy link
Member Author

skjnldsv commented Apr 6, 2023

@szaimen self signed certificate?
Service workers cannot be loaded on self signed, this is a browser issue.

clicking on a file in the trashbin downloads the file but with incorrect mimetype ending

Can you clarify what do oyou mean with the ending? The extention?
If so this is an issue with the dav backend 😉 (while we could mitigate with the front)
Will open a ticket afterwards 👍

@szaimen
Copy link
Contributor

szaimen commented Apr 6, 2023

ending? The extention?

yes I mean the extension.

Downloaded files look like this for example:
image

@szaimen
Copy link
Contributor

szaimen commented Apr 6, 2023

@szaimen self signed certificate?
Service workers cannot be loaded on self signed, this is a browser issue.

Ah I see. Then this is probably a no-issue.

@szaimen
Copy link
Contributor

szaimen commented Apr 6, 2023

BTW is there already some chunking implemented so that only a few files are loaded and then only when scrolling further down? (I have not tested this)...

@juliushaertl
Copy link
Member

  • Previews show a black box

You mean when it's loading? Because I do see previews :) image

Still doesn't after retesting. I have the following console trace, that looks related:

PreviewService.ts:30 Uncaught (in promise) ReferenceError: caches is not defined
    at isCachedPreview (PreviewService.ts:30:1)
    at _callee$ (FileEntry.vue?vue&type=script&lang=ts&:269:1)
    at tryCatch (core-common.js:223538:40)
    at Generator.invoke (core-common.js:223773:22)
    at Generator.next (core-common.js:223598:21)
    at asyncGeneratorStep (FileEntry.vue?vue&type=script&lang=ts&:2:1)
    at _next (FileEntry.vue?vue&type=script&lang=ts&:3:1)
    at FileEntry.vue?vue&type=script&lang=ts&:3:1
    at new Promise (<anonymous>)
    at FileEntry.vue?vue&type=script&lang=ts&:3:1
  • Sorting by size/name doesn't work yet

Works here! 🤔

Interesting, retested and works now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Files to vue
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants