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

core: add postprocess progress when upload success #2535

Merged
merged 2 commits into from
Oct 15, 2020

Conversation

mejiaej
Copy link
Contributor

@mejiaej mejiaej commented Sep 16, 2020

A fix for this issue. I don't know if this is the best way to solve the bug but It works in my case. The postprocess property should be set up if there is any postprocessor, that way the success icon is not going to appear at the end of upload-success. That is possible because there is an existing validation in a dashboard component that checks for that property.

postprocessor2

Question: Are the packages of this PR deployed somewhere? I need to use my forked version but it is becoming complicated since uppy uses mono repo and can't just simple do yarn add https://github.com/myaccount/forked-repo since I only need @uppy/core from my forked version.

@mejiaej mejiaej closed this Sep 16, 2020
@mejiaej mejiaej reopened this Sep 16, 2020
@mejiaej mejiaej force-pushed the master branch 2 times, most recently from 3e8212f to a072d19 Compare September 16, 2020 23:17
@ajkachnic
Copy link
Contributor

ajkachnic commented Sep 17, 2020

Question: Are the packages of this PR deployed somewhere? I need to use my forked version but it is becoming complicated since uppy uses mono repo and can't just simple do yarn add https://github.com/myaccount/forked-repo since I only need @uppy/core from my forked version.

I don't believe they are, but I may be wrong about that. You could try using something like patch-package for the time being

@goto-bus-stop goto-bus-stop self-requested a review September 17, 2020 13:00
Copy link
Contributor

@goto-bus-stop goto-bus-stop left a comment

Choose a reason for hiding this comment

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

Thanks! I didn't think we'd be able to do it cleanly in the current architecture, but this is a nice approach.

packages/@uppy/core/src/index.js Outdated Show resolved Hide resolved
@mejiaej
Copy link
Contributor Author

mejiaej commented Oct 7, 2020

Hi @goto-bus-stop, do you guys think this change will be implemented in an upcoming version?

@arturi arturi merged commit ae4b9df into transloadit:master Oct 15, 2020
@arturi
Copy link
Contributor

arturi commented Oct 15, 2020

Sorry for the delay! Yes, will be out in the next release. We just did one recently, so likely by the end of the month, unless some change will require a quicker turnaround.

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.

4 participants