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

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

Open
stanwest opened this issue Jun 11, 2024 · 7 comments · May be fixed by #7430
Open

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

stanwest opened this issue Jun 11, 2024 · 7 comments · May be fixed by #7430

Comments

@stanwest
Copy link

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.

@stanwest stanwest added enhancement status:Needs Triage Applied to issues that need triage labels Jun 11, 2024
@Anakintano
Copy link

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
Copy link
Collaborator

RRosio commented Jun 18, 2024

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
Copy link
Author

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
Copy link

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
Copy link
Member

jtpio commented Jul 18, 2024

@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 #7384.

Thanks!

@sai-chand04
Copy link

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 sai-chand04 linked a pull request Jul 25, 2024 that will close this issue
@sai-chand04
Copy link

sai-chand04 commented Jul 25, 2024

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:
#7430

@JasonWeill JasonWeill linked a pull request Jul 25, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants