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

Address Issues 127 & 126 #128

Merged
merged 2 commits into from Oct 20, 2017
Merged

Conversation

yregaieg
Copy link
Collaborator

Issue 127 is actually caused by the fact that old forms were never cleaned up, this PR provide extra cleaning A- Whenever uploader-plus is invoked for an upload B- Between multiple files upload after a metadata form has been processed.

Younes Regaieg added 2 commits October 19, 2017 16:45
@yregaieg
Copy link
Collaborator Author

@douglascrp 32e0842 is meant to address #126 (if my understanding of the issue is correct).
When I first introduced the length restriction for the form, I only tested it with date-formcontrol on the top of the form which contributed to me easily missing the issue.

@yregaieg yregaieg changed the title Address Issue 127 Address Issues 127 & 126 Oct 20, 2017
Copy link
Collaborator

@douglascrp douglascrp left a comment

Choose a reason for hiding this comment

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

I think the code block could be converted into a function into the uploader-plus-mixin.js, and then the 4 copies could be replaced by the function being called.
As the SoftwareLoop.UploaderPlusMixin is merged into the other 3 files, the function would be available for them too.

What do you think?

@yregaieg
Copy link
Collaborator Author

@douglascrp couldn't agree more !

@douglascrp douglascrp merged commit 6c7e2f9 into softwareloop:master Oct 20, 2017
@douglascrp
Copy link
Collaborator

@yregaieg PR merged, but I am going to execute the change I commented before.
Thank you for your help.

@douglascrp
Copy link
Collaborator

@yregaieg Dude, I have just finished the changes we were talking about.
Would you mind trying this new version in your test environment?
You can find my changes at https://github.com/douglascrp/uploader-plus/tree/issue-127-B

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