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

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
Member

@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
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.

Warn leaving users about running uploads
4 participants