Skip to content

Commit

Permalink
[full-ci] More app top bar fixes (#7362)
Browse files Browse the repository at this point in the history
* Move Resource interface to web-client

* Add getFileResource helper to AppFileHandling composable

* web-app-external: use getFileResource helper

* Fix permissions type in Resource interface

* web-app-pdf-viewer: make AppTopBar always visible

* web-app-pdf-viewer: use getFileResource helper

* web-app-text-editor: make AppTopBar always visible

* web-app-text-editor: add error display

* web-app-text-editor: use getFileResource helper

* web-app-preview: simplify raw file loading

... by moving complexity to useAppFileHandling composable

* web-app-pdf-viewer: Use getUrlForResource

* web-app-preview: Use revokeUrl from file handling composable

* Add current PR to unreleased changelog item

* useAppFileHandling.getUrlForResource: add disposition option

which can be either inline or attachment and can be used to define the purpose of the retrieved url.
Chrome allows us to use signed/downloadURLs for video playback although they send a Content-Disposition header, so we can just use a signed url and don't need to create a blob url for this.
We cannot embed PDFs in the same way, as they trigger downloads. So we need to create a blob url for those.

If you need a download url, it's fine (or even desired) for the response
to contain the Content-Disposition header.
  • Loading branch information
dschmidt authored Jul 28, 2022
1 parent 7752b52 commit b1b16f8
Show file tree
Hide file tree
Showing 37 changed files with 214 additions and 163 deletions.
1 change: 1 addition & 0 deletions changelog/unreleased/enhancement-add-app-top-bar-component
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ We've added a app top bar component for consistency,
which will be used by the apps: preview, text-editor and pdf-viewer.

https://github.com/owncloud/web/pull/7217
https://github.com/owncloud/web/pull/7362
10 changes: 1 addition & 9 deletions packages/web-app-external/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@
import { mapGetters } from 'vuex'
import ErrorScreen from './components/ErrorScreen.vue'
import LoadingScreen from './components/LoadingScreen.vue'
import { DavProperties } from 'web-pkg/src/constants'
import { buildResource } from 'files/src/helpers/resources'
import { computed, unref } from '@vue/composition-api'
import { queryItemAsString, useAppDefaults, useRouteQuery } from 'web-pkg/src/composables'
import { defineComponent } from '@vue/runtime-core'
Expand Down Expand Up @@ -92,7 +90,7 @@ export default defineComponent({
this.loading = true
try {
const filePath = this.currentFileContext.path
const fileId = this.fileId || (await this.getFileInfoResource(filePath)).fileId
const fileId = this.fileId || (await this.getFileResource(filePath)).fileId
// fetch iframe params for app and file
const configUrl = this.configuration.server
Expand Down Expand Up @@ -135,12 +133,6 @@ export default defineComponent({
this.loading = false
this.loadingError = true
}
},
methods: {
async getFileInfoResource(path) {
const file = await this.getFileInfo(path, DavProperties.Default)
return buildResource(file)
}
}
})
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import SetSpaceImage from '../../mixins/spaces/actions/setImage'
import SetSpaceReadme from '../../mixins/spaces/actions/setReadme'
import SpaceNavigate from '../../mixins/spaces/actions/navigate'
import { PropType } from '@vue/composition-api'
import { Resource } from '../../helpers/resource'
import { Resource } from 'web-client'
export default {
name: 'ContextActions',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ import {
} from 'web-pkg/src/composables'
import Rename from '../../mixins/actions/rename'
import { defineComponent, PropType } from '@vue/composition-api'
import { extractDomSelector, Resource } from '../../helpers/resource'
import { extractDomSelector } from '../../helpers/resource'
import { Resource } from 'web-client'
import { ShareTypes } from '../../helpers/share'
import { createLocationSpaces } from '../../router'
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/components/Search/List.vue
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ import MixinFileActions from '../../mixins/fileActions'
import MixinFilesListFilter from '../../mixins/filesListFilter'
import MixinFilesListScrolling from '../../mixins/filesListScrolling'
import { searchLimit } from '../../search/sdk/list'
import { Resource } from '../../helpers/resource'
import { Resource } from 'web-client'
import { useStore } from 'web-pkg/src/composables'
const visibilityObserver = new VisibilityObserver()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ import MixinDeclineShare from '../../mixins/actions/declineShare'
import MixinFilesListFilter from '../../mixins/filesListFilter'
import MixinMountSideBar from '../../mixins/sidebar/mountSideBar'
import { useResourcesViewDefaults } from '../../composables'
import { Resource } from '../../helpers/resource'
import { Resource } from 'web-client'
import { useCapabilityShareJailEnabled, useStore } from 'web-pkg/src/composables'
import { createLocationSpaces } from '../../router'
import ListInfo from '../../components/FilesList/ListInfo.vue'
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/components/TrashBin.vue
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import ContextActions from './FilesList/ContextActions.vue'
import { useResourcesViewDefaults } from '../composables'
import { bus } from 'web-pkg/src/instance'
import { defineComponent } from '@vue/composition-api'
import { Resource } from '../helpers/resource'
import { Resource } from 'web-client'
export default defineComponent({
name: 'TrashBin',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { useSort, SortDir } from '../sort/'
import { useMutationSubscription, useRouteQuery, useStore } from 'web-pkg/src/composables'
import { determineSortFields } from '../../helpers/ui/resourceTable'
import { Task } from 'vue-concurrency'
import { Resource } from '../../helpers/resource'
import { Resource } from 'web-client'

interface ResourcesViewDefaultsOptions<T, U extends any[]> {
loadResourcesTask?: Task<T, U>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Route } from 'vue-router'
import { UppyResource } from 'web-runtime/src/composables/upload'
import { buildWebDavFilesPath, buildWebDavSpacesPath } from '../../helpers/resources'
import { User } from '../../helpers/user'
import { User, Graph } from 'web-client'
import {
useCapabilityShareJailEnabled,
useClientService,
Expand All @@ -15,7 +15,6 @@ import { SHARE_JAIL_ID } from '../../services/folder'
import * as uuid from 'uuid'
import path from 'path'
import { useGraphClient } from 'web-client/src/composables'
import { Graph } from 'web-client'

interface UploadHelpersResult {
inputFilesToUppyFiles(inputFileOptions): UppyResource[]
Expand Down
3 changes: 1 addition & 2 deletions packages/web-app-files/src/fileSideBars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import { isLocationSpacesActive, isLocationTrashActive, isLocationPublicActive }
import { spaceRoleEditor, spaceRoleManager } from './helpers/share'
import { Panel } from '../../web-pkg/src/components/sidebar'

import { Resource } from './helpers/resource'
import { User } from './helpers/user'
import { Resource, User } from 'web-client'
import Router from 'vue-router'

function $gettext(msg: string): string {
Expand Down
3 changes: 2 additions & 1 deletion packages/web-app-files/src/helpers/resource/copyMove.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Resource, extractNameWithoutExtension } from './index'
import { Resource } from 'web-client'
import { extractNameWithoutExtension } from './index'
import { join } from 'path'
import { buildResource } from '../resources'
import { DavProperties } from 'web-pkg/src/constants'
Expand Down
55 changes: 1 addition & 54 deletions packages/web-app-files/src/helpers/resource/resource.ts
Original file line number Diff line number Diff line change
@@ -1,59 +1,6 @@
import { User } from '../user'
import { Resource } from 'web-client'
import fileExtensions from '../extensions/fileExtensions'

// TODO: find a good location for the Resource interface. Needed in other repos as well, so it needs to be deployed to npm.
// TODO: add more fields to the resource interface. Extend into different resource types: FileResource, FolderResource, ShareResource, IncomingShareResource, OutgoingShareResource, ...
export interface Resource {
id: number | string
fileId?: string
storageId?: string
name?: string
path: string
webDavPath?: string
downloadURL?: string
type?: string
status?: number
spaceRoles?: any[]
mimeType?: string
isFolder?: boolean
sdate?: string
mdate?: string
indicators?: any[]
size?: number
permissions?: number
starred?: boolean
etag?: string
sharePermissions?: number
shareTypes?: number[]
privateLink?: string

canCreate?(): boolean
canUpload?(): boolean
canDownload?(): boolean
canShare?(): boolean
canRename?(): boolean
canBeDeleted?(): boolean
canBeRestored?(): boolean

isReceivedShare?(): boolean
isMounted?(): boolean

getDomSelector?(): string

resourceOwner?: User
owner?: User[]
ownerDisplayName?: string
ownerId?: string
sharedWith?: string
shareOwner?: string
shareOwnerDisplayname?: string

extension?: string
share?: any

ddate?: string
}

export const extractStorageId = (id?: string): string => {
if (!id || typeof id !== 'string') {
return ''
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Resource } from './resource'
import { Resource } from 'web-client'

export const isSameResource = (r1: Resource, r2: Resource): boolean => {
if (!r1 || !r2) return false
Expand Down
11 changes: 3 additions & 8 deletions packages/web-app-files/src/helpers/resources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,8 @@ import {
spaceRoleManager,
spaceRoleViewer
} from './share'
import {
extractDomSelector,
extractExtensionFromFile,
extractStorageId,
Resource
} from './resource'
import { User } from './user'
import { extractDomSelector, extractExtensionFromFile, extractStorageId } from './resource'
import { User, Resource } from 'web-client'
import { SHARE_JAIL_ID } from '../services/folder'

export function renameResource(resource, newName, newPath) {
Expand Down Expand Up @@ -71,7 +66,7 @@ export function buildResource(resource): Resource {
? resource.fileInfo[DavProperty.ContentSize]
: resource.fileInfo[DavProperty.ContentLength],
indicators: [],
permissions: resource.fileInfo[DavProperty.Permissions] || '',
permissions: (resource.fileInfo[DavProperty.Permissions] as string) || '',
starred: resource.fileInfo[DavProperty.IsFavorite] !== '0',
etag: resource.fileInfo[DavProperty.ETag],
sharePermissions: resource.fileInfo[DavProperty.SharePermissions],
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/helpers/share/share.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { User } from '../user'
import type { User } from 'web-client'
import type { ShareRole } from './role'
import type { SharePermission } from './permission'

Expand Down
1 change: 0 additions & 1 deletion packages/web-app-files/src/helpers/user/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export * from './avatarUrl'
export * from './types'
2 changes: 1 addition & 1 deletion packages/web-app-files/src/views/Favorites.vue
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ import ContextActions from '../components/FilesList/ContextActions.vue'
import { createLocationSpaces } from '../router'
import { useResourcesViewDefaults } from '../composables'
import { defineComponent } from '@vue/composition-api'
import { Resource } from '../helpers/resource'
import { Resource } from 'web-client'
import { useStore } from 'web-pkg/src/composables'
const visibilityObserver = new VisibilityObserver()
Expand Down
3 changes: 2 additions & 1 deletion packages/web-app-files/src/views/Personal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ import ContextActions from '../components/FilesList/ContextActions.vue'
import { createLocationSpaces } from '../router'
import { useResourcesViewDefaults } from '../composables'
import { defineComponent, unref, computed } from '@vue/composition-api'
import { Resource, move } from '../helpers/resource'
import { move } from '../helpers/resource'
import { Resource } from 'web-client'
import { useGraphClient } from 'web-client/src/composables'
import { useCapabilityShareJailEnabled, useRouteParam } from 'web-pkg/src/composables'
import KeyboardActions from '../components/FilesList/KeyboardActions.vue'
Expand Down
3 changes: 2 additions & 1 deletion packages/web-app-files/src/views/PublicFiles.vue
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ import Pagination from '../components/FilesList/Pagination.vue'
import ContextActions from '../components/FilesList/ContextActions.vue'
import { breadcrumbsFromPath, concatBreadcrumbs } from '../helpers/breadcrumbs'
import { defineComponent } from '@vue/composition-api'
import { Resource, move } from '../helpers/resource'
import { move } from '../helpers/resource'
import { Resource } from 'web-client'
import { usePublicLinkPassword, useStore } from 'web-pkg/src/composables'
import KeyboardActions from '../components/FilesList/KeyboardActions.vue'
Expand Down
3 changes: 2 additions & 1 deletion packages/web-app-files/src/views/shares/SharedResource.vue
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ import { createLocationSpaces } from '../../router'
import { useResourcesViewDefaults } from '../../composables'
import { defineComponent, unref } from '@vue/composition-api'
import { fetchResources } from '../../services/folder'
import { Resource, move } from '../../helpers/resource'
import { move } from '../../helpers/resource'
import { Resource } from 'web-client'
import { breadcrumbsFromPath, concatBreadcrumbs } from '../../helpers/breadcrumbs'
import { useRouteParam, useRouteQuery } from 'web-pkg/src/composables'
import KeyboardActions from '../../components/FilesList/KeyboardActions.vue'
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/views/shares/SharedViaLink.vue
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ import ContextActions from '../../components/FilesList/ContextActions.vue'
import { createLocationSpaces } from '../../router'
import { useResourcesViewDefaults } from '../../composables'
import { defineComponent } from '@vue/composition-api'
import { Resource } from '../../helpers/resource'
import { Resource } from 'web-client'
import { shareQuickLinkHelp } from '../../helpers/contextualHelpers'
import { useStore } from 'web-pkg/src/composables'
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-files/src/views/shares/SharedWithMe.vue
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import AppBar from '../../components/AppBar/AppBar.vue'
import SharedWithMeSection from '../../components/Shares/SharedWithMeSection.vue'
import { ShareStatus } from '../../helpers/share'
import { computed, defineComponent, unref } from '@vue/composition-api'
import { Resource } from '../../helpers/resource'
import { Resource } from 'web-client'
export default defineComponent({
components: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ import ContextActions from '../../components/FilesList/ContextActions.vue'
import { createLocationSpaces } from '../../router'
import { useResourcesViewDefaults } from '../../composables'
import { defineComponent } from '@vue/composition-api'
import { Resource } from '../../helpers/resource'
import { Resource } from 'web-client'
import { useStore } from 'web-pkg/src/composables'
const visibilityObserver = new VisibilityObserver()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ref, readonly } from '@vue/composition-api'
import { createWrapper } from './spec'
import { SortDir, SortOptions, useSort } from '../../../../src/composables'
import { Resource } from '../../../../src/helpers/resource'
import { Resource } from 'web-client/src/helpers/resource'

describe('useSort', () => {
it('should be valid', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {
extractDomSelector,
extractExtensionFromFile,
extractNameWithoutExtension,
Resource
extractNameWithoutExtension
} from '../../../../src/helpers/resource'
import { Resource } from 'web-client'

const resourceWithoutExtension = {
name: 'file'
Expand Down
26 changes: 13 additions & 13 deletions packages/web-app-pdf-viewer/src/App.vue
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
<template>
<main>
<app-top-bar :resource="resource" @close="closeApp" />
<loading-screen v-if="loading" />
<error-screen v-else-if="loadingError" />
<div v-else class="oc-height-1-1">
<app-top-bar :resource="resource" @close="closeApp" />
<object class="pdf-viewer oc-width-1-1" :data="blobUrl" type="application/pdf" />
<object class="pdf-viewer oc-width-1-1" :data="url" type="application/pdf" />
</div>
</main>
</template>
<script>
<script lang="ts">
import { useAppDefaults } from 'web-pkg/src/composables'
import AppTopBar from 'web-pkg/src/components/AppTopBar.vue'
import ErrorScreen from './components/ErrorScreen.vue'
import LoadingScreen from './components/LoadingScreen.vue'
import { buildResource } from 'files/src/helpers/resources'
import { defineComponent } from '@vue/runtime-core'
export default {
export default defineComponent({
name: 'PDFViewer',
components: {
ErrorScreen,
Expand All @@ -33,8 +33,8 @@ export default {
loading: true,
loadingError: false,
filePath: '',
blobUrl: '',
resource: {}
url: '',
resource: null
}),
created() {
this.loadPdf(this.currentFileContext)
Expand All @@ -46,10 +46,10 @@ export default {
async loadPdf(fileContext) {
try {
this.loading = true
const response = await this.getFileContents(fileContext.path, { responseType: 'blob' })
this.blobUrl = URL.createObjectURL(response.body)
const fileInfo = await this.getFileInfo(fileContext.path, {})
this.resource = buildResource(fileInfo)
this.resource = await this.getFileResource(fileContext.path)
this.url = await this.getUrlForResource(this.resource, {
disposition: 'inline'
})
} catch (e) {
this.loadingError = true
console.error('Error fetching pdf', e)
Expand All @@ -58,10 +58,10 @@ export default {
}
},
unloadPdf() {
URL.revokeObjectURL(this.blobUrl)
this.revokeUrl(this.url)
}
}
}
})
</script>
<style scoped>
.pdf-viewer {
Expand Down
Loading

0 comments on commit b1b16f8

Please sign in to comment.