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

Disable resource click and default editor action if embed mode is enabled #10786

Merged
merged 9 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from 8 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Change: Disable opening files in embed mode

We have disabled to open files in the embed mode, since opening or editing files is not in scope of the embed mode

https://github.com/owncloud/web/pull/10786
https://github.com/owncloud/web/issues/10635
7 changes: 6 additions & 1 deletion packages/web-pkg/src/components/FilesList/ResourceTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ export default defineComponent({
setup(props, context) {
const capabilityStore = useCapabilityStore()
const { getMatchingSpace } = useGetMatchingSpace()
const { isLocationPicker } = useEmbedMode()
const { isLocationPicker, isEnabled: isEmbedModeEnabled } = useEmbedMode()

const configStore = useConfigStore()
const { options: configOptions } = storeToRefs(configStore)
Expand Down Expand Up @@ -562,6 +562,7 @@ export default defineComponent({
targetRouteCallback: computed(() => props.targetRouteCallback)
}),
isLocationPicker,
isEmbedModeEnabled,
toggleSelection,
areFileExtensionsShown,
latestSelectedId
Expand Down Expand Up @@ -1010,6 +1011,10 @@ export default defineComponent({
return false
}

if (this.isEmbedModeEnabled && !resource.isFolder) {
return false
}

// TODO: remove as soon as unsynced shares are accessible
if (isIncomingShareResource(resource) && !resource.syncEnabled) {
return false
Expand Down
7 changes: 7 additions & 0 deletions packages/web-pkg/src/components/FilesList/ResourceTile.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
class="oc-card-media-top oc-flex oc-flex-center oc-flex-middle oc-m-rm"
:resource="resource"
:folder-link="resourceRoute"
:is-resource-clickable="isResourceClickable"
tabindex="-1"
@click="$emit('click')"
>
Expand Down Expand Up @@ -58,6 +59,7 @@
:resource="resource"
:is-icon-displayed="false"
:is-extension-displayed="isExtensionDisplayed"
:is-resource-clickable="isResourceClickable"
:folder-link="resourceRoute"
@click="$emit('click')"
/>
Expand Down Expand Up @@ -108,6 +110,11 @@ export default defineComponent({
required: false,
default: false
},
isResourceClickable: {
type: Boolean,
required: false,
default: true
},
isExtensionDisplayed: {
type: Boolean,
default: true
Expand Down
10 changes: 9 additions & 1 deletion packages/web-pkg/src/components/FilesList/ResourceTiles.vue
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
:resource="resource"
:resource-route="getRoute(resource)"
:is-resource-selected="isResourceSelected(resource)"
:is-resource-clickable="isResourceClickable(resource)"
:is-extension-displayed="areFileExtensionsShown"
:resource-icon-size="resourceIconSize"
:draggable="dragDrop"
Expand Down Expand Up @@ -145,7 +146,8 @@ import {
useResourceRouteResolver,
useTileSize,
useResourcesStore,
useViewSizeMax
useViewSizeMax,
useEmbedMode
} from '../../composables'

export default defineComponent({
Expand Down Expand Up @@ -208,6 +210,7 @@ export default defineComponent({
const { $gettext } = useGettext()
const resourcesStore = useResourcesStore()
const { emit } = context
const { isEnabled: isEmbedModeEnabled } = useEmbedMode()
const viewSizeMax = useViewSizeMax()
const viewSizeCurrent = computed(() => {
return Math.min(unref(viewSizeMax), props.viewSize)
Expand Down Expand Up @@ -277,6 +280,10 @@ export default defineComponent({
return props.selectedIds.includes(resource.id)
}

const isResourceClickable = (resource: Resource) => {
return !(unref(isEmbedModeEnabled) && !resource.isFolder)
}

const emitSelect = (selectedIds) => {
emit('update:selectedIds', selectedIds)
}
Expand Down Expand Up @@ -486,6 +493,7 @@ export default defineComponent({
getResourceCheckboxLabel,
selectSorting,
isSortFieldSelected,
isResourceClickable,
currentSortField,
resourceIconSize,
ghostElementRef,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { useAppsStore, useConfigStore, useResourcesStore } from '../../piniaStor
import { ApplicationFileExtension } from '../../../apps'
import { Resource, SpaceResource } from '@ownclouders/web-client'
import { storeToRefs } from 'pinia'
import { useEmbedMode } from '../../embedMode'

export const EDITOR_MODE_EDIT = 'edit'
export const EDITOR_MODE_CREATE = 'create'
Expand All @@ -47,6 +48,7 @@ export const useFileActions = () => {
const router = useRouter()
const { $gettext } = useGettext()
const isSearchActive = useIsSearchActive()
const { isEnabled: isEmbedModeEnabled } = useEmbedMode()

const { openUrl } = useWindowOpen()

Expand Down Expand Up @@ -86,6 +88,10 @@ export const useFileActions = () => {
])

const editorActions = computed(() => {
if (unref(isEmbedModeEnabled)) {
return []
}

return appsStore.fileExtensions
.map((fileExtension): FileAction => {
const appInfo = appsStore.apps[fileExtension.app]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import { mock } from 'vitest-mock-extended'
import { computed } from 'vue'
import { Identity } from '@ownclouders/web-client/src/generated'

const mockUseEmbedMode = vi.fn().mockReturnValue({ isLocationPicker: computed(() => false) })
const mockUseEmbedMode = vi
.fn()
.mockReturnValue({ isLocationPicker: computed(() => false), isEnabled: computed(() => false) })

vi.mock('../../../../src/helpers/contextMenuDropdown')
vi.mock('../../../../src/composables/embedMode', () => ({
Expand Down Expand Up @@ -361,6 +363,16 @@ describe('ResourceTable', () => {

expect(wrapper.emitted().fileClick).toBeUndefined()
})

it('does not emit fileClick upon clicking on a resource when embed mode is enabled', async () => {
mockUseEmbedMode.mockReturnValue({
isEnabled: computed(() => true)
})
const { wrapper } = getMountedWrapper()
const tr = await wrapper.find('.oc-tbody-tr-forest .oc-resource-name')
await tr.trigger('click')
expect(wrapper.emitted().fileClick).toBeUndefined()
})
})

describe('resource details', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import ResourceTiles from '../../../../src/components/FilesList/ResourceTiles.vu
import { sortFields } from '../../../../src/helpers/ui/resourceTiles'
import { Resource } from '@ownclouders/web-client'
import { mock } from 'vitest-mock-extended'
import { computed } from 'vue'
import { extractDomSelector } from '@ownclouders/web-client/src/helpers'

vi.mock('../../../../src/composables/viewMode', async (importOriginal) => ({
...(await importOriginal<any>()),
Expand All @@ -11,13 +13,34 @@ vi.mock('../../../../src/composables/viewMode', async (importOriginal) => ({
})
}))

const mockUseEmbedMode = vi.fn().mockReturnValue({ isEnabled: computed(() => false) })
vi.mock('../../../../src/composables/embedMode', () => ({
useEmbedMode: vi.fn().mockImplementation(() => mockUseEmbedMode())
}))

const router = {
push: vi.fn(),
afterEach: vi.fn(),
currentRoute: {
name: 'some-route-name',
query: {},
params: {
driveAliasAndItem: ''
}
},
resolve: (r) => {
return { href: r.name }
}
}

const spacesResources = [
{
id: '1',
name: 'Space 1',
path: '',
type: 'space',
isFolder: true,
indicators: [],
getDriveAliasAndItem: () => '1'
},
{
Expand All @@ -26,10 +49,35 @@ const spacesResources = [
path: '',
type: 'space',
isFolder: true,
indicators: [],
getDriveAliasAndItem: () => '2'
}
]

const resources = [
{
id: 'forest',
driveId: 'forest',
name: 'forest.jpg',
path: 'images/nature/forest.jpg',
extension: 'jpg',
thumbnail: 'https://cdn.pixabay.com/photo/2015/09/09/16/05/forest-931706_960_720.jpg',
indicators: [],
isFolder: false,
type: 'file',
tags: ['space', 'tag', 'moon'],
size: '111000234',
hidden: false,
syncEnabled: true,
outgoing: false,
shareRoles: [],
sharePermissions: [],
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('forest')
}
]

describe('ResourceTiles component', () => {
const originalGetElementById = document.getElementById
const originalGetComputedStyle = window.getComputedStyle
Expand Down Expand Up @@ -71,9 +119,18 @@ describe('ResourceTiles component', () => {
})

it('emits fileClick event upon click on tile', async () => {
const { wrapper } = getWrapper({ resources: spacesResources })
await wrapper.find('resource-tile-stub').trigger('click')
expect(wrapper.emitted('click')).toBeTruthy()
const { wrapper } = getWrapper({ resources: resources })
await wrapper.find('.oc-tiles-item .oc-resource-name').trigger('click')
expect(wrapper.emitted().fileClick[0][0].resources[0].name).toMatch('forest.jpg')
})

it('does not emit fileClick event upon click on tile when embed mode is enabled', async () => {
mockUseEmbedMode.mockReturnValue({
isEnabled: computed(() => true)
})
const { wrapper } = getWrapper({ resources: resources })
await wrapper.find('.oc-tiles-item .oc-resource-name').trigger('click')
expect(wrapper.emitted().fileClick).toBeUndefined()
})

it('emits update:selectedIds event on resource selection and sets the selection', () => {
Expand All @@ -82,7 +139,9 @@ describe('ResourceTiles component', () => {
selectedIds: [spacesResources[0].id]
})
wrapper.vm.toggleSelection(spacesResources[0])
expect(wrapper.find('resource-tile-stub').attributes('isresourceselected')).toEqual('true')
expect(
wrapper.findComponent({ name: 'resource-tile' }).props('isResourceSelected')
).toBeTruthy()
expect(wrapper.emitted('update:selectedIds')).toBeTruthy()
})

Expand All @@ -103,6 +162,7 @@ describe('ResourceTiles component', () => {
it('emits the "sort"-event', async () => {
const index = 2
const { wrapper } = getWrapper({ sortFields })
;(wrapper.vm.$refs.sortDrop as any).tippy = { hide: vi.fn() }
await wrapper.findAll('.oc-tiles-sort-list-item').at(index).trigger('click')
expect(wrapper.emitted('sort')).toBeTruthy()
expect(wrapper.emitted('sort')[0][0]).toEqual({
Expand Down Expand Up @@ -137,7 +197,9 @@ describe('ResourceTiles component', () => {
])('passes the "viewSize" to the OcTile component', async (data) => {
const { wrapper } = getWrapper({ resources: spacesResources, viewSize: data.viewSize })
await wrapper.vm.$nextTick()
expect(wrapper.find('resource-tile-stub').attributes('resourceiconsize')).toEqual(data.expected)
expect(wrapper.findComponent({ name: 'resource-tile' }).props('resourceIconSize')).toEqual(
data.expected
)
})

function getWrapper(props = {}, slots = {}) {
Expand All @@ -151,8 +213,15 @@ describe('ResourceTiles component', () => {
...slots
},
global: {
plugins: [...defaultPlugins({ designSystem: false })],
stubs: { ResourceTile: true }
plugins: [...defaultPlugins()],
mocks: {
$route: router.currentRoute,
$router: router
},
provide: {
$router: router,
$route: router.currentRoute
}
}
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`ResourceTiles component > renders a footer slot 1`] = `
"<div data-v-67d39700="" id="tiles-view" class="oc-px-m oc-pt-l">
<!--v-if-->
<oc-list data-v-67d39700="" class="oc-tiles oc-flex"> </oc-list>
<ul data-v-67d39700="" class="oc-list oc-my-rm oc-mx-rm oc-tiles oc-flex"> </ul>
<!--v-if-->
<div data-v-67d39700="" class="oc-tiles-footer">Hello, ResourceTiles footer!</div>
</div>"
Expand All @@ -12,19 +12,73 @@ exports[`ResourceTiles component > renders a footer slot 1`] = `
exports[`ResourceTiles component > renders an array of spaces correctly 1`] = `
"<div data-v-67d39700="" id="tiles-view" class="oc-px-m oc-pt-l">
<!--v-if-->
<oc-list data-v-67d39700="" class="oc-tiles oc-flex">
<ul data-v-67d39700="" class="oc-list oc-my-rm oc-mx-rm oc-tiles oc-flex">
<li data-v-67d39700="" class="oc-tiles-item has-item-context-menu">
<resource-tile-stub data-v-67d39700="" resource="[object Object]" isresourceselected="false" isextensiondisplayed="true" resourceiconsize="xlarge" draggable="false" resourceroute="[object Object]"></resource-tile-stub>
<div data-v-67d39700="" class="oc-tile-card oc-card oc-card-default oc-rounded" data-item-id="1" draggable="false"><a draggable="false" class="oc-card-media-top oc-flex oc-flex-center oc-flex-middle oc-m-rm" tabindex="-1" attrs="[object Object]"></a>
<div class="oc-card-body oc-p-s">
<div class="oc-flex oc-flex-between oc-flex-middle">
<div class="oc-flex oc-flex-middle oc-text-truncate resource-name-wrapper">
<div class="oc-resource oc-text-overflow">
<!--v-if-->
<div class="oc-resource-details oc-text-overflow"><a draggable="false" class="oc-text-overflow" attrs="[object Object]"></a>
<div class="oc-resource-indicators">
<!--v-if-->
</div>
</div>
</div>
</div>
<div class="oc-flex oc-flex-middle">
<!-- Slot for indicators !-->
<!-- Slot for individual actions -->
<!-- Slot for contextmenu --> <button data-v-67d39700="" type="button" aria-label="Show context menu" class="oc-button oc-rounded oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-raw resource-tiles-btn-action-dropdown" id="context-menu-trigger-1">
<!--v-if-->
<!-- @slot Content of the button --> <span class="oc-icon oc-icon-m oc-icon-passive"><!----></span>
<div id="context-menu-drop-1" class="oc-drop oc-box-shadow-medium oc-rounded oc-overflow-hidden">
<div class="oc-card oc-card-body oc-background-secondary oc-p-s"></div>
</div>
</button>
</div>
</div>
<!--v-if-->
</div>
</div>
</li>
<li data-v-67d39700="" class="oc-tiles-item has-item-context-menu">
<resource-tile-stub data-v-67d39700="" resource="[object Object]" isresourceselected="false" isextensiondisplayed="true" resourceiconsize="xlarge" draggable="false" resourceroute="[object Object]"></resource-tile-stub>
<div data-v-67d39700="" class="oc-tile-card oc-card oc-card-default oc-rounded" data-item-id="2" draggable="false"><a draggable="false" class="oc-card-media-top oc-flex oc-flex-center oc-flex-middle oc-m-rm" tabindex="-1" attrs="[object Object]"></a>
<div class="oc-card-body oc-p-s">
<div class="oc-flex oc-flex-between oc-flex-middle">
<div class="oc-flex oc-flex-middle oc-text-truncate resource-name-wrapper">
<div class="oc-resource oc-text-overflow">
<!--v-if-->
<div class="oc-resource-details oc-text-overflow"><a draggable="false" class="oc-text-overflow" attrs="[object Object]"></a>
<div class="oc-resource-indicators">
<!--v-if-->
</div>
</div>
</div>
</div>
<div class="oc-flex oc-flex-middle">
<!-- Slot for indicators !-->
<!-- Slot for individual actions -->
<!-- Slot for contextmenu --> <button data-v-67d39700="" type="button" aria-label="Show context menu" class="oc-button oc-rounded oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-raw resource-tiles-btn-action-dropdown" id="context-menu-trigger-2">
<!--v-if-->
<!-- @slot Content of the button --> <span class="oc-icon oc-icon-m oc-icon-passive"><!----></span>
<div id="context-menu-drop-2" class="oc-drop oc-box-shadow-medium oc-rounded oc-overflow-hidden">
<div class="oc-card oc-card-body oc-background-secondary oc-p-s"></div>
</div>
</button>
</div>
</div>
<!--v-if-->
</div>
</div>
</li>
<li data-v-67d39700="" class="ghost-tile" aria-hidden="true"></li>
<li data-v-67d39700="" class="ghost-tile" aria-hidden="true"></li>
<li data-v-67d39700="" class="ghost-tile" aria-hidden="true"></li>
<li data-v-67d39700="" class="ghost-tile" aria-hidden="true"></li>
<li data-v-67d39700="" class="ghost-tile" aria-hidden="true"></li>
</oc-list>
</ul>
<!--v-if-->
<div data-v-67d39700="" class="oc-tiles-footer"></div>
</div>"
Expand Down
Loading