-
Notifications
You must be signed in to change notification settings - Fork 25
feat: add context menu action to remove space image #829
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
Conversation
ed57fbc to
7c938e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a “Delete image” action for spaces and groups all space-image related commands under a new “Space image” dropdown, while renaming the upload label.
- Introduce
useSpaceActionsDeleteImagecomposable with modal confirmation and service calls - Update unit tests for the new delete action and rename upload label
- Refactor context menus and sidebar components to nest upload, set icon, and delete under a “Space image” drop
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/web-pkg/src/composables/actions/spaces/useSpaceActionsDeleteImage.ts | New composable implementing delete flow, store updates, and messages |
| packages/web-pkg/tests/unit/composables/actions/spaces/useSpaceActionsDeleteImage.spec.ts | Tests for visibility, handler modal, and success/error messaging |
| packages/web-pkg/src/composables/actions/spaces/index.ts | Export useSpaceActionsDeleteImage |
| packages/web-app-files/src/composables/actions/spaces/useSpaceActionsUploadImage.ts | Change label from “Edit image” to “Upload image” |
| packages/web-app-files/src/components/Spaces/SpaceHeader.vue | Clear imageContent when no image data |
| packages/web-app-files/src/components/Spaces/SpaceContextActions.vue | Add delete-image action, define Space image dropdown |
| packages/web-app-files/src/components/SideBar/Actions/SpaceActions.vue | Add delete-image action to sidebar |
Comments suppressed due to low confidence (2)
packages/web-pkg/tests/unit/composables/actions/spaces/useSpaceActionsDeleteImage.spec.ts:59
- The test for permission-granted visibility omits
spaceImageData, causingisVisibleto return false. MockspaceImageDataalongsidecanEditImageto ensure the test accurately reflects the visible state.
resources: [mock<SpaceResource>({ canEditImage: () => true })]
packages/web-app-files/src/components/SideBar/Actions/SpaceActions.vue:69
- [nitpick] Action aliases differ between components (
deleteImageActionsvsdeleteSpaceImageActions). Rename one to match the other for consistency.
const { actions: deleteSpaceImageActions } = useSpaceActionsDeleteImage()
| } from '@opencloud-eu/web-pkg' | ||
| import { useSpaceActionsUploadImage } from '../../composables' | ||
| import { computed, defineComponent, PropType, Ref, ref, toRef, unref, VNodeRef } from 'vue' | ||
| import { MenuSection } from '@opencloud-eu/web-pkg/src/components/ContextActions/ContextActionMenu.vue' |
Copilot
AI
Jun 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Importing MenuSection directly from an internal path may break if the package structure changes. Consider exporting it from the package's public API surface instead.
JammingBen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works nice, I found one weird edge case though: when creating a new space, I have the option to remove the space image, even if I didn't set it before. Removing it then renders some icon until I refresh the page and it shows the default image again.
packages/web-app-files/src/components/Spaces/SpaceContextActions.vue
Outdated
Show resolved
Hide resolved
…ns.vue Co-authored-by: Jannik Stehle <50302941+JammingBen@users.noreply.github.com>
That's because the backend sets a default space image from their templates |
kulmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
Description
Main motivation of this PR:
Move 'Upload image' and 'Set icon' context menu items into 'Space image' drop and add 'Delete image' action
Misc improvements in this PR:
How Has This Been Tested?
Types of changes