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

don't cancel all files when clicking "done" #4771

Merged
merged 4 commits into from
Nov 7, 2023
Merged

Conversation

mifi
Copy link
Contributor

@mifi mifi commented Oct 30, 2023

calling cancelAll also causes any active assembly to be cancelled (when waitForEncoding: false). Doesn't make sense to me that assembly should be cancelled when a user clicks the "Done" button after an upload has completed. Also it doesn't make sense to cancel uploads when clicking "done"

The button was introduced in #2653

@arturi
Copy link
Contributor

arturi commented Oct 31, 2023

When Dashboard is using with an inline mode, we can't just hide modal when “done” is pressed — the state will stay the same. And even in modal, after it’s called back, the “complete” or “error” state will still be there. I’ve pushed a commit to reset state to default, but we should actually implement an uppy.reset() method in Core, as I’ve suggested before.

Comment on lines 70 to 76
this.uppy.setState({
totalProgress: 0,
error: null,
recoveredState: null,
allowNewUpload: true,
files: {},
})
Copy link
Member

Choose a reason for hiding this comment

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

Note that this is practically identical to resetProgress with the only difference that files aren't removed. Should indeed be streamlined.

resetProgress () {
const defaultProgress = {
percentage: 0,
bytesUploaded: 0,
uploadComplete: false,
uploadStarted: null,
}
const files = { ...this.getState().files }
const updatedFiles = {}
Object.keys(files).forEach(fileID => {
updatedFiles[fileID] = {
...files[fileID],
progress: {
...files[fileID].progress, ...defaultProgress,
},
}
})
this.setState({
files: updatedFiles,
totalProgress: 0,
allowNewUpload: true,
error: null,
recoveredState: null,
})
this.emit('reset-progress')
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes i also suggested that during the call, but the reasoning was that it's very slow because it iterates over all files. but in this case we don't need to do that because all files will be removed anyways. but yea we should probably reuse parts of that function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pushed some code reuse

@abdirahmn1
Copy link

To anyone having a problem with this bug at the moment., use the following workaround till this becomes available. Set waitForEncoding: true on Transloadit options.

const uppy = new Uppy(<uppy_options>).use(TransloadIt, {
    assemblyOptions: {
      params: {
        auth: { key: "your transloadit key"},
        template_id: "your transloadit template id",
      },
    },
    waitForEncoding: true,
  });

The done button that would cancel the assembly would not appear until the assembly is completed.

Co-authored-by: Artur Paikin <artur@arturpaikin.com>
@arturi arturi merged commit d72a937 into main Nov 7, 2023
13 checks passed
@arturi arturi deleted the dont-cancel-assembly branch November 7, 2023 22:08
@github-actions github-actions bot mentioned this pull request Nov 8, 2023
github-actions bot added a commit that referenced this pull request Nov 8, 2023
| Package                | Version | Package                | Version |
| ---------------------- | ------- | ---------------------- | ------- |
| @uppy/aws-s3           |   3.5.0 | @uppy/provider-views   |   3.7.0 |
| @uppy/aws-s3-multipart |   3.9.0 | @uppy/react            |   3.2.0 |
| @uppy/companion        |  4.11.0 | @uppy/transloadit      |   3.4.0 |
| @uppy/companion-client |   3.6.0 | @uppy/tus              |   3.4.0 |
| @uppy/core             |   3.7.0 | @uppy/url              |   3.4.0 |
| @uppy/dashboard        |   3.7.0 | @uppy/utils            |   5.6.0 |
| @uppy/image-editor     |   2.3.0 | @uppy/xhr-upload       |   3.5.0 |
| @uppy/locales          |   3.4.0 | uppy                   |  3.19.0 |

- @uppy/dashboard: Remove uppy-Dashboard-isFixed when uppy.close() is invoked (Artur Paikin / #4775)
- @uppy/core,@uppy/dashboard: don't cancel all files when clicking "done" (Mikael Finstad / #4771)
- @uppy/utils: refactor to TS (Antoine du Hamel / #4699)
- @uppy/locales: locales: add ca_ES (ordago / #4772)
- @uppy/companion: Companion+client stability fixes, error handling and retry (Mikael Finstad / #4734)
- @uppy/companion: add getBucket metadata argument (Mikael Finstad / #4770)
- @uppy/core: simplify types with class generic (JokcyLou / #4761)
- @uppy/image-editor: More image editor improvements (Evgenia Karunus / #4676)
- @uppy/react: add useUppyState (Merlijn Vos / #4711)
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

4 participants