removed vue-clipboard2 from package.json #3593
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. |
Julian1998
force-pushed
the
remove-vue-clipboard2
branch
from
67ae0bb to
7932de8
Compare
|
@LukasHirt I absolutely don't like my part where I create a custom event for not crashing the event handler method. Do you think we can delete the whole event handler as it just seems to prevent fast double clicks on the copy to clipboard icon/button. |
I think part of that is also responsible for giving feedback to the user that the link has been copied. At first glance, it might be enough to use local data object in the component. But there might be something that I have overlooked so you can try it and see if it'll work still fine 😉 |
Yeah it is responsible for feedback... BUT we never implemented the visual feedback in the sidebar |
We did.
Same for private link. |
|
Ah I was talking about something like a notification message |
Julian1998
force-pushed
the
remove-vue-clipboard2
branch
from
7932de8 to
359a13c
Compare
Julian1998
force-pushed
the
remove-vue-clipboard2
branch
from
359a13c to
039d753
Compare
|
@LukasHirt well I slightly adapted my code but we can't get rid of these events. We need to emit an event so the success handler method gets triggered. I guess there are several ways to deal with that, but emitting an event seems to be the best in my opinion. What do you think? |
|
@LukasHirt please re-review :) |
| @@ -63,8 +63,7 @@ | |||
| > | |||
| <oc-icon | |||
| v-if="!linksCopied[link.url]" | |||
| v-clipboard:copy="link.url" | |||
| v-clipboard:success="$_clipboardSuccessHandler" | |||
| @click="$_copyPublicLinkToClipboard()" | |||
There was a problem hiding this comment.
Move the click pls to the wrapping button.
Is there something connected to the event outside the link item component? As far as I understood (haven't looked that close though so I might be missing something) we emit the event, then set the copied link globally and then read it again in the link item component. Is that correct? What about not emitting the event but instead using local data object in the component? |
|
Well.... the linkListItem triggers the onCopy event which gets processed in the fileLinkSidebar component. This way we can reuse the successHandler in the filesidebar component for the linkListItem as well as some code for the clipboard in the sidebar itself |
|
@Julian1998 Address pls this comment #3593 (comment) |
My requested re-review was not on intended. I am sorry. Unluckily there was no way to undo this. |
No worries 😉 |
|
@Julian1998 same here, do you know when you will be able to schedule time for this? or should someone else take over? |
|
@Julian1998 just stumbled across this when checking open issues, shall we finish your work or would you like to continue working on it? The project structure has changed a bit, happy to offer some guidance if applicable! |
|
@pascalwengerter I forgot to unassign. We could keep this PR open and someone else could build a new solution based on this branch. But I probably won't finish it. Best luck |
|
Superseded by #5147 |
Description
Removed vue-clipboard2 from the package.json file dependencies
Related Issue
Motivation and Context
We already got copy-to-clipboard so vue-clipboard would be redundant
How Has This Been Tested?
Types of changes
Checklist:
Open tasks: