Skip to content
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

»Share via link« should automatically copy the link to the clipboard #276

Closed
jancborchardt opened this issue Jul 1, 2016 · 16 comments
Closed

Comments

@jancborchardt
Copy link
Member

(In the sharing section of the sidebar.) That will make the sharing flow a lot easier.

@jancborchardt jancborchardt added enhancement design Design, UI, UX, etc. 1. to develop Accepted and waiting to be taken care of medium feature: sharing labels Jul 1, 2016
@MorrisJobke MorrisJobke added the good first issue Small tasks with clear documentation about how and in which place you need to fix things in. label Jul 1, 2016
@go2sh
Copy link
Contributor

go2sh commented Jul 5, 2016

Were do you want to hook in? I'm about to create a PR but I'm not familiar with the coding style. I checked the code and thought about adding a function to the this.model.on('change:linkShare',..) callback or even into the render function. https://github.com/nextcloud/server/blob/master/core/js/sharedialoglinkshareview.js#108

@jancborchardt
Copy link
Member Author

cc @schiessle and @MorrisJobke for input on @go2sh's question. :)

@Faldon
Copy link
Contributor

Faldon commented Jul 5, 2016

As far as I could see (was playing the whole day with this issue), your proposed hook is the only way to go @go2sh . I can grab the link in the shareitem model, but a) I'd rather not have displaying stuff in the model code and b) I was not able to select the text for copying.
Unfortunately I am stuck here. I wanted to use the commandExecute('copy') function, but it seems to be allowed only after a user-generated event.
What would be the way to go here?

@go2sh
Copy link
Contributor

go2sh commented Jul 5, 2016

There is anouter commandExecute('selectAll'). As far as I understand the docs, it selects all text in an editable region. This should solve the selecting problem. So the following could be done:

  • Let the render finish
  • Focus the text field
  • commandExecute('selectAll')
  • commandExecute('copy')

Alternative there is a jquery function called .select() which should select all text as the jquery docs show https://api.jquery.com/select/.

I'll try to write a small function tomorrow and if it works, I'll create a PR where the rest can be discussed.

@go2sh
Copy link
Contributor

go2sh commented Jul 5, 2016

Btw. Part of the code is already there as the text field has a onLinkTextClick event handler which creates the focus and selects the text. :-)

@icewind1991
Copy link
Member

see owncloud/core#25418

@go2sh
Copy link
Contributor

go2sh commented Jul 9, 2016

I was done with that too plus the autocopy. I though about automatically copying it to clipboard. It might erase content, that the user is not are of. Then you are never able to share a link without thinking about your clipboard. I guess clicking on another button is also acceptable. But I don't mind. Should I submit a PR or do you want to include upstream?

@rullzer
Copy link
Member

rullzer commented Jul 11, 2016

I think we should wait for owncloud/core#25418 indeed. I do not want autocopy. I should have control over my own clipboard.

@rullzer
Copy link
Member

rullzer commented Aug 28, 2016

Ok so the clipboard thingy is in. Not the autocopy. I still think that is a bad idea.

@jancborchardt
Copy link
Member Author

Ref #1273 also for the same thing on local link.

@rullzer good point with that it’s not possible on OS X.

@weeman1337
Copy link
Member

It now also works with Safari.

Anyway the "copy" function is now part of the share section:
image

The last point to decide is whether to auto copy or not.
I can understand both sides:
Control over your clipboard ↔ convenience

So, what should we do here @nextcloud/designers

@nextcloud-bot nextcloud-bot removed the stale Ticket or PR with no recent activity label Sep 26, 2018
@skjnldsv
Copy link
Member

@weeman1337 actually the actual code is already supposed to do that, but is broken because the click is triggered before the render (time for the enable public share request to complete)

$('.share-menu .icon-more').click();
$('.share-menu .icon-more + .popovermenu .clipboardButton').click();

I have a pending fix for that: https://github.com/nextcloud/server/compare/public-share-link-menu-copy

But I really dislike what I did, shady javascript scripts to ensure the element is present. :/

@jancborchardt
Copy link
Member Author

jancborchardt commented Dec 21, 2018

The last point to decide is whether to auto copy or not.
I can understand both sides:
Control over your clipboard ↔ convenience

You create a share link, and that mostly means you want to share it. Also, the label literally says "Share link", so you could argue that copying to the clipboard is perfectly normal.

Convenience definitely is more important in this case. This is another of these cases like catching Ctrl-F for our search field, Ctrl-S for saving where lots of people initially disagreed but it just works so well. We are here to make lives easier for regular people, so this is what we do.

@jancborchardt jancborchardt added this to the Nextcloud 18 milestone Mar 13, 2019
@skjnldsv skjnldsv added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of good first issue Small tasks with clear documentation about how and in which place you need to fix things in. labels Jul 10, 2019
@skjnldsv
Copy link
Member

Peek 10-07-2019 14-55

@jancborchardt
Copy link
Member Author

@skjnldsv 🚀 🎉

@skjnldsv
Copy link
Member

Fixed with #15719

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants