ENH: Display a dialog to save trans and fids before quitting the coreg app#10305
ENH: Display a dialog to save trans and fids before quitting the coreg app#10305larsoner merged 41 commits intomne-tools:mainfrom
Conversation
output.mp4 |
|
I will extract the changes made to the GUI API in another PR. I also have to work on a notebook alternative and testing. |
|
If you haven't started, I think it's okay to do the notebook separately. The Qt interface is much more commonly used here |
|
Okay then, let's go with Qt first. |
|
@larsoner I added a |
|
During one interaction I got a segfault caused by: It was for MRI fiducials, and the button was labeled "Don't save". I think "Discard" is more common in UI designs. Also, any Qt event handler should be wrapped with @safe_event so that, even if there are bugs like this, the UI does not segfault. |
|
When I load, lock fids, set to uniform scale mode, then set scaling to 99 instead of 100, I get a box saying that the head<->MRI transform is not saved. This is actually not correct -- it is the head<->MRI trans I loaded, what I didn't save was the "scaled subject MRI". So I think you need to track three states:
Another point -- when the dialog comes up, if I click "Save", it correctly pulls up a save dialog. But if I then hit "cancel" on the system dialog -- and thus don't save anything -- the UI still exists. I think it should only exit if the user actually saved a file. In this sense, the cancel of the system dialog works like the cancel of the previous dialog, and the only way a user can "lose their work" is if they actually click the "Discard" button. |
Oh no, on my system the name is
I see
Makes sense |
Okay so it's a standard (my version of) macOS thing, that's fine -- just update the |
|
Hopefully it's good enough now 🙏 |
8b4d300 to
4d669c6
Compare
larsoner
left a comment
There was a problem hiding this comment.
LGTM +1 for merge when it comes back green
|
I still got: when I tweaked a fiducial, hit the X to close, then hit "Don't save". I pushed a commit to get rid of the assertion |
|
What is the exact value of |
|
I put it in the code commented out |
|
... more specifically: |
While testing https://github.com/hoechenberger/mne-installers on Windows, I ran So it uses "Discard" too! I also ran into other bugs specific to Windows but now I can debug properly and fix them in another PR.
|
Great -- I would say these are high priority since they will probably make the coreg gui unusable on Windows :( It now works for me on macOS, thanks @GuillaumeFavelier ! |
|
I'll work on windows for a while to fix those 👍 |
…the coreg app (mne-tools#10305)" This reverts commit 639d736.

This PR basically rewrites the
closeEvent()function to allow displaying a dialog in the coreg app in case the user did not savetrans.It is still work in progress. In its current state, it works but the code to achieve it needs refactoring. Plus it does not save the fids yet.
Closes #10236
It's an item of #8833