Skip to content

Commit

Permalink
Fix sidebar opening for different dav root and non-dav files
Browse files Browse the repository at this point in the history
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
  • Loading branch information
skjnldsv committed Oct 24, 2022
1 parent a7fd5d9 commit f95f066
Show file tree
Hide file tree
Showing 12 changed files with 79 additions and 35 deletions.
20 changes: 13 additions & 7 deletions cypress/e2e/non-dav-files.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ describe('Open non-dav files in viewer', function() {

it('Open login background', function() {
const fileInfo = {
filename: '/core/img/app-background.png',
basename: 'logo.png',
mime: 'image/png',
source: '/core/img/app-background.png',
filename: '/core/img/app-background.jpg',
basename: 'app-background.jpg',
mime: 'image/jpeg',
source: '/core/img/app-background.jpg',
etag: 'abc',
hasPreview: false,
fileid: 123,
Expand All @@ -63,14 +63,20 @@ describe('Open non-dav files in viewer', function() {
.and('not.have.class', 'icon-loading')
})

it('See the menu icon and title on the viewer header', function() {
cy.get('body > .viewer .modal-title').should('contain', 'logo.png')
cy.get('body > .viewer .modal-header button.action-item__menutoggle').should('be.visible')
it('See the title and close button on the viewer header', function() {
cy.get('body > .viewer .modal-title').should('contain', 'app-background.jpg')
cy.get('body > .viewer .modal-header button.header-close').should('be.visible')
})

it('Does not see navigation arrows', function() {
cy.get('body > .viewer button.prev').should('not.be.visible')
cy.get('body > .viewer button.next').should('not.be.visible')
})

it('Does not see the menu or sidebar button', function() {
// Menu does not exist
cy.get('body > .viewer .modal-header button.action-item__menutoggle').should('not.exist')
cy.get('.action-button__icon.icon-menu-sidebar').should('not.exist')
})

})
Binary file modified cypress/snapshots/base/visual-regression.cy.js/non-dav-base.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified cypress/snapshots/base/visual-regression.cy.js/video-base.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions js/viewer-main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/viewer-main.js.map

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
"@nextcloud/stylelint-config": "^2.2.0",
"@nextcloud/webpack-vue-config": "^5.3.0",
"babel-loader-exclude-node-modules-except": "^1.2.1",
"cypress": "^10.8.0",
"cypress": "^10.10.0",
"cypress-visual-regression": "^1.7.0",
"eslint-plugin-cypress": "^2.12.1",
"jest": "^28.1.3",
Expand Down
2 changes: 1 addition & 1 deletion src/components/Images.vue
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export default {
return this.src
}
// If there is no preview and we have a drirect source
// If there is no preview and we have a direct source
// load it instead
if (this.source && !this.hasPreview) {
return this.source
Expand Down
4 changes: 2 additions & 2 deletions src/mixins/Mime.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ export default {
// file path relative to user folder
hasPreview: {
type: Boolean,
required: true,
default: false,
},
// unique file id
fileid: {
type: [Number, String],
required: true,
required: false,
},
// list of all the visible files
fileList: {
Expand Down
38 changes: 30 additions & 8 deletions src/utils/davUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,42 @@
import { generateRemoteUrl } from '@nextcloud/router'
import { getCurrentUser } from '@nextcloud/auth'

const getRootPath = function() {
if (getCurrentUser()) {
return generateRemoteUrl(`dav/files/${getCurrentUser().uid}`)
/**
* Get the current dav root path
* e.g /remote.php/dav/files/USERID
* or /public.php/webdav for public shares
*/
export const getRootPath = function() {
if (!isPublic()) {
return generateRemoteUrl(`dav${getUserRoot()}`)
} else {
return generateRemoteUrl('webdav').replace('/remote.php', '/public.php')
}
}

const isPublic = function() {
return !getCurrentUser()
/**
* Get the user root path relative to
* the dav service endpoint
*/
export const getUserRoot = function() {
if (isPublic()) {
throw new Error('No user logged in')
}

return `/files/${getCurrentUser()?.uid}`
}

const getToken = function() {
return document.getElementById('sharingToken') && document.getElementById('sharingToken').value
/**
* Is the current user an unauthenticated user?
*/
export const isPublic = function() {
return !getCurrentUser()
}

export { getRootPath, getToken, isPublic }
/**
* Get the current share link token
*/
export const getToken = function() {
return document.getElementById('sharingToken')
&& document.getElementById('sharingToken').value
}
19 changes: 16 additions & 3 deletions src/utils/fileUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
*/
import { dirname } from '@nextcloud/paths'
import { generateUrl } from '@nextcloud/router'

import camelcase from 'camelcase'
import { getRootPath, getToken, isPublic } from './davUtils.js'

import { getRootPath, getToken, getUserRoot, isPublic } from './davUtils.js'
import { isNumber } from './numberUtil.js'

/**
Expand Down Expand Up @@ -129,15 +129,28 @@ const genFileInfo = function(obj) {
* @param {object} fileInfo The fileInfo
* @param {string} fileInfo.filename the file full path
* @param {string} fileInfo.basename the file name
* @param {string} fileInfo.source the file source if any
* @return {string}
*/
const getDavPath = function({ filename, basename }) {
const getDavPath = function({ filename, basename, source = '' }) {
const prefixUser = getUserRoot()

// TODO: allow proper dav access without the need of basic auth
// https://github.com/nextcloud/server/issues/19700
if (isPublic()) {
return generateUrl(`/s/${getToken()}/download?path={dirname}&files={basename}`,
{ dirname: dirname(filename), basename })
}

// If we have a source but we're not a dav resource, return null
if (source && !source.includes(prefixUser)) {
return null
}

// Workaround for files with different root like /remote.php/dav
if (filename.startsWith(prefixUser)) {
filename = filename.slice(prefixUser.length)
}
return getRootPath() + encodeFilePath(filename)
}

Expand Down
9 changes: 6 additions & 3 deletions src/views/Viewer.vue
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
{{ t('viewer', 'Edit') }}
</NcActionButton>
<!-- Menu items -->
<NcActionButton v-if="Sidebar && !isSidebarShown"
<NcActionButton v-if="Sidebar && sidebarOpenFilePath && !isSidebarShown"
:close-after-click="true"
icon="icon-menu-sidebar"
@click="showSidebar">
Expand Down Expand Up @@ -167,7 +167,7 @@ import isFullscreen from '@nextcloud/vue/dist/Mixins/isFullscreen.js'
import isMobile from '@nextcloud/vue/dist/Mixins/isMobile'
import { extractFilePaths, sortCompare } from '../utils/fileUtils.js'
import { getRootPath } from '../utils/davUtils.js'
import { getRootPath, getUserRoot } from '../utils/davUtils.js'
import canDownload from '../utils/canDownload.js'
import cancelableRequest from '../utils/CancelableRequest.js'
import Error from '../components/Error.vue'
Expand Down Expand Up @@ -290,6 +290,9 @@ export default {
sidebarFile() {
return this.Sidebar && this.Sidebar.file
},
sidebarOpenFilePath() {
return this.currentFile?.davPath?.split(getUserRoot())[1]
},
/**
* Is the current user allowed to delete the file?
Expand Down Expand Up @@ -906,7 +909,7 @@ export default {
// TODO: also hide figure, needs a proper method for it in server Sidebar
if (OCA?.Files?.Sidebar) {
await OCA.Files.Sidebar.open(this.currentFile.filename)
await OCA.Files.Sidebar.open(this.sidebarOpenFilePath)
}
},
Expand Down

0 comments on commit f95f066

Please sign in to comment.