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

Fix add/remove animations for collaborators and links #2952

Merged
merged 5 commits into from
Feb 6, 2020

Conversation

kulmann
Copy link
Member

@kulmann kulmann commented Jan 30, 2020

Detailed Description

Collaborator and link forms (adding/editing) unnecessarily trigger mutations on loading states (SHARES_LOADING and LINKS_LOADING). This causes animations for adding and removing collaborators/links to fail in the respective panels. While fixing this, some old code has been cleaned up as well.

  • removed triggers for busy-state for collaborator and link saving/creating. using local variable in respective form components now. This fixes the animations in the sidebar panels when adding or removing items, as the loader is not triggered anymore. As saving can be represented within the respective form, there is no need to push this state into the store.
  • removed unused code in collaborator editing form
  • changed buttons in collaborator forms to represent a saving-state when still in saving mode
  • re-introduced a data change detection for collaborators.
  • switched promise handling when adding collaborators to using p-queue

Related Issue

Motivation and Context

Fix animations and clean up old code.

How Has This Ben Tested?

  • local in Chrome and Firefox
  • selenium

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:

@kulmann kulmann added the Status:Needs-Review Needs review from a maintainer label Jan 30, 2020
@kulmann kulmann requested a review from PVince81 January 30, 2020 23:15
@kulmann kulmann self-assigned this Jan 30, 2020
@update-docs
Copy link

update-docs bot commented Jan 30, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@kulmann kulmann force-pushed the fix-modification-animation branch 2 times, most recently from 0f3a7cc to 1b47cfb Compare January 31, 2020 07:41
@PVince81
Copy link
Contributor

Code looks good.

However somehow now when deleting a collaborator or link there is a small 1-2 second delay where nothing happens until the animation begins. I think we do need some kind of spinner there, ideally also something that locks the row so one cannot click twice on delete.

- removed triggers for busy-state for collaborator and link saving/creating.
using local variable in respective form components now. This fixes the
animations in the sidebar panels when adding or removing items, as the
loader is not triggered anymore. As saving can be represented within the
respective form, there is no need to push this state into the store.
- removed unused code in collaborator editing form
- changed buttons in collaborator forms to represent a saving-state when still in
saving mode
- re-introduced a data change detection for collaborators.
- switched promise handling when adding collaborators to using p-queue
@kulmann kulmann force-pushed the fix-modification-animation branch from 1b47cfb to 0688f34 Compare February 6, 2020 09:22
@kulmann
Copy link
Member Author

kulmann commented Feb 6, 2020

@PVince81 The removal on a public link or collaborator now shows a loading spinner where the delete button was before, as soon as delete is clicked. The edit button is removed from the ui at the same time. Another thing I noticed is, that the first add and last removal didn't trigger the animation. Fixed that as well. Please re-review. :-)

@PVince81
Copy link
Contributor

PVince81 commented Feb 6, 2020

As discussed, let's align the spinner sizes with the icon sizes in ODS

@PVince81
Copy link
Contributor

PVince81 commented Feb 6, 2020

Just tested, looks fine as you said.

I've raised owncloud/owncloud-design-system#631 to align spinner sizes with icon sizes.

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81 PVince81 merged commit 2fb2e85 into master Feb 6, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix-modification-animation branch February 6, 2020 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Needs-Review Needs review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Animations broken on adding/deleting public links or collaborators
2 participants