-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix display of stack traces within RStudio's viewer pane #996
Conversation
dash-renderer/src/components/error/FrontEnd/FrontEndError.react.js
Outdated
Show resolved
Hide resolved
style={{ | ||
width: 'calc(600px - 67px)', | ||
height: '75vh', | ||
border: 'none', |
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.
Do we actually need any of these style attrs?
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 kept them because we used them previously, but it might be ok to 🔪 from both R and Python. The important bits are in the FrontEndError.css
file anyhow.
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.
The <iframe>
needs explicit sizing because the containing page doesn't know what's in it, and border because perhaps iframes have a border by default. But the unframed string version should be closer to the front-end version, which gets most of its styling from .dash-fe-error__info
.
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.
fixed in bf08944
…t.js Co-Authored-By: alexcjohnson <johnson.alex.c@gmail.com>
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.
Looks great! 💃
This PR proposes to modify
dash-renderer
to check whether the stack trace being sent up by the backend includes the string<!DOCTYPE HTML
. If it does, the content is embedded within aniframe
to avoid CSS leaking caused by the Werkzeug HTML which Dash for Python provides.If it does not, the content is included in a simple nested
div
instead, with customized CSS to properly line-wrap the preformatted content, and to automatically use a scrollbar if the stack trace is sufficiently long.After merging, the stack trace within the dev tools UI for Dash for R apps will look like this:
To be merged concurrently with plotly/dashR#137. Fixes #994.