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

Clear upload data when Aborting cura profile upload modal. #1894

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
2 participants
@eyal0
Copy link
Contributor

commented May 1, 2017

This fixes #1893 .

Also, only bind click to the "Confirm" button once instead of binding and unbinding it for each file. Binding it just once seems cleaner.

What does this PR do and why is it necessary?

Clear the upload data before loading the cura profile modal dialog so that the dialog contents are consistent with the upload data.

How was it tested? How can it be tested by the reviewer?

Follow procedure in the bug.

Any background context you want to provide?

In the bug.

What are the relevant tickets if any?

#1893

Clear upload data when Aborting cura profile upload modal.
This fixes #1893 .

Also, only bind click to the "Confirm" button once instead of binding and unbinding it for each file.  Binding it just once seems cleaner.
@foosel

This comment has been minimized.

Copy link
Owner

commented May 2, 2017

I've cherry picked this as f5fad51 to maintenance and merged there. Since it's a bug fix, it makes sense to push it out with the next release (1.3.3).

Since Github currently doesn't pick up cherry picks properly, I'll now close this manually since it's merged. It won't be marked as merged though, please don't be alarmed by that.

As a side note, I updated the PR guidelines a while ago, PRs for bug fixes may now also be done against maintenance, the bot won't flag that anymore. So feel free to file fixes like that against the maintenance branch instead of devel, makes cherry-picking unnecessary and hence allows Github to pick up on merge state correctly :)

@foosel foosel closed this May 2, 2017

@eyal0

This comment has been minimized.

Copy link
Contributor Author

commented May 2, 2017

Enjoy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.