-
Notifications
You must be signed in to change notification settings - Fork 12
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
Support HTML in React context #66
Conversation
013ce7f
to
655b456
Compare
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more. @@ Coverage Diff @@
## main #66 +/- ##
==========================================
+ Coverage 9.72% 72.29% +62.57%
==========================================
Files 5 5
Lines 144 148 +4
==========================================
+ Hits 14 107 +93
+ Misses 130 41 -89
|
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.
Cool new feature for {shiny.react}
🎸
I have 2 comments on the PR.
- NAMESPACE is missing the exported function and is not detecting
html
classes inasReactData
method, defaulting toasReactData.default
- needs
devtools::document()
- needs
- The
isHTML(str)
is too broad and is parsing text that contains a tag in it.- The function and call can probably be removed as it works without them
- 🟡 Unless I'm missing a required feature that will parse any string that contains a html tag
- The function and call can probably be removed as it works without them
Reproducible example for testing my comments on 2.
pkgload::load_all()
library(shiny)
library(shiny.react)
shinyApp(
ui = tagList(
ReactContext(
HTML(
"<div style='font-weight: bold;'>Hello <span style='font-weight: normal;'>my friend</span></div>"
)
),
ReactContext(
"<div style='font-weight: bold;'>Hello (just plain text)</div>"
),
reactOutput("react_output")
),
server = function(input, output) {
output$react_output <- renderReact({
HTML(
"<div style='font-weight: bold;'>Hello (renderReact)</div>"
)
})
}
)
Screenshot of what I understand to be the expected result
Thanks @averissimo, that was great review. I resolved your comments and added a few |
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.
LGTM!! 💯
Great that you added the test!
Sorry for the delay, I thought I already approved this ages ago.
Closes #13
Test app
@kamilzyla Are there more cases which we should support?