-
Notifications
You must be signed in to change notification settings - Fork 9
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
Disable the confirmation dialog for static pages #133
Comments
Hi @danielhollas, thanks for bringing this out. I also once took some time looking at this and I think we should not turn the notification off for all notebooks. I think maybe in the jupyterkernel can introduce a rule to recognise for some specific names of the "created on the fly" notebook from appmode and only for these notebooks we turn off the popped out dialog. |
Hm, not sure if messing with jupyterkernel is a good idea. I think the logical place to put this is Appmode, if we want to disable it for all apps. For now as a stop gap solution, I would at least add the JS snippet to the Home app, because that is the most annoying one with which the user always interacts, and the pop-up is sooo annoying every time I want to close the tab. |
Couldn't agree more. Please go ahead with the home app. If that works well we can move on and try more static notebooks. |
I just want to make you aware that this dialogue was added to the app mode on purpose. Appmode would simply not close the notebook leaving users with many notebooks running in the background. Here is the context: oschuett/appmode#51 |
@yakutovicha thanks for the context! Yep, I saw the code in the Appmode. Fortunately, the approach in this PR does not interfere with that. To clarify, the dialog itself does not come from Appmode, but from Jupyter notebook, see in the new Docker stack file window.onbeforeunload = function () {
.
.
.
} This is the function that we want to (conditionally) override. In the appmode, the event listener is registered via So this change should be safe and I tested that the kernel gets cleaned / killed even without the dialog. |
Currently, whenever one tries to navigate away from an appmode app, (e.g. by clicking a URL link, or closing a tab), a confirmation dialog always pops up, even without any real interaction with the page. This is especially annoying for the home app. The confirmation dialog comes from the jupyter notebook, because Appmode always copies the notebook and executes it, which leaves it in an "unsaved" state from the jupyter notebook point of view. Here we simply override the 'onbefereunload' event handler that is defined. For now this fix is only applied to the home page. If there are no issues, we could fix this globally by implementing the fix directly in the Appmode itself, specifically here: https://github.com/oschuett/appmode/blob/3998dc1a6d61f06581fe818351e70860c26c442a/appmode/static/main.js#L136 Note that in the past, the `onbeforeunload` event used to be overrided in Appmode, but later the handler was moved to the `unload` event, which exposed the confirmation dialog. See oschuett/appmode@8665aa6 The unload event handler in Appmode takes care of the cleanup, i.e. shutting down the kernel. Since we're modifying a different event, we should be safe (and I have tested that sessions get cleaned properly). See #133 for further technical details and discussion.
Currently, whenever one tries to navigate away from an appmode app, (e.g. by clicking a URL link, or closing a tab), a confirmation dialog always pops up, even without any real interaction with the page. This is especially annoying for the home app. The confirmation dialog comes from the jupyter notebook, because Appmode always copies the notebook and executes it, which leaves it in an "unsaved" state from the jupyter notebook point of view. Here we simply override the 'onbefereunload' event handler that is defined. For now this fix is only applied to the home page. If there are no issues, we could fix this globally by implementing the fix directly in the Appmode itself, specifically here: https://github.com/oschuett/appmode/blob/3998dc1a6d61f06581fe818351e70860c26c442a/appmode/static/main.js#L136 Note that in the past, the `onbeforeunload` event used to be overrided in Appmode, but later the handler was moved to the `unload` event, which exposed the confirmation dialog. See oschuett/appmode@8665aa6 The unload event handler in Appmode takes care of the cleanup, i.e. shutting down the kernel. Since we're modifying a different event, we should be safe (and I have tested that sessions get cleaned properly). See #133 for further technical details and discussion.
The change is merged for the home page. I'll keep this issue open to remind us to implement this properly later in the Appmode. |
Currently, whenever you try to navigate away from an appmode app (e.g. by clicking a URL link, or closing a tab), I almost always get a confirmation dialog, even if I didn't really interact with the page. This gets especially annoying for the home app (e.g. every time I want to close it, I need to confirm).
I would propose to disable this dialog for pages that are static, such as the home page, appstore and perhaps others.
I've tested that the following code in a jupyter code cell achieves the effect.
The original dialog seems to come from jupyter notebook, see file
/opt/conda/lib/python3.9/site-packages/notebook/static/notebook/js/notebook.js
I've verified that even with the override, the python kernel gets killed when the app window is closed. It seems that this is handled by the appmode via the event listener here:
https://github.com/oschuett/appmode/blob/master/appmode/static/main.js#L26
Of course this would need to be tested more carefully.
@unkcpz @yakutovicha LMK your thoughts on this, happy to take this on if you agree, once we're done with the current changes.
The text was updated successfully, but these errors were encountered: