make it possible to share the current folder #1618

ChristophWurst · 2016-10-04T11:04:54Z

With this PR I want to add a little share icon to the file list breadcrumb view to reflect whether the current folder is shared or not. On top of this enhancement, I'll add a click handler for opening the share tab view, to finally be able share the current folder without having to navigate up one level.

@rullzer @schiessle I'm totally new to the sharing code. What's the best way to determine whether the current folder has been shared or not? From inspecting responses on my test env I've observed that the share-types property of the directory's PROPFIND request is only set if the folder was shared. Is that correct? Can I rely on that property or should I use something different?

TODO:

show Shared + icon when the folder is shared, similar to the file list rows
show nothing for now if the folder is not shared (there should be a icon/button later though)
bind to change events on the directory's file info model and update the UI accordingly
add tests

Fix #1072

mention-bot · 2016-10-04T11:04:57Z

@ChristophWurst, thanks for your PR! By analyzing the history of the files in this pull request, we identified @PVince81, @butonic and @icewind1991 to be potential reviewers.

ChristophWurst · 2016-10-04T11:07:14Z

cc @jancborchardt for the UI part. Do you have an idea how it should look like?

jancborchardt · 2016-10-04T13:53:43Z

Yeah, the UI part is described in #1072 :) Anything else you need?

ChristophWurst · 2016-10-04T17:11:05Z

Alright, opening the share tab was easier than I thought. However, the passed OCA.Files.FileInfoModel seems to lack some properties that are passed when opening the sidebar for sharing files. Therefore it currently claims that sharing is not allowed.

rullzer · 2016-10-05T06:18:54Z

@ChristophWurst yes that property is only set if the folder was shared. I'm not sure if it is recursive at the moment. but that is a different issue.

codecov-io · 2016-10-10T10:26:35Z

Current coverage is 57.80% (diff: 0.00%)

Merging #1618 into master will decrease coverage by <.01%

@@             master      #1618   diff @@
==========================================
  Files          1094       1095     +1   
  Lines         62798      62849    +51   
  Methods        6996       7006    +10   
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          36301      36328    +27   
- Misses        26497      26521    +24   
  Partials          0          0

Diff Coverage	File Path
0%	apps/files_sharing/appinfo/app.php

Powered by Codecov. Last update 1c8f9d6...9bd181b

ChristophWurst · 2016-10-17T16:28:24Z

apps/files_sharing/js/sharebreadcrumbview.js

@@ -68,6 +76,9 @@
 					dirInfo: self._dirInfo
 				});
 			});
+			this._shareTab.on('sharesChanged', function(shareModel) {
+				alert('aaoobb');


this is triggered correctly. However, the shareTypes property of the FileInfoModel is not upated, therefore the view does not reflect the current sharing status

Ah mmm joy. I'll look into that.

jancborchardt · 2016-10-18T08:50:48Z

remove »aaoobb« alert ;)
when the folder is shared via link, the indicator should use the correct icon icon-public to be symmetric to what we do in the file list
there are 2 nested spans for the icon-share. Instead of that, the span next to the folder name should directly have the icon-share class and also the pointer style, because it already has the padding for clickability too. No need for nesting here.
small glitch: When navigating back up from a subfolder, the folder name is erased from the breadcrumb, but the share indicator still shows. Should vanish as well.
reduce whitespace around icon a bit
we do not need the single icon class anymore. Any class prefixed icon- takes care of that

Otherwise fuck yeah, awesome work so we can finally easily share photo folders! :)

schiessle · 2016-11-02T11:35:53Z

I just tried it but I couldn't find the option to share the current folder. But maybe it is to early to try it out.

Just one remark, because @ChristophWurst asked for sharing stuff. Instead of having a different code path to create shares, how about just opening the sidebar for the selected folder if you click on the bread-crumb and then use the existing share dialog? The only thing we would need to implement here is the click on the bread-crumb to open the sidebar for the folder? Maybe the same could/should happen if you click on a empty space in the file listing?

jancborchardt · 2016-11-02T14:13:15Z

Instead of having a different code path to create shares, how about just opening the sidebar for the selected folder if you click on the bread-crumb and then use the existing share dialog? The only thing we would need to implement here is the click on the bread-crumb to open the sidebar for the folder?

That was my impression of what @ChristophWurst does here. :)

ChristophWurst · 2016-11-02T14:37:26Z

@schiessle @jancborchardt that's exactly what this PR does :-)

ChristophWurst · 2016-11-07T10:44:50Z

apps/files_sharing/js/sharebreadcrumbview.js

+				var shareTypes = [];
+				var shares = shareModel.getSharesWithCurrentItem();
+
+				for(i = 0; i < shares.length; i++) {


TODO: i is undefined

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

…hare tab Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

…y info model changes Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

rullzer · 2016-11-07T14:41:59Z

Ok this should(tm) work now.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

ChristophWurst · 2016-11-08T09:38:21Z

Ok this should(tm) work now.

Confirmed, it works. 👍 for @rullzer's changes :-)

rullzer · 2016-11-08T09:56:28Z

Almost goes without saying but 👍 for the intial stuff

MorrisJobke · 2016-11-08T10:13:41Z

I'm on it - I will fix the styling

* properly center share icon * also apply styles for public share icon Signed-off-by: Morris Jobke <hey@morrisjobke.de>

MorrisJobke · 2016-11-08T10:17:29Z

I fixed the styling:

properly center share icon
also apply styles for public share icon

Before:

After:

Tested in Safari, Firefox, Chrome and IE11 👍

skjnldsv · 2016-11-10T15:51:55Z

core/css/styles.css

@@ -901,7 +901,7 @@ div.crumb > span {
 	color: #555;
 }
 div.crumb.last a {
-	padding-right: 14px;
+	padding-right: 0px;


In fact this is causing an issue on the home folder since the span next to it is hidden.

div.crumb.last a + span { margin-left: -24px; }

Will do the trick, can you test @MorrisJobke ?

Doesn't work here - then the icon is overlapped in subfolders.

@skjnldsv Could you open an issue for this?

Yes I can :)

ChristophWurst added enhancement 2. developing Work in progress feature: sharing feature: files labels Oct 4, 2016

ChristophWurst added this to the Nextcloud 11.0 milestone Oct 4, 2016

MorrisJobke mentioned this pull request Oct 4, 2016

Nextcloud 11 overview #745

Closed

47 tasks

ChristophWurst force-pushed the show-folder-shared-status branch from ab69753 to d4c34a9 Compare October 4, 2016 17:09

ChristophWurst changed the title ~~[WIP] show whether the current folder was shared or not~~ [WIP] make it possible to share the current folder Oct 4, 2016

ChristophWurst force-pushed the show-folder-shared-status branch from d4c34a9 to 6a790f4 Compare October 10, 2016 08:16

ChristophWurst changed the title ~~[WIP] make it possible to share the current folder~~ make it possible to share the current folder Oct 10, 2016

rullzer force-pushed the show-folder-shared-status branch from 3310da4 to 3f1e935 Compare October 14, 2016 09:22

ChristophWurst commented Oct 17, 2016

View reviewed changes

jancborchardt added the design Design, UI, UX, etc. label Oct 18, 2016

jancborchardt mentioned this pull request Oct 25, 2016

New folder creation flow: descend into newly created folder by clicking enter again #1905

Closed

rullzer force-pushed the show-folder-shared-status branch from 9af4af6 to e2d16db Compare November 7, 2016 10:27

ChristophWurst commented Nov 7, 2016

View reviewed changes

ChristophWurst added 4 commits November 7, 2016 14:58

show whether the current folder was shared or not

c5597ba

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

let users share the curren folder

bc1cb8a

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

refactor share permission logic into own method to reuse it for the s…

9e5e120

…hare tab Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

visually reflect sharing state of current folder

5bf7e55

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

ChristophWurst and others added 6 commits November 7, 2016 14:58

the root folder must not be shared

36cf779

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

update breadcrumb view whenever the share information on the director…

c55718a

…y info model changes Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

Listen to sharetab changes

1190ce1

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

detail fixes for share current folder

8de4d4f

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>

Actually update the state

b39a17e

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

Show link icon when shared by link

07d027f

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

rullzer force-pushed the show-folder-shared-status branch from 9bd181b to 07d027f Compare November 7, 2016 14:39

Add tests

0c1fd16

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

rullzer force-pushed the show-folder-shared-status branch from b4e8204 to 0c1fd16 Compare November 7, 2016 20:02

rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 8, 2016

fix share icon position and remove debug log

6c32349

Fix styling of the breadcrumbs icon

742ece8

* properly center share icon * also apply styles for public share icon Signed-off-by: Morris Jobke <hey@morrisjobke.de>

MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 8, 2016

MorrisJobke merged commit 5f15020 into master Nov 8, 2016

MorrisJobke deleted the show-folder-shared-status branch November 8, 2016 15:23

skjnldsv reviewed Nov 10, 2016

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make it possible to share the current folder #1618

make it possible to share the current folder #1618

ChristophWurst commented Oct 4, 2016 •

edited

Loading

mention-bot commented Oct 4, 2016

ChristophWurst commented Oct 4, 2016

jancborchardt commented Oct 4, 2016

ChristophWurst commented Oct 4, 2016

rullzer commented Oct 5, 2016

codecov-io commented Oct 10, 2016 •

edited

Loading

ChristophWurst Oct 17, 2016

rullzer Nov 7, 2016

jancborchardt commented Oct 18, 2016 •

edited by rullzer

Loading

schiessle commented Nov 2, 2016

jancborchardt commented Nov 2, 2016

ChristophWurst commented Nov 2, 2016

ChristophWurst Nov 7, 2016 •

edited by rullzer

Loading

rullzer commented Nov 7, 2016

ChristophWurst commented Nov 8, 2016

rullzer commented Nov 8, 2016

MorrisJobke commented Nov 8, 2016

MorrisJobke commented Nov 8, 2016

skjnldsv Nov 10, 2016

skjnldsv Nov 10, 2016

MorrisJobke Nov 10, 2016

skjnldsv Nov 10, 2016

make it possible to share the current folder #1618

make it possible to share the current folder #1618

Conversation

ChristophWurst commented Oct 4, 2016 • edited Loading

mention-bot commented Oct 4, 2016

ChristophWurst commented Oct 4, 2016

jancborchardt commented Oct 4, 2016

ChristophWurst commented Oct 4, 2016

rullzer commented Oct 5, 2016

codecov-io commented Oct 10, 2016 • edited Loading

Current coverage is 57.80% (diff: 0.00%)

ChristophWurst Oct 17, 2016

Choose a reason for hiding this comment

rullzer Nov 7, 2016

Choose a reason for hiding this comment

jancborchardt commented Oct 18, 2016 • edited by rullzer Loading

schiessle commented Nov 2, 2016

jancborchardt commented Nov 2, 2016

ChristophWurst commented Nov 2, 2016

ChristophWurst Nov 7, 2016 • edited by rullzer Loading

Choose a reason for hiding this comment

rullzer commented Nov 7, 2016

ChristophWurst commented Nov 8, 2016

rullzer commented Nov 8, 2016

MorrisJobke commented Nov 8, 2016

MorrisJobke commented Nov 8, 2016

skjnldsv Nov 10, 2016

Choose a reason for hiding this comment

skjnldsv Nov 10, 2016

Choose a reason for hiding this comment

MorrisJobke Nov 10, 2016

Choose a reason for hiding this comment

skjnldsv Nov 10, 2016

Choose a reason for hiding this comment

ChristophWurst commented Oct 4, 2016 •

edited

Loading

codecov-io commented Oct 10, 2016 •

edited

Loading

jancborchardt commented Oct 18, 2016 •

edited by rullzer

Loading

ChristophWurst Nov 7, 2016 •

edited by rullzer

Loading