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.

Sign up for GitHub

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

Jump to bottom

feat(f2v): Move to new file actions api #1878

Merged
merged 7 commits into from Aug 24, 2023
Merged

feat(f2v): Move to new file actions api #1878

merged 7 commits into from Aug 24, 2023

Conversation

juliushaertl
Copy link
Member

@juliushaertl juliushaertl commented Aug 18, 2023 •
edited

First draft for making viewer working again after nextcloud/server#39808 was merged

  • Follow up removing the fallback to the old actions for public share links once migrated to vue

@juliushaertl juliushaertl mentioned this pull request Aug 18, 2023
Merged
10 tasks
juliushaertl
juliushaertl commented Aug 18, 2023
View reviewed changes
src/views/Viewer.vue
@@ -270,7 +271,7 @@ export default {
isSidebarShown: false,
isFullscreenMode: false,
canSwipe: true,
isStandalone: !(OCA && OCA.Files && 'fileActions' in OCA.Files),
isStandalone: !(OCA && OCA.Files && 'fileActions' in OCA.Files), // FIXME this probably needs some adjustment
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left this comment intentionally, as i think we'll need to adjust this as soon as the old code bits are dropped and public share links are switched as well. We'll need a different approach then

@juliushaertl
Copy link
Member Author

Let's see what CI thinks about this, seems to do the trick so far

@juliushaertl
Copy link
Member Author

@skjnldsv Is there a tracking task for migrating the public share file list and is that on the plan for 28? Otherwise we'd still keep both approaches in here

@skjnldsv
Copy link
Member

@skjnldsv Is there a tracking task for migrating the public share file list and is that on the plan for 28? Otherwise we'd still keep both approaches in here

nextcloud/server#39914, added it on top

@juliushaertl juliushaertl mentioned this pull request Aug 21, 2023
Merged
4 tasks
@skjnldsv skjnldsv added 2. developing Work in progress technical debt Technical issue labels Aug 22, 2023
@max-nextcloud
Copy link
Contributor

max-nextcloud commented Aug 22, 2023 •
edited by juliushaertl

Things that are still failing:

There's probably more but i think these account for 90% of the failures.

@juliushaertl
Copy link
Member Author

files don't open in shares

Mainly because there it still uses the old file actions api #1878 (comment)

juliushaertl and others added 6 commits August 24, 2023 09:32
@juliushaertl
feat(f2v): Move to new file actions api
a4ce30a 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl
tmp: add dev build for running text against viewer
72a74a7 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@max-nextcloud @juliushaertl
fix(cypress): use updated file row selector
ecfc812 
Signed-off-by: Max <max@nextcloud.com>
@max-nextcloud @juliushaertl
fix: also register handler for aliases
69ece0d 
Add aliases to supported `_mimetypes` in `registerHandler`.

Otherwise files of the aliase mimetypes can not be opened in Viewer.

Signed-off-by: Max <max@nextcloud.com>
@max-nextcloud @juliushaertl
tmp: bring back legacy file handler for shares
3c0d482 
Public shares still use the old mechanism.
Lets keep it available until shares have also been migrated.

Signed-off-by: Max <max@nextcloud.com>
@max-nextcloud @juliushaertl
test(cypress): adjust selectors and share from details tab
3aa3bfc 
There is no direct link to the share tab right now.

Signed-off-by: Max <max@nextcloud.com>
@juliushaertl
Copy link
Member Author

Snapshots should be automatically updated after https://github.com/nextcloud/viewer/blob/master/.github/workflows/cypress-snapshot-update.yml

@juliushaertl juliushaertl added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 24, 2023
@juliushaertl juliushaertl added this to the Nextcloud 28 milestone Aug 24, 2023
@juliushaertl juliushaertl marked this pull request as ready for review August 24, 2023 07:36
@skjnldsv
Copy link
Member

skjnldsv commented Aug 24, 2023 •
edited

sharing/single-file-share.cy.js still fails :)

Didn't see your comment

@juliushaertl
Copy link
Member Author

juliushaertl commented Aug 24, 2023 •
edited

Hm, passed before and doesn't look viewer related:

Screenshot 2023-08-24 at 09 57 03

Looks like coming from the files app:

src/components/FileEntry.vue
573:				img.fetchpriority = this.visible ? 'high' : 'auto'

@skjnldsv
Copy link
Member

Can you explain why do we still need the registerLegacyAction ? :)

@skjnldsv
Copy link
Member

skjnldsv commented Aug 24, 2023 •
edited

Hm, passed before and doesn't look viewer related:

Screenshot 2023-08-24 at 09 57 03

Looks like coming from the files app:

src/components/FileEntry.vue
573:				img.fetchpriority = this.visible ? 'high' : 'auto'

I've seen this before. iirc it's when we logout too fast. There is an error thrown somewhere in the Files app. I need to catch it 🤔

@juliushaertl
Copy link
Member Author

juliushaertl commented Aug 24, 2023 •
edited

Can you explain why do we still need the registerLegacyAction ? :)

Only temporarily until the public pages are migrated to vue as well. Otherwise we would loose a lot of test coverage in text, collabora so having this intermediate step would save us from lots of effort on checking test failures or skipping those tests and not having those covered for now

@skjnldsv
Copy link
Member

Alright!

@skjnldsv

This comment was marked as resolved.

Sign in to view
@skjnldsv
chore: build assets
b26f569 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 24, 2023
@juliushaertl juliushaertl merged commit 5fe844e into master Aug 24, 2023
25 of 27 checks passed
@juliushaertl juliushaertl deleted the enh/f2v-fileactions branch August 24, 2023 08:39
@ChristophWurst ChristophWurst mentioned this pull request Nov 15, 2023
Closed
8 tasks
@Pytal Pytal mentioned this pull request Dec 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@max-nextcloud max-nextcloud Awaiting requested review from max-nextcloud

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish technical debt Technical issue
Projects
None yet
Milestone
Nextcloud 28
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@juliushaertl @skjnldsv @max-nextcloud