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

Resharers now visible on top of owner entry #3076

Merged
merged 3 commits into from
Feb 27, 2020
Merged

Conversation

PVince81
Copy link
Contributor

Description

For received shares, the resharers user display names are now shown
on top of the owner entry in the collaborators list, with a reshare
icon, instead of having their own entry in the collaborators list.

Moved reshare related tests to the reshare feature file.

Related Issue

Motivation and Context

Based on feedback after reviewing the feature.

How Has This Been Tested?

  • manual test
  • acceptance test

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

@PVince81 PVince81 self-assigned this Feb 21, 2020
@kulmann
Copy link
Contributor

kulmann commented Feb 21, 2020

Bildschirmfoto 2020-02-21 um 13 09 04
Bildschirmfoto 2020-02-21 um 13 08 45

I like the xsmall icon for the resharer better than the small one.

@LukasHirt
Copy link
Collaborator

LukasHirt commented Feb 21, 2020

Feels a bit odd to me seeing there twice the same name in case of the same owner and sharer.

image

@PVince81
Copy link
Contributor Author

Honestly feels a bit odd to me seeing there twice the same user in case of the same owner and sharer.

bug in this PR

@PVince81
Copy link
Contributor Author

  • Added smaller icon
  • Fixed bug about resharer appearing as the same name
  • Refactored share object
  • Added popup when clicking on the reshare info:

image

  • tweak the popup styles

@PVince81
Copy link
Contributor Author

@kulmann @LukasHirt please re-review

I grew tired of .info so now have reduced its usage.

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 21, 2020

  • change popup text to "Shared with you by"
  • move popup position, maybe down a bit so the owner row stays fully visible

@PVince81
Copy link
Contributor Author

PR adjusted according to comments and checkboxes

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 25, 2020

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 26, 2020

  • use OcDisclosureDrop instead of OcDrop as the popup is not a menu sticking to OcDrop for now as OcDisclosureDrop is currently not customizable enough

@PVince81
Copy link
Contributor Author

Possibly related failures:

  • tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature:63
  • tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature:332
  • tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature:509
  • tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature:520
  • and many resharer scenarios, tests/acceptance/features/webUIResharing/reshareUsers.feature:13 being one of them

@PVince81
Copy link
Contributor Author

fixed the test issue: the selector for the collaborator name was confused because the reshare popup was using the same class name. Fixed by using a different class name in the popup.

@PVince81
Copy link
Contributor Author

Remaining failure:

  • tests/acceptance/features/webUIResharing/reshareUsers.feature:144

@PVince81
Copy link
Contributor Author

Adjusted. Expecting CI to pass.

Then it's done expect for the extra height due to missing ODS update.

For received shares, the resharers user display names are now shown
on top of the owner entry in the collaborators list, with a reshare
icon, instead of having their own entry in the collaborators list.

Moved reshare related tests to the reshare feature file.
@PVince81
Copy link
Contributor Author

rebased to get the ODS update and restart the build due to setup failure

Vincent Petry added 2 commits February 26, 2020 16:38
Added popup with reshare information which lists the resharers, their
avatar and additional info.

Refactored the share/collaborator object to have a clearer distinction
between recipient/collaborator, owner and file owner, which eases the
manipulation that is needed in some places where all fields are copied
over for the resharers and owner.
The info object was just the dirty object returned by the API. Now that
the relevant data has been copied to the parent object in a cleaner
manner, we don't need to pass it in any more.
@PVince81
Copy link
Contributor Author

@LukasHirt all comments addressed, can we merge ?

@PVince81 PVince81 merged commit 626bc88 into master Feb 27, 2020
@delete-merged-branch delete-merged-branch bot deleted the resharers-on-top branch February 27, 2020 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants