jupyter / notebook

Jupyter Interactive Notebook
https://jupyter-notebook.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
11.5k stars 4.8k forks source link

The "Close and Shut Down Notebook" menu command discards state without confirmation #7398

Open stanwest opened 2 months ago

stanwest commented 2 months ago

Problem

When I have a notebook open and select "Close and Shut Down Notebook" from the "File" menu of the notebook interface, notebook (as of v. 7.3.0a0) closes the kernel unconditionally, discarding the computational state. I recommend that the interface confirm that the user wants to close and shut down, as JupyterLab does (at least in v. 4.3.0a0) when one selects its "Close and Shut Down Notebook…" menu item.

Proposed Solution

Restore the confirmation dialog and add an ellipsis to the end of the text in the menu item.

Additional context

This behavior seems to have arisen in #7384.

Anakintano commented 2 months ago

Hi @stanwest . I am new to open-source and want to contribute to this issue . @stanwest I came across this code snippet for close and shut down -> execute: async () => { // Shut the kernel down, without confirmation await commands.execute('notebook:shutdown-kernel', { activate: false }); window.close(); } in the packages/notebook-extension/src/index.ts path . To this problem

const confirmation = confirm('Are you sure you want to close the notebook and shut down the kernel?'); if (confirmation) { await commands.execute('notebook:shutdown-kernel', { activate: false }); window.close(); } would this be a right approach ? thoughI have not tested it yet .

I would highly appreciate your input,

Thanks .

RRosio commented 2 months ago

From Triage: The reason why @JasonWeill implemented this as is was for Notebook 6 parity. If you'd like to have this behavior, it would need to be implemented as a configuration option.

stanwest commented 1 month ago

The reason why @JasonWeill implemented this as is was for Notebook 6 parity.

Thank you for triaging this and commenting. I understand and agree that parity has some weight, and yet I believe it would be much better for Notebook to ask the user if they really do want to lose state.

If you'd like to have this behavior, it would need to be implemented as a configuration option.

I don't follow that it must be that way, but if it were an option, I would recommend that the default be to ask for confirmation. Users who want no confirmation as in v. 6 could set the option accordingly.

sai-chand04 commented 1 month ago

Hi @jtpio, i would like to contribute to this issue and plan to start working on it. Please let me know if there are any specific guidelines or things i should be aware of.

jtpio commented 1 month ago

@sai-chand04 here are the contributing guidelines: https://github.com/jupyter/notebook/blob/main/CONTRIBUTING.md

Please feel free to open a PR when you have something, even if it's not fully finished and still a draft.

Looks like the idea in this idea would be to implement a setting to allow for showing the dialog and allow for the behavior before https://github.com/jupyter/notebook/pull/7384.

Thanks!

sai-chand04 commented 4 weeks ago

Thank you for the guidance! I have started working on allowing the dialog to show and restoring the behavior before #7384. I will open a PR soon, even if it's still in draft form, for initial feedback.

Thanks!

sai-chand04 commented 3 weeks ago

Hi @jtpio I have created a PR for this issue. The PR includes the changes for the restoring the close and shutdown confirmation dialog as of now. Could you please take a look and review the changes? Your feedback would be greatly appreciated. Thanks in advance for your time and guidance!

PR link: https://github.com/jupyter/notebook/pull/7430