Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Fix UI while a share is being added#417

Merged
MorrisJobke merged 1 commit into
masterfrom
fix-ui-while-a-share-is-added
Mar 23, 2018
Merged

Fix UI while a share is being added#417
MorrisJobke merged 1 commit into
masterfrom
fix-ui-while-a-share-is-added

Conversation

@danxuliu

Copy link
Copy Markdown
Member

After calling Gallery.Share.share the UI was restored (the working icon was hidden, the input field enabled...). However, share is asynchronous, so the UI was restored while the share was still being
added. Now the UI is restored in the share callbacks, so it is restored once the sharing finished, either successfully or with a failure; in this later case now a notification is also shown.

Probably not worth a backport, although it should be straightforward to do it.

How to test

  • Modify the code of ShareAPIController and add sleep(10); to the beginning of the create method to simulate a slow server
  • Open an album in the Gallery app
  • Show the Share dialog for the album
  • In the search field type the name of a user that exists
  • Click on the name of that user shown in the dropdown to add the share

Expected result
The input field is disabled and the working icon is shown until the share is finally added.

Actual result
The input field is enabled and the working icon is hidden immediately.

After calling "Gallery.Share.share" the UI was restored (the working
icon was hidden, the input field enabled...). However, "share" is
asynchronous, so the UI was restored while the share was still being
added. Now the UI is restored in the share callbacks, so it is restored
once the sharing finished, either successfully or with a failure; in
this later case now a notification is also shown.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu added bug Something isn't working design Related to the design feature:sharing 3. to review Waiting for reviews labels Mar 23, 2018
@danxuliu danxuliu added this to the Nextcloud 14 milestone Mar 23, 2018
@codecov

codecov Bot commented Mar 23, 2018

Copy link
Copy Markdown

Codecov Report

Merging #417 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #417   +/-   ##
=========================================
  Coverage     82.55%   82.55%           
  Complexity      360      360           
=========================================
  Files            38       38           
  Lines          1313     1313           
=========================================
  Hits           1084     1084           
  Misses          229      229

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f413a1e...080539d. Read the comment docs.

@MorrisJobke MorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and works 👍

@MorrisJobke MorrisJobke merged commit 2363e4f into master Mar 23, 2018
@MorrisJobke MorrisJobke deleted the fix-ui-while-a-share-is-added branch March 23, 2018 08:00
@MorrisJobke MorrisJobke added the backport-request Pending backport by the backport-bot label Mar 23, 2018
@MorrisJobke

Copy link
Copy Markdown
Member

Probably not worth a backport, although it should be straightforward to do it.

Let's do it ... stable13 should also fix the paper cuts ;)

@MorrisJobke

Copy link
Copy Markdown
Member

Let's do it ... stable13 should also fix the paper cuts ;)

But only if it applies without problems ;)

@danxuliu danxuliu removed the backport-request Pending backport by the backport-bot label Mar 23, 2018
@danxuliu

Copy link
Copy Markdown
Member Author

Let's do it ... stable13 should also fix the paper cuts ;)

But only if it applies without problems ;)

Done; and although I have not checked it, it probably applies without problems on stable12 too if needed ;-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

3. to review Waiting for reviews bug Something isn't working design Related to the design

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants