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.

Sign up for GitHub

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

Jump to bottom

Standardize dialogs to shut down all kernels #16624

Closed
joaopalmeiro opened this issue Jul 29, 2024 · 1 comment · Fixed by #16629
Closed

Standardize dialogs to shut down all kernels #16624

joaopalmeiro opened this issue Jul 29, 2024 · 1 comment · Fixed by #16629
Labels
enhancement

Comments

@joaopalmeiro
Copy link
Contributor

Hi! 👋

Problem

The dialogs to confirm or cancel the action of shutting down all kernels are different depending on whether this action is triggered from the sidebar or the command palette:

Screenshot 2024-07-29 at 17 59 32 Screenshot 2024-07-29 at 17 59 42

The body text and the cancel button label are not standardized.

Proposed Solution

Adopt the body text and the cancel button label of the dialog when the action of shutting down all kernels is triggered from the sidebar:

jupyterlab/packages/running-extension/src/kernels.tsx

Lines 88 to 97 in 78cd6b6

shutdownAll: () => kernels.shutdownAll(),
refreshRunning: () =>
Promise.all([kernels.refreshRunning(), sessions.refreshRunning()]),
runningChanged,
shutdownLabel: trans.__('Shut Down Kernel'),
shutdownAllLabel: trans.__('Shut Down All'),
shutdownAllConfirmationText: trans.__(
'Are you sure you want to permanently shut down all running kernels?'
)
});

... in the equivalent dialog when the same action is triggered from the command palette:

jupyterlab/packages/mainmenu-extension/src/index.ts

Lines 535 to 555 in 78cd6b6

commands.addCommand(CommandIDs.shutdownAllKernels, {
label: trans.__('Shut Down All Kernels…'),
isEnabled: () => {
return !app.serviceManager.sessions.running().next().done;
},
execute: () => {
return showDialog({
title: trans.__('Shut Down All?'),
body: trans.__('Shut down all kernels?'),
buttons: [
Dialog.cancelButton({ label: trans.__('Dismiss') }),
Dialog.warnButton({ label: trans.__('Shut Down All') })
]
}).then(result => {
if (result.button.accept) {
return app.serviceManager.sessions.shutdownAll();
}
});
}
});
}

Additional context

  • Operating System and version: macOS Sonoma 14.5
  • Browser and version: Chrome 127.0.6533.72
  • JupyterLab version: 4.3.0a2
  • Python version: 3.10.13

Let me know what you think and if I can open a PR. Thanks!
@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to new issues that need triage label Jul 29, 2024
@krassowski
Copy link
Member

Good catch! I think it's pretty clear cut that standardising it would be welcome. As to wording I wonder if it should mention the number of kernels that will be shut down.

@JasonWeill JasonWeill removed the status:Needs Triage Applied to new issues that need triage label Jul 30, 2024
@JasonWeill JasonWeill mentioned this issue Jul 30, 2024
Open
@joaopalmeiro joaopalmeiro mentioned this issue Jul 31, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Standardize dialogs to shut down all kernels joaopalmeiro/jupyterlab
3 participants
@krassowski @joaopalmeiro @JasonWeill