Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added upload pending prompt on close/reload #4840

Merged
merged 3 commits into from
May 28, 2021

Conversation

anastr0
Copy link
Contributor

@anastr0 anastr0 commented Mar 23, 2021

Description

Added an unload event listener that prompts user if there are pending uploads while user closes/ reloads/ navigates to another URL.

Related Issue

Motivation and Context

The pending uploads are cancelled on closing/reloading window. A prompt is required to warn users of upload cancellation.

How Has This Been Tested?

  • tested with the latest changes from owncloud/web master branch
  • tested by uploading many files and also with large files
  • tested on tab close/ reload/ navigating to another URL
  • tested on Chrome 88.0 and Firefox 86.0

Screenshots (if appropriate):

owncloud-2590

Types of changes

  • Detect tab close/ reload/ navigation and prompt if any uploads are pending.

Checklist:

  • Added unload event listener
  • Remove unload event listener before component destroy

@update-docs
Copy link

update-docs bot commented Mar 23, 2021

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.

@CLAassistant
Copy link

CLAassistant commented Mar 23, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

Works like a charm locally, I'll do a quick research and see if we can add a test to our suite :)

@anaswaratrajan could you add a changelog item to this PR inside the changelog/unreleased folder? Name of the file should follow the schema of enhancement-your-change (since it's neither a bugfix nor a breaking change), and for the content of the file you can take inspiration from the existing changelog items!

Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@pascalwengerter
Copy link
Contributor

@kulmann there seems to be a way to test this behaviour (see SeleniumHQ/selenium#8638), but not sure if we want to have tests for this and if so on which level to add them..

@anastr0
Copy link
Contributor Author

anastr0 commented Mar 26, 2021

@pascalwengerter @kulmann Should I look into adding a unit test using jest?

@pascalwengerter
Copy link
Contributor

@pascalwengerter @kulmann Should I look into adding a unit test using jest?

That would be lovely, yeah! Not sure how to handle long-running uploads in there, but perhaps setting the uploadPending somewhere between 0 and 100 manually would be a first step?

Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! 🚀

@kulmann kulmann merged commit 4928f82 into owncloud:master May 28, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn leaving users about running uploads
4 participants