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

WIP PDF Viewer #6575

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"packages/web-app-files",
"packages/web-app-markdown-editor",
"packages/web-app-media-viewer",
"packages/web-app-pdf-viewer",
"packages/web-app-search",
"packages/web-client",
"packages/web-pkg",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ export default {
menuItemsContext() {
const fileHandlers = [
...this.$_fileActions_editorActions,
...this.$_fileActions_loadExternalAppActions(this.filterParams.resources),
...this.$_fetch_items
...this.$_fileActions_loadExternalAppActions(this.filterParams.resources)
]

return [...fileHandlers].filter((item) => item.isEnabled(this.filterParams))
Expand Down
71 changes: 0 additions & 71 deletions packages/web-app-files/src/mixins/actions/fetch.js

This file was deleted.

3 changes: 0 additions & 3 deletions packages/web-app-files/src/mixins/fileActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@ import Delete from './actions/delete'
import DownloadArchive from './actions/downloadArchive'
import DownloadFile from './actions/downloadFile'
import Favorite from './actions/favorite'
import Fetch from './actions/fetch'
import Move from './actions/move'
import Navigate from './actions/navigate'
import Rename from './actions/rename'
import Restore from './actions/restore'
import kebabCase from 'lodash-es/kebabCase'

const actionsMixins = [
'fetch',
'navigate',
'downloadArchive',
'downloadFile',
Expand All @@ -44,7 +42,6 @@ export default {
DownloadFile,
DownloadArchive,
Favorite,
Fetch,
Move,
Navigate,
Rename,
Expand Down
10 changes: 10 additions & 0 deletions packages/web-app-pdf-viewer/l10n/.tx/config
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[main]
host = https://www.transifex.com

[owncloud-web.pdf-viewer]
file_filter = locale/<lang>/LC_MESSAGES/app.po
minimum_perc = 0
source_file = template.pot
source_lang = en
type = PO

1 change: 1 addition & 0 deletions packages/web-app-pdf-viewer/l10n/translations.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
6 changes: 6 additions & 0 deletions packages/web-app-pdf-viewer/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "pdf-viewer",
"version": "0.0.0",
"description": "ownCloud web PDF viewer integration",
"license": "AGPL-3.0"
}
93 changes: 93 additions & 0 deletions packages/web-app-pdf-viewer/src/App.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<template>
<main>
<loading-screen v-if="loading" />
<error-screen v-else-if="loadingError" />
<object
v-else
class="pdf-viewer oc-width-1-1 oc-height-1-1"
:data="blobUrl"
type="application/pdf"
/>
</main>
</template>
<script>
import { mapGetters } from 'vuex'
import { useAppDefaults } from 'web-pkg/src/composables'
import { DavProperties } from 'web-pkg/src/constants'
import { buildResource } from '../../web-app-files/src/helpers/resources'
import HTTPError from 'owncloud-sdk'
import ErrorScreen from './components/ErrorScreen.vue'
import LoadingScreen from './components/LoadingScreen.vue'

export default {
name: 'PDFViewer',
components: {
ErrorScreen,
LoadingScreen
},
setup() {
return {
...useAppDefaults({
applicationName: 'pdf-viewer'
})
}
},
data: () => ({
loading: true,
loadingError: false,
filePath: '',
blobUrl: ''
}),
computed: {
...mapGetters(['getToken'])
},
async created() {
this.filePath = this.currentFileContext.path

try {
let resource = await this.getFileInfo(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be simplified by using getFileContents which you made available via useAppDefaults, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that getFileContents returns text instead of blob.

Copy link
Member

Choose a reason for hiding this comment

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

Then let's add handling of blob as resolveType here

https://github.com/owncloud/owncloud-sdk/blob/b5df7ff922bfc9c1d70763267335df982331cbd4/src/helperFunctions.js#L343

This kind of logic should never be handled in an app imho

Copy link
Member

@dschmidt dschmidt Mar 18, 2022

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@diocas Would you like me to take this over and adjust to that? (also some routing changes in my pipeline once again)

Copy link
Contributor

Choose a reason for hiding this comment

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

Available by bumping the owncloud-sdk to v3.0.0-alpha.3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pascalwengerter feel free to take it, thanks (just add the cern tag to the new PR please)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the feedback @diocas, it'll be @dschmidt who'll take over from here I think 🐎

this.filePath,
this.isPublicLinkContext ? DavProperties.PublicLink : DavProperties.Default
)
resource = buildResource(resource)

let url
const headers = new Headers()
if (this.isPublicLinkContext) {
url = resource.downloadURL
} else {
url = `${this.$client.helpers._davPath}${resource.webDavPath}`
headers.append('Authorization', 'Bearer ' + this.getToken)
headers.append('X-Requested-With', 'XMLHttpRequest')
}

const response = await fetch(url, {
method: 'GET',
headers
})

if (response.status !== 200) {
throw new HTTPError(response.status, response.message)
}

const newBlob = new Blob([await response.blob()], { type: 'application/pdf' })
this.blobUrl = URL.createObjectURL(newBlob)
this.loading = false
} catch (e) {
this.loading = false
this.loadingError = true
console.error('Error fetching pdf', e.statusCode, e)
}

// TODO free memory when leaving
}
}
</script>
<style scoped>
.pdf-viewer {
border: none;
margin: 0;
padding: 0;
overflow: hidden;
}
</style>
11 changes: 11 additions & 0 deletions packages/web-app-pdf-viewer/src/components/ErrorScreen.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<template>
<div class="oc-position-center oc-text-center">
<oc-icon size="xxlarge" name="error-warning" fill-type="line" />
</div>
</template>

<script>
export default {
name: 'ErrorScreen'
}
</script>
12 changes: 12 additions & 0 deletions packages/web-app-pdf-viewer/src/components/LoadingScreen.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<template>
<div class="oc-position-center oc-text-center">
<oc-spinner size="xlarge" />
<p v-translate class="oc-invisible">Loading PDF document</p>
</div>
</template>

<script>
export default {
name: 'LoadingScreen'
}
</script>
34 changes: 34 additions & 0 deletions packages/web-app-pdf-viewer/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import translations from '../l10n/translations'
import App from './App.vue'

const routes = [
{
name: 'pdf-viewer',
path: '/:contextRouteName/:filePath*',
component: App,
meta: {
auth: false,
patchCleanPath: true
}
}
]

const appInfo = {
name: 'PDF viewer',
id: 'pdf-viewer',
icon: 'eye',
extensions: [
{
extension: 'pdf',
newTab: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it's not needed anymore to open in a new tab whenever the top bar is visible (which is always the case now). Also, the closeApp helper offers a way back to the source file in the files app with just one line of code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we will agree with this. We also consider opening in a new tab to be more consistent with other apps. Can I make it configurable? (the last time I looked at it, I think it would require some changes to pass the config here, but this would be an interesting thing to do for other apps as well).

routeName: 'pdf-viewer',
canBeDefault: true
}
]
}

export default {
appInfo,
routes,
translations
}
6 changes: 6 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9815,6 +9815,12 @@ __metadata:
languageName: node
linkType: hard

"pdf-viewer@workspace:packages/web-app-pdf-viewer":
version: 0.0.0-use.local
resolution: "pdf-viewer@workspace:packages/web-app-pdf-viewer"
languageName: unknown
linkType: soft

"pend@npm:~1.2.0":
version: 1.2.0
resolution: "pend@npm:1.2.0"
Expand Down