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

Cura profile upload works even when no file is selected #1893

Closed
eyal0 opened this issue May 1, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@eyal0
Copy link
Contributor

commented May 1, 2017

What were you doing?

  1. Go to Settings.
  2. Select CuraEngine
  3. Press Import Profile...
  4. Browse for valid profile.
  5. Press Confirm. The profile will be uploaded
  6. Press Import Profile... again. This time, don't select a profile.
  7. Press Confirm. The profile will be uploaded again even though no profile is shown.

This is what the screen looks like right before step 7. The Confirm button on this screen works:
image

What did you expect to happen and what happened instead?

I expected the Confirm button not to work when there is no profile selected.

Branch & Commit or Version of OctoPrint

OctoPrint/devel
OctoPrint 1.3.2.post110+gd1d94624 (HEAD branch)

I have read the FAQ.


I'm working on the solution to this in https://github.com/OctoPrint/OctoPrint-Slic3r . I'll solve it there and then port a similar fix for the cura plugin and send a PR.

@eyal0

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2017

This is the fix that I am porting to cura.js: OctoPrint/OctoPrint-Slic3r@dbd31ed

eyal0 added a commit to eyal0/OctoPrint that referenced this issue May 1, 2017

Clear upload data when Aborting cura profile upload modal.
This fixes foosel#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 added a commit that referenced this issue May 2, 2017

Some refactoring in CuraViewModel
  * Use click data-bind for submit button instead of manual click
    binding
  * Keep metadata synchronized (otherwise changing identifier, name,
    description, overwrite or default flag after adding a file does
    absolutely nothing)
  * Disable submit button and fields when no file is selected.
  * Disable submit button (and show spinned) when request is in
    progress.

Related to #1893
@foosel

This comment has been minimized.

Copy link
Owner

commented May 2, 2017

I merged your PR manually via a cherry-pick, see the above commit. Additionally I did some further refactoring because I noticed that the metadata of an uploaded profile didn't update properly after selecting the file to upload. I also made it so the "Confirm" button is actually disabled until you select a file to upload, and swapped out your manual click binding on the file select element with a knockout data bind (which now is possible thanks to your earlier refactoring).

All in all I think that should be pretty solid now. I briefly looked into completely going away from using the add callback on the file upload element and instead going for only using submit, but I ran into some issues there. Needs another look at a later date.

Anyhow, this is already on maintenance, will soon be synced up to devel as well and be part of the 1.3.3 release.

Thanks!

@foosel foosel added this to the 1.3.3 milestone May 2, 2017

@foosel foosel closed this in f5fad51 May 31, 2017

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.