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

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Aug 18, 2023 •
edited
Loading

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

@juliusknorr juliusknorr mentioned this pull request Aug 18, 2023
Merged
10 tasks
juliusknorr
juliusknorr 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

@juliusknorr juliusknorr requested a review from skjnldsv August 18, 2023 06:58
@juliusknorr
Copy link
Member Author

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

@juliusknorr
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

@juliusknorr juliusknorr 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 juliusknorr
Loading

Things that are still failing:

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

@juliusknorr
Copy link
Member Author

files don't open in shares

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

juliusknorr and others added 6 commits August 24, 2023 09:32
@juliusknorr
feat(f2v): Move to new file actions api
a4ce30a 
Signed-off-by: Julius Härtl <[email protected]>
@juliusknorr
tmp: add dev build for running text against viewer
72a74a7 
Signed-off-by: Julius Härtl <[email protected]>
@max-nextcloud @juliusknorr
fix(cypress): use updated file row selector
ecfc812 
Signed-off-by: Max <[email protected]>
@max-nextcloud @juliusknorr
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 <[email protected]>
@max-nextcloud @juliusknorr
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 <[email protected]>
@max-nextcloud @juliusknorr
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 <[email protected]>
@juliusknorr
Copy link
Member Author

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

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

skjnldsv commented Aug 24, 2023 •
edited
Loading

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

Didn't see your comment

@juliusknorr
Copy link
Member Author

juliusknorr commented Aug 24, 2023 •
edited
Loading

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
Loading

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 🤔

@juliusknorr
Copy link
Member Author

juliusknorr commented Aug 24, 2023 •
edited
Loading

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æ <[email protected]>
@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
@juliusknorr juliusknorr merged commit 5fe844e into master Aug 24, 2023
@juliusknorr juliusknorr 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.
3 participants
@juliusknorr @skjnldsv @max-nextcloud