Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.8k
Error background configurable from rendermime settings#18219
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Thanks for making a pull request to jupyterlab! |
3f8bd42 to7bf76ffCompare| "title": "RenderMime", | ||
| "description": "RenderMime settings.", | ||
| "jupyter.lab.setting-icon": "ui-components:text-editor", | ||
| "jupyter.lab.setting-icon-label": "RenderMime", |
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.
I understand that the rationale for adding it inrendermime-extensions is that--jp-rendermime-error-background is used inrendermime stylesheet. There are some reasons not to favour this:
- we need another settings tab and users may struggle to find this setting
- we need to choose icon and title for this settings tab, which are not obviously easily understood (MIME was replaced by Media types in standards, MIME is formal capitalization), the icon for it is not obvious.
- users get another single-setting (ok, two-setting) tab
And advantage of NOT adding it notebook settings is that it allows to apply the setting to both Notebook and Console.
What if instead we added this to Theme settings? After all,--jp-rendermime-error-background values are defined in individual themes. Also, it already has some CSS overrides:
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.
we need to choose icon and title for this settings tab, which are not obviously easily understood (MIME was replaced by Media types in standards, MIME is formal capitalization), the icon for it is not obvious.
I asked myself the same, and decide to not add one 😄
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.
It looks fine to add it to the theme manager plugin 👍
| "errorBackgroundLight": { | ||
| "title": "Error Background Color (Light Theme)", | ||
| "description": "Background color for error output in light themes", | ||
| "type": "string", | ||
| "default": "#fdd" | ||
| }, | ||
| "errorBackgroundDark": { | ||
| "title": "Error Background Color (Dark Theme)", | ||
| "description": "Background color for error output in dark themes", | ||
| "type": "string", | ||
| "default": "#600" | ||
| } |
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.
This could be nested into
"errorBackground": { "title": "Error Background Color" "type": 'object" "properties": { "light": ... "dark": ... }}The advantage is that it will render nicely grouped for the user.
This PR allows configuring the background color for stderr (cell output error).
output.webm
References
Fixes#7892
Code changes
Add a settings schema for the rendermime-extension plugin, with options to set the color in light and dark mode.
User-facing changes
The background color of the error if configured.
Backwards-incompatible changes
None