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

Fix "no selection" panel for root nodes in all files and public links #6505

Merged
merged 2 commits into from
Mar 2, 2022
Merged
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
7 changes: 7 additions & 0 deletions changelog/unreleased/bugfix-right-sidebar-no-selection
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Bugfix: No selection info right sidebar

We fixed that the right sidebar was not showing the "no selection" info panel anymore in the root of "All files". In addition we also use the same "no selection" info panel now in the root nodes of public links.

https://github.com/owncloud/web/issues/6502
https://github.com/owncloud/web/issues/6182
https://github.com/owncloud/web/pull/6505
7 changes: 4 additions & 3 deletions changelog/unreleased/enhancement-update-ods
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
Enhancement: Update ODS to v12.2.0
Enhancement: Update ODS to v12.2.1

We updated the ownCloud Design System to version 12.2.0. Please refer to the full changelog in the ODS release (linked) for more details. Summary:
We updated the ownCloud Design System to version 12.2.1. Please refer to the full changelog in the ODS release (linked) for more details. Summary:

- Enhancement - Apply outstanding background color to oc-card: https://github.com/owncloud/owncloud-design-system/pull/1974
- Enhancement - Redesign OcBreadcrumb: https://github.com/owncloud/web/issues/6218
- Enhancement - Redesign files table related components: https://github.com/owncloud/owncloud-design-system/pull/1958

https://github.com/owncloud/web/pull/6450
https://github.com/owncloud/web/pull/6472
https://github.com/owncloud/owncloud-design-system/releases/tag/v12.2.0
https://github.com/owncloud/web/pull/6505
https://github.com/owncloud/owncloud-design-system/releases/tag/v12.2.1
34 changes: 27 additions & 7 deletions packages/web-app-files/src/components/SideBar/SideBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,11 @@ import { VisibilityObserver } from 'web-pkg/src/observer'
import { DavProperties } from 'web-pkg/src/constants'

import { buildResource } from '../../helpers/resources'
import { isLocationCommonActive, isLocationSharesActive } from '../../router'
import {
isLocationCommonActive,
isLocationPublicActive,
isLocationSharesActive
} from '../../router'
import { computed } from '@vue/composition-api'

import FileInfo from './FileInfo.vue'
Expand Down Expand Up @@ -120,7 +124,7 @@ export default {
},

computed: {
...mapGetters('Files', ['highlightedFile', 'selectedFiles', 'currentFolder']),
...mapGetters('Files', ['highlightedFile', 'selectedFiles', 'publicLinkPassword']),
...mapGetters(['fileSideBars', 'capabilities']),
...mapState('Files/sidebar', { sidebarActivePanel: 'activePanel' }),
activeAvailablePanelName() {
Expand Down Expand Up @@ -189,7 +193,13 @@ export default {
return this.selectedFiles && this.selectedFiles.length > 1
},
isRootFolder() {
return !this.highlightedFile?.path
const pathSegments = this.highlightedFile?.path?.split('/').filter(Boolean) || []
if (isLocationPublicActive(this.$router, 'files-public-files')) {
// root node of a public link has the public link token as path
// root path `/` like for personal home doesn't exist for public links
return pathSegments.length === 1
}
return !pathSegments.length
},
highlightedFileThumbnail() {
return this.highlightedFile?.thumbnail
Expand Down Expand Up @@ -304,12 +314,22 @@ export default {
this.selectedFile = this.highlightedFile
return
}

this.loading = true
try {
const item = await this.$client.files.fileInfo(
this.highlightedFile.webDavPath,
DavProperties.Default
)
let item
if (isLocationPublicActive(this.$router, 'files-public-files')) {
item = await this.$client.publicFiles.getFileInfo(
this.highlightedFile.webDavPath,
this.publicLinkPassword,
DavProperties.PublicLink
)
} else {
item = await this.$client.files.fileInfo(
this.highlightedFile.webDavPath,
DavProperties.Default
)
}

this.selectedFile = buildResource(item)
this.$set(this.selectedFile, 'thumbnail', this.highlightedFile.thumbnail || null)
Expand Down
184 changes: 140 additions & 44 deletions packages/web-app-files/tests/unit/components/SideBar/SideBar.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ import SideBar from '@files/src/components/SideBar/SideBar.vue'
import { createLocationSpaces } from '../../../../src/router'

jest.mock('web-pkg/src/observer')
jest.mock('@files/src/helpers/resources', () => {
const original = jest.requireActual('@files/src/helpers/resources')
return {
...original,
buildResource: jest.fn()
}
})

const simpleOwnFolder = {
type: 'folder',
Expand All @@ -21,63 +28,151 @@ const simpleOwnFolder = {
size: '740'
}

const selectors = {
noSelectionInfoPanel: 'noselection-stub'
}

describe('SideBar', () => {
it('fetches file info if 1 item is selected', () => {
const mockFileInfo = jest.fn()
mockFileInfo.mockReturnValueOnce(Files['/'][1])

createWrapper({
item: simpleOwnFolder,
selectedItems: [simpleOwnFolder],
mocks: { $client: { files: { fileInfo: mockFileInfo } } }
describe('file info', () => {
Copy link
Contributor Author

@kulmann kulmann Mar 2, 2022

Choose a reason for hiding this comment

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

wrapped the file info related test cases into its own describe block, so the following lines are mostly indentation changes. Additional tests start in line 96.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed to make some adjustments though, because I'm mocking buildResource now in this file.

beforeEach(() => {
buildResource.mockImplementation((item) => item)
})
afterEach(() => {
jest.clearAllMocks()
})
it('fetches file info if 1 item is selected', () => {
const mockFileInfo = jest.fn()
mockFileInfo.mockReturnValueOnce(Files['/'][1])

expect(mockFileInfo).toHaveBeenCalledTimes(1)
})

it('fetches file info if the selected item changes', async () => {
const spyOnFetchFileInfo = jest
.spyOn(SideBar.methods, 'fetchFileInfo')
.mockImplementation(jest.fn)
createWrapper({
item: simpleOwnFolder,
selectedItems: [simpleOwnFolder],
mocks: { $client: { files: { fileInfo: mockFileInfo } } }
})

const wrapper = createWrapper({
item: simpleOwnFolder,
selectedItems: [simpleOwnFolder]
expect(mockFileInfo).toHaveBeenCalledTimes(1)
})

// fetchFileInfo is called once in created()
expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(1)
it('fetches file info if the selected item changes', async () => {
const spyOnFetchFileInfo = jest
.spyOn(SideBar.methods, 'fetchFileInfo')
.mockImplementation(jest.fn)

const wrapper = createWrapper({
item: simpleOwnFolder,
selectedItems: [simpleOwnFolder]
})

// fetchFileInfo is called once in created()
expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(1)

// it should be called again when a different file is loaded
const resource = Files['/'][4]
resource.path = `${resource.name}`
wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', resource)
await wrapper.vm.$nextTick()
expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(2)

// and again if the file is renamed
const renamedResource = renameResource(Object.assign({}, resource), 'foobar.png', '')
wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', Object.assign(renamedResource))
await wrapper.vm.$nextTick()
expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(3)
})

// it should be called again when a different file is loaded
const resource = buildResource(Files['/'][4])
resource.path = `${resource.name}`
wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', resource)
await wrapper.vm.$nextTick()
expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(2)
it('does not fetch file info if multiple items are selected', () => {
const mockFileInfo = jest.fn().mockReturnValue({})

// and again if the file is renamed
const renamedResource = renameResource(Object.assign({}, resource), 'foobar.png', '')
wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', Object.assign(renamedResource))
await wrapper.vm.$nextTick()
expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(3)
createWrapper({
item: simpleOwnFolder,
selectedItems: [simpleOwnFolder, simpleOwnFolder],
mocks: { $client: { files: { fileInfo: mockFileInfo } } }
})

jest.resetAllMocks()
expect(mockFileInfo).not.toHaveBeenCalled()
})
})

it('does not fetch file info if multiple items are selected', () => {
const mockFileInfo = jest.fn()

createWrapper({
item: simpleOwnFolder,
selectedItems: [simpleOwnFolder, simpleOwnFolder],
mocks: { $client: { files: { fileInfo: mockFileInfo } } }
describe('no selection info panel', () => {
afterEach(() => {
jest.clearAllMocks()
})
describe('for public links', () => {
it.each([
[
'shows in root node',
{
path: '/publicLinkToken',
noSelectionExpected: true
}
],
[
'does not show in non-root node',
{
path: '/publicLinkToken/some-folder',
noSelectionExpected: false
}
]
])('%s', async (name, { path, noSelectionExpected }) => {
const item = { path }
const mockFileInfo = jest.fn()
mockFileInfo.mockReturnValue(item)
buildResource.mockReturnValue(item)
const wrapper = createWrapper({
item,
selectedItems: [],
mocks: {
$client: { publicFiles: { getFileInfo: mockFileInfo } }
},
currentRouteName: 'files-public-files'
})
await wrapper.vm.$nextTick()
await wrapper.vm.$nextTick()
expect(wrapper.find(selectors.noSelectionInfoPanel).exists()).toBe(noSelectionExpected)
})
})
describe('for all files', () => {
it.each([
[
'shows in root node',
{
path: '/',
noSelectionExpected: true
}
],
[
'does not show in non-root node',
{
path: '/some-folder',
noSelectionExpected: false
}
]
])('%s', async (name, { path, noSelectionExpected }) => {
const item = { path }
const mockFileInfo = jest.fn()
mockFileInfo.mockReturnValue(item)
buildResource.mockReturnValue(item)
const wrapper = createWrapper({
item,
selectedItems: [],
mocks: {
$client: { files: { fileInfo: mockFileInfo } }
}
})
await wrapper.vm.$nextTick()
await wrapper.vm.$nextTick()
expect(wrapper.find(selectors.noSelectionInfoPanel).exists()).toBe(noSelectionExpected)
})
})

expect(mockFileInfo).not.toHaveBeenCalled()
})
})

function createWrapper({ item, selectedItems, mocks }) {
function createWrapper({
item,
selectedItems,
mocks,
currentRouteName = 'files-spaces-personal-home'
}) {
const localVue = createLocalVue()
localVue.use(Vuex)
localVue.use(VueCompositionAPI)
Expand Down Expand Up @@ -111,7 +206,8 @@ function createWrapper({ item, selectedItems, mocks }) {
},
getters: {
highlightedFile: (state) => state.highlightedFile,
selectedFiles: () => selectedItems
selectedFiles: () => selectedItems,
publicLinkPassword: () => ''
},
mutations: {
SET_HIGHLIGHTED_FILE(state, file) {
Expand All @@ -137,7 +233,7 @@ function createWrapper({ item, selectedItems, mocks }) {
mocks: merge(
{
$router: {
currentRoute: createLocationSpaces('files-spaces-personal-home'),
currentRoute: createLocationSpaces(currentRouteName),
resolve: (r) => {
return { href: r.name }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ exports[`SharedWithOthers when the wrapper is not loading anymore when length of
<!---->
</th>
<th class="oc-th oc-table-cell oc-table-cell-align-left oc-table-cell-align-middle oc-table-cell-width-expand oc-text-nowrap oc-table-header-cell oc-table-header-cell-name" aria-sort="none" style="top: 0px;"><span class="oc-table-thead-content header-text">Name</span>
<oc-button-stub type="button" size="medium" arialabel="Sort by name" variation="passive" appearance="raw" justifycontent="center" gapsize="medium" class="oc-button-sort"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></oc-button-stub>
<oc-button-stub type="button" size="medium" arialabel="Sort by name" variation="passive" appearance="raw" justifycontent="center" gapsize="medium" class="oc-button-sort oc-invisible"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></oc-button-stub>
</th>
<th class="oc-th oc-table-cell oc-table-cell-align-right oc-table-cell-align-middle oc-table-cell-width-auto oc-text-nowrap oc-table-header-cell oc-table-header-cell-sharedWith" aria-sort="none" style="top: 0px;"><span class="oc-table-thead-content header-text">Shared with</span>
<oc-button-stub type="button" size="medium" arialabel="Sort by sharedWith" variation="passive" appearance="raw" justifycontent="center" gapsize="medium" class="oc-button-sort"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></oc-button-stub>
<oc-button-stub type="button" size="medium" arialabel="Sort by sharedWith" variation="passive" appearance="raw" justifycontent="center" gapsize="medium" class="oc-button-sort oc-invisible"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></oc-button-stub>
</th>
<th class="oc-th oc-table-cell oc-table-cell-align-right oc-table-cell-align-middle oc-table-cell-width-auto oc-text-nowrap oc-table-header-cell oc-table-header-cell-actions oc-pr-m" style="top: 0px;"><span class="oc-table-thead-content header-text">Actions</span>
<!---->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ exports[`Trashbin View when the view is not loading anymore when length of the p
<th class="oc-th oc-table-cell oc-table-cell-align-left oc-table-cell-align-middle oc-table-cell-width-shrink oc-text-nowrap oc-table-header-cell oc-table-header-cell-select oc-pl-m " style="top: 0px;"><span class="oc-table-thead-content"><div class="resource-table-select-all"><span><input id="resource-table-select-all" type="checkbox" name="checkbox" class="oc-checkbox oc-rounded oc-checkbox-l" value=""> <label for="resource-table-select-all" class="oc-invisible-sr oc-cursor-pointer">Select all resources</label></span></div></span>
<!---->
</th>
<th class="oc-th oc-table-cell oc-table-cell-align-left oc-table-cell-align-middle oc-table-cell-width-expand oc-text-nowrap oc-table-header-cell oc-table-header-cell-name" aria-sort="none" style="top: 0px;"><span class="oc-table-thead-content header-text">Name</span> <button aria-label="Sort by name" class="oc-button-sort oc-button oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-raw"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></button></th>
<th class="oc-th oc-table-cell oc-table-cell-align-left oc-table-cell-align-middle oc-table-cell-width-expand oc-text-nowrap oc-table-header-cell oc-table-header-cell-name" aria-sort="none" style="top: 0px;"><span class="oc-table-thead-content header-text">Name</span> <button aria-label="Sort by name" class="oc-button-sort oc-button oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-raw oc-invisible"><span class="oc-icon oc-icon-s oc-icon-passive"><!----></span></button></th>
<th class="oc-th oc-table-cell oc-table-cell-align-right oc-table-cell-align-middle oc-table-cell-width-auto oc-text-nowrap oc-table-header-cell oc-table-header-cell-status" style="top: 0px;"><span class="oc-table-thead-content header-text">Status</span>
<!---->
</th>
Expand Down
4 changes: 2 additions & 2 deletions packages/web-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"dependencies": {
"@fortawesome/fontawesome-free": "^5.15.4",
"@popperjs/core": "^2.4.0",
"@vue/composition-api": "^1.4.6",
"@vue/composition-api": "^1.4.9",
"axios": "^0.25.0",
"cross-fetch": "^3.0.6",
"easygettext": "^2.16.1",
Expand All @@ -17,7 +17,7 @@
"lodash-es": "^4.17.21",
"luxon": "^2.3.0",
"oidc-client": "1.11.5",
"owncloud-design-system": "12.2.0",
"owncloud-design-system": "^12.2.1",
"owncloud-sdk": "~2.0.0",
"p-queue": "^6.1.1",
"popper-max-size-modifier": "^0.2.0",
Expand Down
Loading