-
Notifications
You must be signed in to change notification settings - Fork 158
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
WIP PDF Viewer #6575
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{} |
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" | ||
} |
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( | ||
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> |
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> |
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> |
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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
} |
There was a problem hiding this comment.
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 viauseAppDefaults
, right?There was a problem hiding this comment.
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.There was a problem hiding this comment.
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
asresolveType
herehttps://github.com/owncloud/owncloud-sdk/blob/b5df7ff922bfc9c1d70763267335df982331cbd4/src/helperFunctions.js#L343
This kind of logic should never be handled in an app imho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
owncloud/owncloud-sdk#1028
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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 🐎