Add option to dismiss dialog #16269
Add option to dismiss dialog #16269
Conversation
|
Thanks for making a pull request to jupyterlab! |
There was a problem hiding this comment.
Thanks for opening this! I added a couple of comments. Also, when I checked out and ran your code, I found that if I stop the server, then see the dialog, then click somewhere on the page not in the dialog's body, the dialog goes away, and I don't see it again, even if I don't have the box checked. Is this something you could fix in this PR?
Co-authored-by: Jason Weill <[email protected]>
Co-authored-by: Jason Weill <[email protected]>
|
Thank you for your review @JasonWeill! OH I think I missed checking for that but I will update this with the appropriate fix for that, thank you for letting me know! |
| const key = 'server-connection-error'; | ||
| const promise = connectionLostCachePromise[key]; |
There was a problem hiding this comment.
After the recent changes, I do not see a reason for this to be a mapping if the key is constant.
| return promise; | ||
| } else { | ||
| try { | ||
| const errorDialogPromise: any = showDialog({ |
There was a problem hiding this comment.
New code should not use any except for tests. any disables type checking and in past lead to numerous user-facing bugs. This will be enforced with #16219
| import { ServerConnection, ServiceManager } from '@jupyterlab/services'; | ||
| import { ITranslator, nullTranslator } from '@jupyterlab/translation'; | ||
| import { IConnectionLost } from './tokens'; | ||
|
|
||
| let displayConnectionLost = true; | ||
| const connectionLostCachePromise: { [key: string]: Promise<any> } = {}; |
There was a problem hiding this comment.
New code should not use any except for tests. Any disables type checking and in past lead to numerous user-facing bugs. This will be enforced with #16219
There was a problem hiding this comment.
Thank you for your review @krassowski I will implement those changes!
Co-authored-by: Michał Krassowski <[email protected]>
References
Related to #8374
Code changes
This PR uses the
showDialogfunction to display an error message with a checkbox giving the user the option to dismiss the popup dialog for the remainder of their tab session.User-facing changes
The user will now see a clickable checkbox that allows them to dismiss the error dialog informing them their connection to the server was lost.
Backwards-incompatible changes
Not applicable