-
Notifications
You must be signed in to change notification settings - Fork 159
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
[full-ci] Refactor CreateAndUpload into composables #8938
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
packages/web-app-files/src/components/FilesList/ContextActions.vue
Outdated
Show resolved
Hide resolved
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.
I love where this is going!
packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Outdated
Show resolved
Hide resolved
packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Outdated
Show resolved
Hide resolved
packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Outdated
Show resolved
Hide resolved
packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Outdated
Show resolved
Hide resolved
packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Outdated
Show resolved
Hide resolved
packages/web-app-files/src/composables/actions/files/useFileActionsCreateNewFile.ts
Outdated
Show resolved
Hide resolved
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.
Few smaller things from my side, looks quite good overall.
Types need to be improved, but that can be done in a follow-up IMO because they should be added and used globally (talking about newFileHandlers
and mimetypesAllowedForCreation
).
Also, we definitely need to test this with a WOPI instance before merging.
packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Outdated
Show resolved
Hide resolved
const openAction = newFileHandler.action | ||
actions.push({ | ||
name: 'create-new-file', | ||
icon: 'add', |
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.
Idea for the future: In CreateAndUpload
we currently load the icon via getIconResource
. Would be nice to somehow handle this here. Maybe we can even remove ext: newFileHandler.ext
then (which is only used for the icon).
packages/web-app-files/src/composables/actions/files/useFileActionsCreateNewFile.ts
Outdated
Show resolved
Hide resolved
packages/web-app-files/src/composables/actions/files/useFileActionsCreateNewFolder.ts
Outdated
Show resolved
Hide resolved
packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Outdated
Show resolved
Hide resolved
packages/web-app-files/src/composables/actions/files/useFileActionsCreateNewFile.ts
Outdated
Show resolved
Hide resolved
906b21a
to
2a3ae5b
Compare
Results for acceptance oC10 https://drone.owncloud.com/owncloud/web/35378/14/1 💥 The acceptance tests failed on retry. Please find the screenshots inside ...
webUICreateFilesFolders-createFile_feature-L13.pngwebUICreateFilesFolders-createFile_feature-L25.pngwebUICreateFilesFolders-createFile_feature-L31.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L19.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L20.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L21.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L22.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L23.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L24.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L25.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L26.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L43.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L44.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L60.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L61.pngwebUICreateFilesFolders-createFolderEdgeCases_feature-L62.pngwebUICreateFilesFolders-createFolders_feature-L12.pngwebUICreateFilesFolders-createFolders_feature-L22.pngwebUICreateFilesFolders-createFolders_feature-L53.pngwebUICreateFilesFolders-createFolders_feature-L61.pngwebUICreateFilesFolders-createFolders_feature-L74.pngwebUIDeleteFilesFolders-deleteFilesFolders_feature-L114.png |
2a3ae5b
to
8bfdfe3
Compare
Results for acceptance oCIS https://drone.owncloud.com/owncloud/web/35378/54/1 |
0582c66
to
3de5db8
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.
One last nitpick from my side, rest LGTM!
packages/web-app-files/src/components/SideBar/Shares/Links/CreateQuickLink.vue
Outdated
Show resolved
Hide resolved
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 one! Potential follow-ups for the future:
- Improve types (especially
newFileHandlers
andmimetypesAllowedForCreation
) - Make use of the
icon
property on theactions
computed (see [full-ci] Refactor CreateAndUpload into composables #8938 (comment)) - Maybe try to reduce code duplication a bit more. The methods for checking the name e.g. are almost identical
Kudos, SonarCloud Quality Gate passed! |
Description
Refactor CreateAndUpload actions into composables
Related Issue
Motivation and Context
In order to use "create new file" (type: xyz) or "create new folder" on multiple places we refactored them into composables
As needed in the linked whitespace contextmenu
Types of changes
Checklist:
Open tasks: