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

React storage/cancel upload from process file #5179

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

raystorm
Copy link

Description of changes

Adds support for the ability to cancel an Upload from processFile()
by returning a rejected promise.

Issue #, if available

#5099

Description of how you validated changes

  • Added a Unit Test to ensure removeUpload() is called when a rejected promise is returned from processFile().
  • Added a New Example, that forces a Promise.reject() in examples.
    ran the example manually with additional Logging code
    (not committed, and now removed to watch/verify operation.)

Checklist

  • Have read the Pull Request Guidelines
  • PR description included
  • Relevant documentation is changed or added (and PR referenced)
  • yarn test passes and tests are updated/added
  • No side effects or sideEffects field updated

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@raystorm raystorm requested a review from a team as a code owner April 24, 2024 21:22
Copy link

changeset-bot bot commented Apr 24, 2024

🦋 Changeset detected

Latest commit: 92400ec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@aws-amplify/ui-react-storage Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@calebpollman
Copy link
Contributor

calebpollman commented Apr 24, 2024

@raystorm Thanks for opening this! Need to discuss with the team, will update here once we have next steps

@calebpollman
Copy link
Contributor

@raystorm Just wanted to update you here, we are aligned with adding the update to processFile and will be reviewing your PR. Thanks again for contributing!

@calebpollman
Copy link
Contributor

@raystorm We recently made some updates to the StorageManager to support new functionality exposed on the uploadData API (consumed from aws-amplify/storage) used by the component, which has caused some merge conflicts with between main and your PR. Can help out with resolvng the merge conflicts, but wanted to give you the option to handle resolvng them i you prefer

  * move ProcessFile Error/reject handling to `getInput()`
@raystorm
Copy link
Author

@raystorm We recently made some updates to the StorageManager to support new functionality exposed on the uploadData API (consumed from aws-amplify/storage) used by the component, which has caused some merge conflicts with between main and your PR. Can help out with resolvng the merge conflicts, but wanted to give you the option to handle resolvng them i you prefer

Merge Conflicts are resolved. You may want to run the test example from my branch locally, to make sure everything still works properly. The Error Handling code is the same, but moved to getInput() I lost my local setup for running the examples, when I cleaned up some accidentally pushed env changes on my side. So I was unable to re-run the examples locally. as a secondary validation of my changes this time.

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.

None yet

2 participants