-
Notifications
You must be signed in to change notification settings - Fork 357
Use <pre>
tag for error messages in showErrorMessage
dialog
#1410
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
Conversation
Could a maintainer please approve running the required workflows for this PR? |
src/notifications.ts
Outdated
showErrorMessage( | ||
trans.__('Error'), | ||
{ | ||
message: React.createElement( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you rename the file to notifications.tsx
, add import * as React from 'react';
import and then use jsx syntax in here? I think no other file in this repo uses createElement
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviewing. Done in the latest revision. All CI checks have passed. Can this be merged now?
Thanks again!
...to preserve line breaks and use a monospace font so e.g. pre-commit errors are readable. Partially addresses jupyterlab#1407.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, looks better than before, thank you @ctcjab!
<pre>
tag for error messages in showErrorMessage
dialog
Thanks for merging @krassowski! Would it be possible to cut a new patch release with this improvement? Looks like |
I was thinking that if #1408 becomes ready to merge soon, we could release these together (I see there is an active discussion in comments). If it does not settle in a week or so, I can cut a patch release as-is. |
I believe #1408 would benefit from a 3rd opinion. I'm for whatever gets the job done. If my suggestions are getting in a way of having good solution merged, then I'm ready to take them back. |
...to preserve line breaks and use a monospace font so e.g. pre-commit errors are readable.
Partially addresses #1407.