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

Add option to dismiss dialog #16269

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 39 additions & 2 deletions packages/application/src/connectionlost.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// Copyright (c) Jupyter Development Team.
// Distributed under the terms of the Modified BSD License.

import { showErrorMessage } from '@jupyterlab/apputils';
import { Dialog, showDialog } from '@jupyterlab/apputils';
import { ServerConnection, ServiceManager } from '@jupyterlab/services';
import { ITranslator, nullTranslator } from '@jupyterlab/translation';
import { IConnectionLost } from './tokens';

let displayConnectionLost = true;
let serverConnectionLost: Promise<void | Dialog.IResult<string>> | undefined;
Comment on lines +9 to +10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just stylistic, but I would suggest to wrap this in a Private namespace and move to the end of the file. See how this is done on in the clipboard package for example:

/**
* The namespace for module private data.
*/
namespace Private {
/**
* The application clipboard instance.
*/
export let instance = new MimeData();
}

This will makes it clear these variables are private, and also allow you to write a structured docstring for each of them :)


/**
* A default connection lost handler, which brings up an error dialog.
*/
Expand All @@ -22,6 +25,40 @@ export const ConnectionLost: IConnectionLost = async function (
'JupyterLab will continue trying to reconnect.\n' +
'Check your network connection or Jupyter server configuration.\n'
);
const buttons = [Dialog.okButton({ label: trans.__('Dismiss') })];

if (!displayConnectionLost) {
return;
}

if (serverConnectionLost) {
// Wait for the pre-existing promise to complete
await serverConnectionLost;
return;
}

return showErrorMessage(title, { message: networkMsg });
serverConnectionLost = showDialog({
title: title,
body: networkMsg,
checkbox: {
label: trans.__('Do not show this message again in this session.'),
caption: trans.__(
'If checked, you will not see a dialog informing you about an issue with server connection in the future.'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'If checked, you will not see a dialog informing you about an issue with server connection in the future.'
'If checked, you will not see a dialog informing you about an issue with server connection in this session.'

)
},
buttons: buttons
})
.then(result => {
if (result.isChecked) {
displayConnectionLost = false;
}
return;
})
.catch(error => {
console.error('An error occurred while showing the dialog: ', error);
})
.finally(() => {
serverConnectionLost = undefined;
});
return;
};