-
Notifications
You must be signed in to change notification settings - Fork 1.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
[poc] embed html widgets in the console #12048
Conversation
This is still very rough, but if we get this to work, then it's just a matter of writing some |
1237a9d
to
5c5d29f
Compare
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.
Overall LGTM!
One open question: do these widgets need to be persisted in some way in the console history, or do we need any special handling there? This is mainly relevant to RStudio Server, where we need to persist the console history for the user in case they reload the browser (or close and then later reconnect).
It's probably fine if we don't do anything here but worth testing to make sure that these widgets don't break anything (just in case).
LOG_ERROR(error); | ||
|
||
// if it's in the temp dir then we can serve it via the help server, | ||
// otherwise we need to show it in an external browser |
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.
Is this comment still true?
if (isHTMLWidgetPath(filePath)) | ||
{ | ||
// view it | ||
consoleShowWidget(module_context::sessionTempDirUrl(path), height); |
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.
Nit: if there's only one "target" code path you could probably reduce the indentation in this function with some early returns for cases where validity conditions aren't satisfied.
src/gwt/src/org/rstudio/studio/client/common/shell/ShellWidget.java
Outdated
Show resolved
Hide resolved
src/gwt/src/org/rstudio/studio/client/workbench/views/console/shell/Shell.java
Outdated
Show resolved
Hide resolved
6a7276b
to
f45f3b4
Compare
f45f3b4
to
f50a5b4
Compare
👀 |
b8e32ce
to
46e89a0
Compare
76ac021
to
fed915e
Compare
d6c86e0
to
77ef58d
Compare
Co-authored-by: Kevin Ushey <kevin@rstudio.com>
Co-authored-by: Kevin Ushey <kevin@rstudio.com>
…`<iframe>` height (300)
fed915e
to
0687502
Compare
shelving this for now to declutter the pull requests, we may come back to this later, so keeping the branch just in case. |
Intent
display html widgets directly in the console. This may be useful for e.g. error messages, e.g. r-lib/rlang#1440
Approach
.rs.api.console_viewer()
similar to.rs.api.viewer()
(for widgets) but the rsult goes i the console:Automated Tests
QA Notes
Checklist
NEWS.md