feat: add cropping to space images #722
Conversation
AlexAndBear
force-pushed
the
issues/709
branch
3 times, most recently
from
f5b5590 to
f575f36
Compare
|
@kulmann unfortantly its not really that easy to check if the image has the aspect ratio 16/9 in the e2e test |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new cropping feature for space images by introducing a dedicated modal and updating the image upload workflow. Key changes include:
- Implementation of the SpaceImageModal component utilizing cropperjs for image cropping.
- Updates to both E2E and unit tests to support the new cropping flow.
- Modifications in space image action handling across web-app-files and related contexts.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/support/objects/app-files/spaces/actions.ts | Updated response matching to account for cropped images and trigger a confirm button click. |
| packages/web-pkg/tests/unit/components/Spaces/SpaceImageModal.spec.ts | New unit tests for the cropping modal functionality. |
| packages/web-pkg/src/components/Spaces/SpaceImageModal.vue | New modal component for cropping space images using cropperjs. |
| packages/web-pkg/src/components/Spaces/index.ts | Export the new SpaceImageModal component. |
| packages/web-app-files/tests/unit/composables/actions/spaces/useSpaceActionsUploadImage.spec.ts | Updated tests to reflect changes in the image upload action API. |
| packages/web-app-files/src/composables/actions/spaces/useSpaceActionsUploadImage.ts | Refactored to use a modal dispatch instead of direct file upload. |
| packages/web-app-files/src/components/Spaces/SpaceContextActions.vue | Updated change handler for image input to trigger the new modal. |
| packages/web-app-files/src/components/SideBar/Actions/SpaceActions.vue | Consistent update to the image upload action handler via the modal. |
| errors: [error] | ||
| }) | ||
| } | ||
| dispatchModal({ |
There was a problem hiding this comment.
Verify that 'selectedSpace' is assigned before dispatching the modal, as it is used in customComponentAttrs; consider adding a guard to handle cases where selectedSpace might be null.
kulmann
left a comment
kulmann
left a comment
There was a problem hiding this comment.
It's crazy how much of a UX improvement this it... love it! I'd prefer if the new vue script code would be script setup syntax, since we want to convert our codebase to that. Didn't check the modal component code in detail, yet, because reviewing the diff would be pointless, so I'll wait for the script setup syntax 🙈
| </div> | ||
| </template> | ||
|
|
||
| <script lang="ts"> |
There was a problem hiding this comment.
I'd prefer it if new code would be script setup syntax
Maybe @ScharfViktor has an idea? |
I'll try to do it via evaluate |
|
I noticed that after cropping the |
You mean gif ? Expected |
packages/web-app-files/tests/unit/composables/actions/spaces/useSpaceActionsUploadImage.spec.ts
Outdated
Show resolved
Hide resolved
yeah . |
|
e2e test here #731 |
Description
Related Issue
How Has This Been Tested?
Types of changes