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

refactor(files): Adjust useNavigation composable to enforce active view #49301

Merged
merged 2 commits into from
Nov 16, 2024
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: 4 additions & 3 deletions apps/files/src/components/FileEntry.vue
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
<!-- View columns -->
<td v-for="column in columns"
:key="column.id"
:class="`files-list__row-${currentView?.id}-${column.id}`"
:class="`files-list__row-${currentView.id}-${column.id}`"
class="files-list__row-column-custom"
:data-cy-files-list-row-column-custom="column.id"
@click="openDetailsIfAvailable">
Expand Down Expand Up @@ -135,7 +135,8 @@ export default defineComponent({
const filesStore = useFilesStore()
const renamingStore = useRenamingStore()
const selectionStore = useSelectionStore()
const { currentView } = useNavigation()
// The file list is guaranteed to be only shown with active view - thus we can set the `loaded` flag
const { currentView } = useNavigation(true)
const {
directory: currentDir,
fileId: currentFileId,
Expand Down Expand Up @@ -180,7 +181,7 @@ export default defineComponent({
if (this.filesListWidth < 512 || this.compact) {
return []
}
return this.currentView?.columns || []
return this.currentView.columns || []
},

size() {
Expand Down
10 changes: 5 additions & 5 deletions apps/files/src/components/FileEntry/FileEntryActions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@
</template>

<script lang="ts">
import type { PropType, ShallowRef } from 'vue'
import type { FileAction, Node, View } from '@nextcloud/files'
import type { PropType } from 'vue'
import type { FileAction, Node } from '@nextcloud/files'

import { DefaultType, NodeStatus } from '@nextcloud/files'
import { showError, showSuccess } from '@nextcloud/dialogs'
Expand Down Expand Up @@ -133,12 +133,12 @@ export default defineComponent({
},

setup() {
const { currentView } = useNavigation()
// The file list is guaranteed to be only shown with active view - thus we can set the `loaded` flag
const { currentView } = useNavigation(true)
const enabledFileActions = inject<FileAction[]>('enabledFileActions', [])

return {
// The file list is guaranteed to be only shown with active view
currentView: currentView as ShallowRef<View>,
currentView,
enabledFileActions,
}
},
Expand Down
9 changes: 5 additions & 4 deletions apps/files/src/components/FileEntry/FileEntryName.vue
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ export default defineComponent({
},

setup() {
const { currentView } = useNavigation()
// The file list is guaranteed to be only shown with active view - thus we can set the `loaded` flag
const { currentView } = useNavigation(true)
const { directory } = useRouteParameters()
const renamingStore = useRenamingStore()

Expand Down Expand Up @@ -143,7 +144,7 @@ export default defineComponent({
}
}

if (this.defaultFileAction && this.currentView) {
if (this.defaultFileAction) {
const displayName = this.defaultFileAction.displayName([this.source], this.currentView)
return {
is: 'button',
Expand Down Expand Up @@ -247,8 +248,8 @@ export default defineComponent({
if (status) {
showSuccess(t('files', 'Renamed "{oldName}" to "{newName}"', { oldName, newName }))
this.$nextTick(() => {
const nameContainter = this.$refs.basename as HTMLElement | undefined
nameContainter?.focus()
const nameContainer = this.$refs.basename as HTMLElement | undefined
nameContainer?.focus()
})
} else {
// Was cancelled - meaning the renaming state is just reset
Expand Down
3 changes: 2 additions & 1 deletion apps/files/src/components/FileEntryGrid.vue
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ export default defineComponent({
const filesStore = useFilesStore()
const renamingStore = useRenamingStore()
const selectionStore = useSelectionStore()
const { currentView } = useNavigation()
// The file list is guaranteed to be only shown with active view - thus we can set the `loaded` flag
const { currentView } = useNavigation(true)
const {
directory: currentDir,
fileId: currentFileId,
Expand Down
18 changes: 9 additions & 9 deletions apps/files/src/composables/useNavigation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import type { Navigation, View } from '@nextcloud/files'

import { beforeEach, describe, expect, it, vi } from 'vitest'
import { mount } from '@vue/test-utils'
import { defineComponent } from 'vue'

import { useNavigation } from './useNavigation'
import * as nextcloudFiles from '@nextcloud/files'

const { Navigation, View } = nextcloudFiles

// Just a wrapper so we can test the composable
const TestComponent = defineComponent({
template: '<div></div>',
Expand All @@ -29,7 +29,7 @@ describe('Composables: useNavigation', () => {

describe('currentView', () => {
beforeEach(() => {
navigation = new Navigation()
navigation = new nextcloudFiles.Navigation()
spy.mockImplementation(() => navigation)
})

Expand All @@ -39,7 +39,7 @@ describe('Composables: useNavigation', () => {
})

it('should return already active navigation', async () => {
const view = new View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
const view = new nextcloudFiles.View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
navigation.register(view)
navigation.setActive(view)
// Now the navigation is already set it should take the active navigation
Expand All @@ -48,7 +48,7 @@ describe('Composables: useNavigation', () => {
})

it('should be reactive on updating active navigation', async () => {
const view = new View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
const view = new nextcloudFiles.View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
navigation.register(view)
const wrapper = mount(TestComponent)

Expand All @@ -63,7 +63,7 @@ describe('Composables: useNavigation', () => {

describe('views', () => {
beforeEach(() => {
navigation = new Navigation()
navigation = new nextcloudFiles.Navigation()
spy.mockImplementation(() => navigation)
})

Expand All @@ -73,7 +73,7 @@ describe('Composables: useNavigation', () => {
})

it('should return already registered views', () => {
const view = new View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
const view = new nextcloudFiles.View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
// register before mount
navigation.register(view)
// now mount and check that the view is listed
Expand All @@ -82,8 +82,8 @@ describe('Composables: useNavigation', () => {
})

it('should be reactive on registering new views', () => {
const view = new View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
const view2 = new View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-2', name: 'My View 2', order: 1 })
const view = new nextcloudFiles.View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
const view2 = new nextcloudFiles.View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-2', name: 'My View 2', order: 1 })

// register before mount
navigation.register(view)
Expand Down
9 changes: 6 additions & 3 deletions apps/files/src/composables/useNavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,21 @@ import { onMounted, onUnmounted, shallowRef, triggerRef } from 'vue'

/**
* Composable to get the currently active files view from the files navigation
* @param _loaded If set enforce a current view is loaded
*/
export function useNavigation() {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export function useNavigation<T extends boolean>(_loaded?: T) {
type MaybeView = T extends true ? View : (View | null);
const navigation = getNavigation()
const views: ShallowRef<View[]> = shallowRef(navigation.views)
const currentView: ShallowRef<View | null> = shallowRef(navigation.active)
const currentView: ShallowRef<MaybeView> = shallowRef(navigation.active as MaybeView)

/**
* Event listener to update the `currentView`
* @param event The update event
*/
function onUpdateActive(event: CustomEvent<View|null>) {
currentView.value = event.detail
currentView.value = event.detail as MaybeView
}

/**
Expand Down
4 changes: 2 additions & 2 deletions dist/files-main.js

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files_sharing-init.js.map

Large diffs are not rendered by default.

Loading