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

Start with unified Shared entry to view all shares, ref #5559 #6686

Closed
wants to merge 1 commit into from

Conversation

jancborchardt
Copy link
Member

My uneducated start at getting an Overview of all shared files – ref #5559. The goal is to have the »Shared« entry list all shares, and be collapsed by default. Only expanding it will show the detailed views we have now.

Need help / some proper developer to take over obviously, mainly on two fronts:

  • We need to list all shares: with you, with others and by link. The parameter sharedWithUser doesn’t give the flexibility to return all? »linksOnly« is false by default so that should work.
  • The current »Shared with you«, »Shared with others« and »Shared by link« should become subfolders of the new »Shared« folder. getNavigationManager()->add() doesn’t seem to have the flexibility to add subfolders instead of blindly into the list either.

Then there’s of course stuff like handling cases where some share settings are disabled and entries don’t show up anyway, and fixing the tests – which seem incomplete in the first place as link shares are missing?

cc @nextcloud/sharing @nextcloud/javascript

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
@rullzer
Copy link
Member

rullzer commented Sep 28, 2017

I really don't want to add more OCS API endpoints. Just do the calls separatly and merge the result in js

@jancborchardt
Copy link
Member Author

@rullzer no idea how to do that, can you help?

@jancborchardt
Copy link
Member Author

So how can we proceed here @rullzer @schiessle @nextcloud/javascript? (I'm not a JS developer)

We should really have a basic "Shared" entry showing all shares regardless of type.

@jancborchardt
Copy link
Member Author

@MorrisJobke @blizzz @schiessle yet another one where JS / sharing knowledg is needed.

@jancborchardt jancborchardt added this to the Nextcloud 14 milestone Nov 9, 2017
@MorrisJobke
Copy link
Member

cc @nextcloud/javascript

@rullzer
Copy link
Member

rullzer commented May 23, 2018

As this seems to be inactive for quite some time I'll close this.
If somebody tackles #5559 they will see it linked and can continue.

@rullzer rullzer closed this May 23, 2018
@rullzer rullzer deleted the sharing-overview branch May 23, 2018 16:34
@jancborchardt jancborchardt restored the sharing-overview branch May 28, 2018 14:23
@jancborchardt
Copy link
Member Author

Sorry, this is not a nice way to handle this. I’m asking for pointers since ages … it’s still something I am tackling, and anyone who would start working on #5559 is likely to need similar help since they won’t be so familiar with Nextcloud.

@jancborchardt jancborchardt reopened this May 28, 2018
@codecov
Copy link

codecov bot commented May 28, 2018

Codecov Report

Merging #6686 into master will decrease coverage by 46.1%.
The diff coverage is 26.08%.

@@             Coverage Diff              @@
##             master   #6686       +/-   ##
============================================
- Coverage     53.03%   6.92%   -46.11%     
  Complexity    22583   22583               
============================================
  Files          1417    1417               
  Lines         87866   87889       +23     
  Branches       1341    1343        +2     
============================================
- Hits          46600    6089    -40511     
- Misses        41266   81800    +40534
Impacted Files Coverage Δ Complexity Δ
apps/files_sharing/appinfo/app.php 0% <0%> (-90%) 0 <0> (ø)
apps/files_sharing/js/app.js 70% <35.29%> (-7.11%) 0 <0> (ø)
...ddleware/Security/Exceptions/NotAdminException.php 0% <0%> (-100%) 1% <0%> (ø)
lib/private/Lockdown/Filesystem/NullStorage.php 0% <0%> (-100%) 40% <0%> (ø)
lib/private/Security/TrustedDomainHelper.php 0% <0%> (-100%) 13% <0%> (ø)
apps/user_ldap/lib/Mapping/GroupMapping.php 0% <0%> (-100%) 1% <0%> (ø)
lib/private/Files/Cache/StorageGlobal.php 0% <0%> (-100%) 7% <0%> (ø)
...s/dav/lib/Connector/Sabre/Exception/FileLocked.php 0% <0%> (-100%) 3% <0%> (ø)
lib/private/SystemTag/SystemTag.php 0% <0%> (-100%) 5% <0%> (ø)
lib/private/Repair/NC11/FixMountStorages.php 0% <0%> (-100%) 5% <0%> (ø)
... and 823 more

@jancborchardt
Copy link
Member Author

cc @skjnldsv this is the work-in-progress for the unified shares page, which can be used as a base for the pending shares.

@skjnldsv
Copy link
Member

@jancborchardt see #9909

@jancborchardt
Copy link
Member Author

@skjnldsv I’m not going to be able to implement endpoints ;D as said, I need someone with more JS or PHP knowledge to take over.

@skjnldsv
Copy link
Member

skjnldsv commented Jun 22, 2018

My uneducated start at getting an Overview of all shared files – ref #5559. The goal is to have the »Shared« entry list all shares, and be collapsed by default. Only expanding it will show the detailed views we have now.

Ooookay, let's go!
So it's a bit like the Favorite pr #9720 ? But with shares?

I worked a lot on those lately for the restore share pr, which is a part of #2192
Aaaand it's a nightmare. If you won't mind, my suggestion would be to postpone for 15. Since I want to rebuild the file app to vueJs as well, it should be fairly better to implement this. Right now the plugin system is terrible and really hard to work with (I compared it to discovering an old 2010 js library :p )

What do you think?

@jancborchardt
Copy link
Member Author

jancborchardt commented Jun 22, 2018

It wouldn’t be so similar to the favorites, since there we actually put content in the left navigation – like the favorite folders.

This is simply about changing it from:

  • All files
  • Recent
  • Favorites
  • Shared with you
  • Shared with others
  • Shared by link

To:

  • All files
  • Recent
  • Favorites
  • Shared (collapsed by default, and listing all shares, no matter if by you, with you, or links)
    • By you
    • With you
    • Links

Ok? :)

@newhinton
Copy link
Contributor

@skjnldsv I had the same problem which @jancborchardt mentioned here for the subfolders in the left navigation. I have "solved" this by creating a sublist, which all following items are appended to until an "end"-element is added which terminates this sublist. Since it seems that there is a demand for proper sublist-support, should we design a system which handles this better and more general than my current implementation?

@skjnldsv
Copy link
Member

@newhinton I'm not sure to understand the issue you're encountering. You're talking about design or php code?

@newhinton
Copy link
Contributor

newhinton commented Jun 23, 2018

@skjnldsv I mean the php code, the technical implementation of sublists in the appnavigation.php
( and it's not really an issue, more a proposition to create a way to easily add subitems to any items in the navigationbar)

@skjnldsv
Copy link
Member

@newhinton let's continue on your pr :)

@MorrisJobke
Copy link
Member

Nothing for 14 it seems -> moved to 15

Some start has been made with allow to get back unshared group shares in 14 - see #9909

@skjnldsv
Copy link
Member

skjnldsv commented Jul 6, 2018

The current »Shared with you«, »Shared with others« and »Shared by link« should become subfolders of the new »Shared« folder. getNavigationManager()->add() doesn’t seem to have the flexibility to add subfolders instead of blindly into the list either.

This will be in #9720, could we try to merge this quick so we can move forward?

@jancborchardt
Copy link
Member Author

Closing in favor of #10230

@jancborchardt jancborchardt deleted the sharing-overview branch July 16, 2018 14:21
@MorrisJobke MorrisJobke removed this from the Nextcloud 15 milestone Jul 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants