Load and display quick actions #3573
Conversation
LukasHirt
force-pushed
the
feature/quick-actions
branch
from
01e8ea5
to
59374aa
Compare
| @@ -25,6 +25,7 @@ | |||
| "@babel/preset-env": "^7.6.0", | |||
| "@babel/register": "^7.6.0", | |||
| "babel-loader": "^8.0.6", | |||
| "copy-to-clipboard": "^3.3.1", | |||
There was a problem hiding this comment.
We already have vue-clipboard2 in core. The lib you chose looks like it's adopted by a broader community, but on first glance I couldn't decide for one. However, we should only use one library, otherwise we increase the bundle size without a good reason. Probably best to leave this as is (or use the one we already have in core) and write a technical debt ticket for quick research on which clipboard lib we want to use.
There was a problem hiding this comment.
The problem is that vue-clipboard2 cannot be used here when "just" importing the bundle. It needs to be initialised in the Vue instance. This would mean that we'd need to pass it as part of the context to the handler function and since this is not a feature which would be used that often probably, it felt wrong to have it in there.
But you're absolutely right, having two bundles for one feature is ugly.
There was a problem hiding this comment.
In this case, it would make sense IMO to use the new bundle instead of vue-clipboard2. The advantage of the latter is the ready-to-use Vue directive. With the new bundle, we'd need to either use methods then or create our own directive. If you'd be fine with this change I would go ahead and replace it. I'd do this in separate PR though not to mix different parts of the app in one PR.
There was a problem hiding this comment.
Cool, fully agree. And having it in a separate PR makes more sense. 👍
apps/files/src/default.js
Outdated
| icon: 'link', | ||
| handler: ctx => { | ||
| // FIXME: Translate name | ||
| const params = { name: 'Quick action link' } |
There was a problem hiding this comment.
Does const params = { name: $gettext('Quick action link') } not work here for translation? Or just forgotten / overlooked?
There was a problem hiding this comment.
Unfortunately no. $gettext function implemented in this file is only a dummy function which ensures that strings will be exported by the easygettext but not translated. That is done via the vue-gettext which is initialised in the Vue instance.
I thought we had a ticket about this but cannot find it now (only this similar one about translations in themes #1942 ). I'll try to look a bit more and if I won't find it, I'll open a new one.
|
@kulmann I tried to address your comments and pushed a new commit with few changes - added the missing "an" in changelog, moved quick actions to own file to make the default.js file a bit cleaner and passed store into context to give more flexibility to devs and to clean a bit up the quick actions component. Pls re-review. |
To provide a quick way of executing certain actions, we have implemented quick actions. Every extension can define own quick action by creating an object called quickActions and exporting it. Quick actions are then displayed in the actions column inside of the files list.
LukasHirt
force-pushed
the
feature/quick-actions
branch
from
dd3ba3d
to
a9651d9
Compare
Description
Adding an extension point into files apps for quick actions.
By creating and exporting object called quickActions, developers can define an action which will be then displayed in the files list.
Screenshots (if appropriate):
Types of changes
Open tasks: