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

GCODE Scripts changes not saved if moving to another settings screen #824

Closed
eboston opened this issue Mar 25, 2015 · 9 comments
Closed
Labels
needs information More information is needed to further process this issue or PR

Comments

@eboston
Copy link

eboston commented Mar 25, 2015

  1. What were you doing?
    Setting GCODE Scripts.
  2. What did you expect to happen?
    Changes to the GCODE Scripts to be saved if moved to another Settings screen.
  3. What happened instead?
    Changes were lost with no warning they would not be saved.
  4. Branch & Commit or Version of OctoPrint:
    Version: 1.2.0-dev-668-g3ebd54d (devel branch)
  5. Printer model & used firmware incl. version
    (if applicable - always include if unsure):
    Printrbot Simple Metal
  6. Browser and Version of Browser, Operating
    System running Browser (if applicable - always
    include if unsure):
    Chrome v41.0.2272.101 m
  7. Link to octoprint.log on gist.github.com or pastebin.com:
    https://gist.github.com/10750a14d53beec5b38e.git
  8. Link to contents of terminal tab or serial.log on
    gist.github.com or pastebin.com (if applicable - always
    include if unsure or reporting communication issues):
  9. Link to contents of Javascript console in the browser
    on gist.github.com or pastebin.com or alternatively a
    screenshot (if applicable - always include if unsure
    or reporting UI issues):

n/a

  1. Screenshot(s) showing the problem (if applicable - always
    include if unsure or reporting UI issues):

n/a

I have read the FAQ.

If I click the save button, it does save the changes, but closes the settings window. This is annoying when you have several changes to make. Adding an APPLY button would resolve the issue. If changing to another settings screens and there are unsaved changes, a warning message could be displayed.

@foosel
Copy link
Member

foosel commented May 4, 2015

I'm honestly not seeing the problem here. You say when you click Save the changes do get applied. If you have to make multiple changes, just make multiple changes and save them all together at the end? Or am I missing something here?

@foosel foosel added the needs information More information is needed to further process this issue or PR label May 4, 2015
@eboston
Copy link
Author

eboston commented May 4, 2015

Make a change on one screen, but don't click save. Move to another screen, then back to the original one you made changes to. Changes are gone.

@markwal
Copy link
Member

markwal commented May 4, 2015

Steps:

  1. Settings->GCODE scripts
  2. Type something one of the boxes like "M70 P1" in the After print job completes box
  3. Click "temperatures"
  4. Click "GCODE scripts"

Expected:
The thing you typed in step 2 to be in the box where you typed it.

Result:
It's gone

Why is this the expected result?
So the user can change some gcode scripts, some temperatures, revisit some gcode scripts, change some plugin settings and then hit Save and have them all go at once.

The steps I gave repro for me on devel: fcdb556

@foosel
Copy link
Member

foosel commented May 6, 2015

Please tell me what I'm doing wrong: Video

@eboston
Copy link
Author

eboston commented May 6, 2015

Just checked this out on the latest devel branch and it seems to be keeping the changes when switching screens.

@markwal
Copy link
Member

markwal commented May 6, 2015

Ha ha. It's fixed by my pull request 3e5298f.

git checkout e5de000
reload page
repros

git checkout devel
reload page
no repro

I'm guessing because that change stopped the "show" propagation and somewhere in there something refreshed the settings data from the server and now it doesn't.

@markwal
Copy link
Member

markwal commented May 6, 2015

Yep. That's it. Just verified in the debugger. In settings.js, self.onSettingsShown used to get called on every pane change and refresh the data from the server. After 3e5298f it doesn't.

@foosel
Copy link
Member

foosel commented May 6, 2015 via email

@foosel
Copy link
Member

foosel commented May 7, 2015

Ok, also updated the Changelog, closing this

@foosel foosel closed this as completed May 7, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs information More information is needed to further process this issue or PR
Projects
None yet
Development

No branches or pull requests

3 participants