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.

Sign up for GitHub

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

Jump to bottom

Feature/shares overview #10230

Merged
merged 10 commits into from
Jul 17, 2018
Merged

Feature/shares overview #10230

merged 10 commits into from
Jul 17, 2018

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Jul 13, 2018 •
edited
Loading

Fixes #5559

Found bugs (unrelated):

  • All sharing in sidebar appear as "reshare not allowed" (master as well)
  • Error in console when opening the sidebar (master as well)
Uncaught (in promise) TypeError: Right-hand side of 'instanceof' is not an object
    at FileList.updateRow (sharedfilelist.js:165)

rullzer and others added 3 commits July 13, 2018 07:31
@rullzer @skjnldsv
Tiny start of the share overview
1f9e500 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@skjnldsv
Upgraded navigation submenu management and api + created sharing submenu
6f45607 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Fix fav quickaccess ressource addition
e1a2bb9 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added this to the Nextcloud 14 milestone Jul 13, 2018
@skjnldsv skjnldsv self-assigned this Jul 13, 2018
@skjnldsv skjnldsv requested review from rullzer and schiessle July 13, 2018 10:02
@skjnldsv
Fixed sharing coding style and display - Added all shares overview
34368cc 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

Fixed if condition

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the feature/shareoverview branch from 9652faf to 34368cc Compare July 13, 2018 10:24
@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 13, 2018
@skjnldsv
Fixed phpunit
a349f2a 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
MorrisJobke
MorrisJobke approved these changes Jul 13, 2018
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works 👍

@MorrisJobke
Copy link
Member

MorrisJobke commented Jul 13, 2018 •
edited by skjnldsv
Loading

One small thing:

  • in the overview received shares are listed as "Shared with MYNAME" instead of "Shared by OTHERPERSON" (which is the case in the section "Shared with you"):

How it is (as admin):
bildschirmfoto 2018-07-13 um 17 45 44
How it should be (as admin):
bildschirmfoto 2018-07-13 um 17 46 32

@skjnldsv
Fix share by/with check
1879085 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Copy link
Member Author

@MorrisJobke : Fixed! :)

@nextcloud-bot nextcloud-bot mentioned this pull request Jul 14, 2018
Open
@jancborchardt
Copy link
Member

jancborchardt commented Jul 16, 2018 •
edited by MorrisJobke
Loading

  • Should be called "Shares" instead of "Share overview"
  • "Shared with others" should be the first entry in the sublist cause it’s your files
  • Why is "Deleted shares" not a subentry of it?
  • The highlight bar of the subentries is floating in mid-air? Should also be on the left side of the viewport

Otherwise real nice, wohoo! :)

@jancborchardt jancborchardt mentioned this pull request Jul 16, 2018
Closed
2 tasks
@MorrisJobke
Copy link
Member

I fixed most of the comments from @jancborchardt except the last one which should be addressed separately.

@MorrisJobke
Rename "Shares overview" to "Shares"
98cf2ba 
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Make section "deleted shares" a subsection of "shares"
c0fcc0e 
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Reorder position of "Shared by me" and "Shared with me"
92e3069 
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
jancborchardt
jancborchardt approved these changes Jul 16, 2018
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, checked with @MorrisJobke :)

@MorrisJobke 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 Jul 16, 2018
@jancborchardt
Copy link
Member

Follow-up polishing issue at #10261

@MorrisJobke
Copy link
Member

@skjnldsv Mind to look at the failing jsunit tests? :(

@skjnldsv
Copy link
Member Author

@MorrisJobke I cry so much with those tests! ^^

Why is "Deleted shares" not a subentry of it?

it was not to be confusing since the deleted shares are not compatible with the overview. So the user does not thinks they should appear on the overview :)

@skjnldsv
Jsuinit fixes
cdb4243 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Copy link
Member Author

All fix! :)

LGTM 🚢

@MorrisJobke MorrisJobke merged commit a155f80 into master Jul 17, 2018
@MorrisJobke MorrisJobke deleted the feature/shareoverview branch July 17, 2018 10:53
@nickvergessen nickvergessen mentioned this pull request Jul 23, 2018
Merged
@nextcloud-bot nextcloud-bot mentioned this pull request Jul 26, 2018
Closed
This was referenced Oct 29, 2018
Closed
Closed
@nextcloud-bot nextcloud-bot mentioned this pull request Nov 9, 2018
Open
15 tasks
@nextcloud-bot nextcloud-bot mentioned this pull request Nov 20, 2018
Closed
@danxuliu danxuliu mentioned this pull request May 6, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jancborchardt jancborchardt jancborchardt approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@rullzer rullzer Awaiting requested review from rullzer

@schiessle schiessle Awaiting requested review from schiessle

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

Assignees

@skjnldsv skjnldsv

Labels
4. to release Ready to be released and/or waiting for tests to finish design Design, UI, UX, etc. enhancement feature: sharing high
Projects
None yet
Milestone
Nextcloud 14
Development

Successfully merging this pull request may close these issues.
4 participants
@skjnldsv @MorrisJobke @jancborchardt @rullzer