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

Use "Local Links" on the client #5023

Closed
1 of 3 tasks
dragotin opened this issue Jun 30, 2016 · 26 comments
Closed
1 of 3 tasks

Use "Local Links" on the client #5023

dragotin opened this issue Jun 30, 2016 · 26 comments
Assignees
Labels
p2-high Escalation, on top of current planning, release blocker ReadyToTest QA, please validate the fix/enhancement
Milestone

Comments

@dragotin
Copy link
Contributor

dragotin commented Jun 30, 2016

Expected behaviour

Since 9.1 ownCloud server will understand the "local links" as described in owncloud/core#11732

For a good platform experience, there are a couple of places in the client where we could make use of the local links.

Maybe we should introduce a kind of "local link" symbol that copies the local link to the clipboard, similar to the "copy full SHA" button on github.

The symbol could appear here:

  • In the sync protocol in the file column (or in a column next to it, to make it easier)
  • In the activity files concerned by an activity could display a link copy button
  • In the file manager context menu, with an entry "Copy ownCloud Link".

I see the problem that users will not understand the difference between a public link that works without that the user has an account on the ownCloud that has the file and the local link discussed here, that requires a valid user.

Related: owncloud/core#25317

@dragotin dragotin added this to the 2.3.0 milestone Jun 30, 2016
@guruz guruz modified the milestones: 2.4.0, 2.3.0 Oct 31, 2016
@hodyroff
Copy link

Yes, important for 2.4. Include into the share menu, or make it a seperate right click for easiest experience.
User story: Just sitting in the train and could use this right now ;)
If share menu we could also include the /download link thing requested: #5627

@SamuAlfageme
Copy link
Contributor

Implement server permalink feature (owncloud/core#24434) via context menu shortcut.

Scenario:

There's a file an user wants to point at to a colleague he's already sharing the parent directory with. Instead of generating a new shared link for the individual file, he can use a permalink to point to any file in the shared folder hierarchy: e.g.

https://<server>/index.php/f/29 will always point to the same file on both accounts no matter if it's displaced within any of the file trees.

An entry in the file explorer's context menu could be used as shortcut to directly:

  • Copy the permalink to the clipboard: ("Copy permalink")
  • Open file in the containing folder context (file's row highlighted) ("Open <server> in browser")

The permalink is available for both shared and non-shared files so should be available for both use cases.

@pmaier1
Copy link

pmaier1 commented Apr 4, 2017

Yup, #5023 (comment) looks good to me except that I would prefer Copy permanent link and would change Open <server> in browser to View in browser (why would you mention <server> there?)

@pmaier1
Copy link

pmaier1 commented Apr 6, 2017

After some discussions we decided to change the name of the permalink to "private link" (owncloud/core#27593) as it reflects that you need access, doesn't necessarily exclude other users and fits with public links. Still need to promote to all platforms but FYI.

@ogoffart ogoffart added the p2-high Escalation, on top of current planning, release blocker label Apr 28, 2017
@ckamm
Copy link
Contributor

ckamm commented May 8, 2017

@SamuAlfageme @ogoffart So, to make this concrete:

If a file is shared with any users or groups:

  • The "Users and Groups" section of the file share dialog allows copying or mailing of the private link.
  • The context menu for the file gets another option "Copy ownCloud private link"

Adding a new context menu is slightly involved because it needs to touch all desktop integrations, the rest would be straightforward.

@SamuAlfageme
Copy link
Contributor

@ckamm yup 👍

If a file is shared with any users or groups:

Actually, doesn't need to be shared: all the files/folders in server have a private link by default:

The "Users and Groups" section of the file share dialog allows copying or mailing of the private link.

In the early mockups for the new sharing dialog, I imagined the "copy link" button near the file name (like in the web UI) since it's bonded to the file itself rather than specific users/groups and the concept can generate confusion:

I think it's enough if we start just including it on the dialog and add the context menu entry in the future.

@guruz
Copy link
Contributor

guruz commented May 8, 2017

One problem is that in the client we have the oc:id and not the oc:fileid.
The oc:id also contains the server instance ID string, not just the numeric file id.
e.g. 00000003ocr2n5bhxjuy.
We could just split() before oc but that's ugly..

FYI @ogoffart @PVince81 @DeepDiver1975

@ckamm
Copy link
Contributor

ckamm commented May 8, 2017

@guruz Yes, I just noticed the same thing. Is it safe to assume the first 8 bytes of oc:id are the fileid? It seems to be guaranteed by apps/dav/lib/Connector/Sabre/Node.php:getFileId(). If we make use of the specific structure of fileid we need to at least make a note of it there.

Why do we use id over fileid, btw? Federated shares? Then how do we generate the permalink for files in federated shares? Hmh.

@guruz
Copy link
Contributor

guruz commented May 8, 2017

@ckamm I think just legacy reasons.. https://github.com/owncloud/core/search?q=%22oc%3Aid%22&type=Commits&utf8=%E2%9C%93
Back in the days the fileId was just for move detection(?), only now we want to use it for more things.

Maybe we could just start to fetch oc:fileid also and PROPFIND all directories as part of the update.

Then how do we generate the permalink for files in federated shares? Hmh.

hm

@guruz
Copy link
Contributor

guruz commented May 8, 2017

Is it safe to assume the first 8 bytes of oc:id are the fileid

Maybe yes until owncloud/core#22330

@ckamm
Copy link
Contributor

ckamm commented May 8, 2017

@guruz What if we transitioned to fileid only as a migration step? We could query id and fileid, take fileid if supported or otherwise the first 8 bytes of id?

That'd only work if oc:id had this structure from the start, of course. Investigation shows me that this structure is in place since 6.0.0. I'm not sure id existed before?

@pmaier1
Copy link

pmaier1 commented May 8, 2017

In the early mockups for the new sharing dialog, I imagined the "copy link" button near the file name (like in the web UI) since it's bonded to the file itself rather than specific users/groups and the concept can generate confusion

As the 'private link' is not related to sharing but exists for every file we should not have it in the sharing dialog but just put it in the context menu on the same level as 'Share with ownCloud' (or make it a sub-entry: "ownCloud" -> Sub1: "Share"; Sub2: "Copy ownCloud private link"; Sub3: "Open with Browser").

@ckamm
Copy link
Contributor

ckamm commented May 9, 2017

@pmaier1 Conceptually yes, but are there usecases where one cares about the permalink that are unrelated to sharing?

But I do understand the desire to easily share the link to a file that is already shared and adding that to the context menu seems good. I'm mildly concerned that for symmetry, we might want to offer: "Open with browser", "Copy link to clipboard", "Copy direct download link to clipboard", "Email link", "Email direct download link" - and that's just too many options for a good ui.

I'd restrict the file context menu to one of these (probably "Copy link to clipboard").

From a usability point of view I'd advocate for having the ability to get at a local link from the sharing dialog as well: I imagine that when you share something with a user you might want to email them about it directly, handing them the local link. So this is where the full breadth of link distribution options could be made available.

@pmaier1
Copy link

pmaier1 commented May 9, 2017

@pmaier1 Conceptually yes, but are there usecases where one cares about the permalink that are unrelated to sharing?

Well, you could use it for yourself as a unique link to a file/folder. You're right, the other use case is more prominent but I also think that separating it from sharing could make it better understandable (most users don't know what the link is for according to my experience).

But I do understand the desire to easily share the link to a file that is already shared and adding that to the context menu seems good. I'm mildly concerned that for symmetry, we might want to offer: "Open with browser", "Copy link to clipboard", "Copy direct download link to clipboard", "Email link", "Email direct download link" - and that's just too many options for a good ui.

I'd say there are two options
a) Context menu has two entries on the same level: "Share with ownCloud" and "Copy ownCloud link to clipboard"
b) Context menu has one entry "ownCloud" and 3-5 of the mentioned ones as a submenu

I like b) so far.

From a usability point of view I'd advocate for having the ability to get at a local link from the sharing dialog as well: I imagine that when you share something with a user you might want to email them about it directly, handing them the local link. So this is where the full breadth of link distribution options could be made available.

Yes, I agree. We could have it there as well. Something for discussion with some more people?

@michaelstingl
Copy link
Contributor

Name should be "private link" according to @pmaier1 in owncloud-archive/documentation#2986

@guruz
Copy link
Contributor

guruz commented Jun 15, 2017

Tracked in PR -> #5763

@guruz guruz closed this as completed Jun 15, 2017
ckamm added a commit to ckamm/owncloud-client that referenced this issue Jun 23, 2017
It puts the specified file's local link into the clipboard.
ckamm added a commit to ckamm/owncloud-client that referenced this issue Jun 23, 2017
ckamm added a commit to ckamm/owncloud-client that referenced this issue Jun 23, 2017
ckamm added a commit to ckamm/owncloud-client that referenced this issue Jun 23, 2017
ckamm added a commit that referenced this issue Jul 7, 2017
* SocketAPI has COPL_LOCAL_LINK / EMAIL_LOCAL_LINK commands
* The nautilus and dolphing shell integrations show a submenu from which
  one can share as well as access the private link.
* The SocketAPI provides a new GET_STRINGS command to access localized
  strings.
* The private link can also be accessed from the user/group sharing
  dialog.
* The numeric file id is extracted from the full id to create the
  private link url.
@ckamm ckamm reopened this Jul 11, 2017
@ckamm ckamm added the ReadyToTest QA, please validate the fix/enhancement label Jul 11, 2017
@SamuAlfageme
Copy link
Contributor

@ckamm @jturcotte just remembered #5462. What do you guys think about possible future integration with the new sharing dialog / context-menu options?

@jturcotte
Copy link
Member

jturcotte commented Jul 12, 2017

I don't think that this feature prevents this from happening, but it does remove the possibility of handling this cross-platform in the sharing dialog (vs. in each shell extension platform implementation). Instead of having an extra sub-menu like you suggested (since we just added one), we could have two entries directly in the context menu.

e.g. the "ownCloud" menu could appear twice as "ownCloud - user1@server1.com" and "ownCloud - user2@server2.com"

Given that though, it feels to me that very few users would end up using this feature, so the cost/benefit ratio of maintaining this on all platforms would be very very high.

@SamuAlfageme
Copy link
Contributor

@michaelstingl @pmaier1 Screenshots for reference:

Context Menu Private Share tab on the Sharing Dialog
3_new_options screen shot 2017-07-14 at 12 26 22

@michaelstingl
Copy link
Contributor

Could we add "Open link in browser" (#3024) to the shell extension too? (without having the auto-login (#633) solved yet)

@ckamm
Copy link
Contributor

ckamm commented Jul 14, 2017

@michaelstingl Sure #5903 (I'm not adding it here because this one is almost done and adding context menu entries needs touches in all shell integrations, making it more complicated than a few lines)

@SamuAlfageme
Copy link
Contributor

After testing how both #6037 and #5763 (fileid generation; for servers not supporting the oc:privatelink) work perfectly this is quite ready to be closed.

Only concerns left would be @dragotin's original proposal in #5023 (comment) to add shortcuts in the "Activity" tab:

  • In the sync protocol in the file column (or in a column next to it, to make it easier)
  • In the activity files concerned by an activity could display a link copy button

... and to rebuild the shell integrations to include the "Open in browser" (#5023 (comment)) - since #6094 would mandate to do such rebuild: could this happen for 2.4? @ckamm

@ckamm
Copy link
Contributor

ckamm commented Oct 25, 2017

@SamuAlfageme Thanks for testing!

The "Open in browser" in the shell extensions is captured by #5903 which is currently scheduled for 2.5. Similarly adding a context menu to the file entries in the protocol and activity tabs is something that I'd also put into 2.5 and a new ticket. (edit: now #6121)

Meta: I think we closed / readytotest'ed this ticket too early and thereby overlooked the remaining aspects of the original suggestion. Creating separate issues for all the component tasks might have helped.

@michaelstingl if you'd really like these in the client earlier that would be doable.

@SamuAlfageme
Copy link
Contributor

@ckamm yass, closing here 👍

@michaelstingl
Copy link
Contributor

@ckamm no real urgency, but thanks for asking

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-high Escalation, on top of current planning, release blocker ReadyToTest QA, please validate the fix/enhancement
Projects
None yet
Development

No branches or pull requests

9 participants