Skip to content
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

HTML ui #45

Closed
emillykkejensen opened this issue Mar 1, 2017 · 11 comments
Closed

HTML ui #45

emillykkejensen opened this issue Mar 1, 2017 · 11 comments

Comments

@emillykkejensen
Copy link

Hi

I was wondering, what I need to do for ECharts2Shiny to work with HTML ui?

@XD-DENG
Copy link
Owner

XD-DENG commented Mar 1, 2017

Hi, thanks for approaching.

May you share more about what you mean by "HTML ui"? Thanks.

@emillykkejensen
Copy link
Author

Well I allready found an solution :-)

With Shiny UI:

App.R:

library(shiny)
library(ECharts2Shiny)
server <- function(input, output) {
  renderWordcloud(div_id = "test", data = c('A', 'A', 'A', 'B', 'B', 'C', 'A'))
}
ui <- fluidPage(
  loadEChartsLibrary(),
  div(id="test", style="width:100%;height:500px;"),
  deliverChart(div_id = "test"),
  title = "Shiny UI version"
)
shinyApp(ui = ui, server = server)

With HTML UI:

Server.R:

library(shiny)
library(ECharts2Shiny)
server <- function(input, output) {
  renderWordcloud(div_id = "test", data = c('A', 'A', 'A', 'B', 'B', 'C', 'A'))
}

www/index.html

<!DOCTYPE html>
<html>
<head>

<meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
<meta name="viewport" content="width=device-width, initial-scale=1" />

<title>HTML version</title>

<link href="shared/shiny.css" rel="stylesheet" />
<link href="shared/bootstrap/css/bootstrap.min.css" rel="stylesheet" />

<script type="application/shiny-singletons"></script> 
<script src="shared/json2-min.js"></script>
<script src="shared/jquery.min.js"></script>
<script src="shared/babel-polyfill.min.js"></script>
<script src="shared/shiny.min.js"></script>

<script src="shared/bootstrap/js/bootstrap.min.js"></script>
<script src="shared/bootstrap/shim/html5shiv.min.js"></script>
<script src="shared/bootstrap/shim/respond.min.js"></script>
 
<script src="echarts.js"></script>

</head>

<body>
  <div class="container-fluid">
    <div id="test" style="width:100%;height:500px;"></div>
    <div id="test" class="shiny-html-output"></div>
  </div>
</body>

</html>

and then add echarts.js to the www/ library

This works, and shows ECharts2Shiny with HTML ui.

However I would like it to not wrap it in 'fluidPage' in the renderWordcloud function - line 124 in word_cloud.R. Generally I think, it would be better to just return the 'js_statement' and then aks the users to enclose it into a renderUI function - like:

output$wordcloud <- renderUI({
  renderWordcloud(data = c('A', 'A', 'A', 'B', 'B', 'C', 'A'))
})

@XD-DENG
Copy link
Owner

XD-DENG commented Mar 1, 2017

Hi @emillykkejensen , thanks so much for exploring and share!

I had the current design as I thought it may be good to include output$wordcloud <- renderUI({ in the function itself so that the users don't have to type it explicitly repeatedly. Given this package has been published for some time and is being used by some people (actually not many LOL), so it may be hard to change the API and the relevant design.

But do let me know if you have any other good suggestions! I would be more than happy to refine it with your input.

@emillykkejensen
Copy link
Author

Okay - here's an idea:

  1. Add: 'wrap_in_renderUI = TRUE' to the functions arguments
  2. Replace:
to_eval <- paste("output$", div_id ," <- renderUI({fluidPage(tags$script(\"",
                     js_statement,
                     "\"))})",
                     sep="")

(line 124-127)
with:

if(wrap_in_renderUI){
    to_eval <- paste("output$", div_id ," <- renderUI({fluidPage(tags$script(\"",
                     js_statement,
                     "\"))})",
                     sep="")
  } else {
    to_eval <- paste("tags$script(\"", js_statement, "\")")
  }

just tried it locally and it work perfect :)

@XD-DENG
Copy link
Owner

XD-DENG commented Mar 1, 2017

Then how should we set the default value for wrap_in_renderUI? If the default value if "FALSE", it will break the current APE; If it's "TRUE", then users always need to specify wrap_in_renderUI = FALSE if they want to type "output$..." by themselves, which may be a burden as well.

If there is no predominant advantage, I may prefer to keep current design.

@emillykkejensen
Copy link
Author

It not a buden to write 'wrap_in_renderUI = FALSE' - it's a burden if one can't use ECharts2Shiny with HTML ui. By defaulting wrap_in_renderUI to TRUE, no change is made to the users currently using the function - but by including this extra argument, more people are able to use it.

There really is no downside to it and there is a major advantage with it - the posibility to use ECharts2Shiny with HTML ui or with other Shiny ui's not based on the fluid layout (eg. fixed layout, flex layout, split layout etc. that do not use fluidPage()) :-)

@XD-DENG
Copy link
Owner

XD-DENG commented Mar 1, 2017

I understand the point you're trying to make now: enabling ECharts2Shiny with HTML ui (www/index.html) etc.

Kindly allow me some time to check the code again and see how we can move forward with the inspiration of your idea. Will get back to you on this for sure ;-)

@emillykkejensen
Copy link
Author

Sounds good :-)

XD-DENG added a commit that referenced this issue Mar 2, 2017
This is based on the issue #45.
@XD-DENG
Copy link
Owner

XD-DENG commented Mar 2, 2017

Hi @emillykkejensen , I have checked the codes and please check the findings as below ;-)

I must admit I've made a "mistake" in statement

to_eval <- paste("output$", div_id ," <- renderUI({fluidPage(tags$script(\"",
                   js_statement,
                   "\"))})",
                   sep="")

Actually we don't need to include fluidPage at all. The statement without it like below will definitely work.

to_eval <- paste("output$", div_id ," <- renderUI({tags$script(\"",
                   js_statement,
                   "\")})",
                   sep="")

I've made this change in branch dev_2, commit f7ecc1f, to all the relevant .R files in /R folder.

Then let's go back to your question: how we can make this package work better with HTML ui. Actually with the current codes, you can already use different layouts (fixed layout, flex layout, split layout etc, as you pointed out). The command above will render the <script></script> in the final applicaiton, what we need to do is only to prepare the corresponding <div>. We had the confusion only because I made the stupid mistake to have included fluidPage in the command above while we don't need it actually.

Hence your question is also solved: we are able to adopt diverse layouts while we don't need to make big change. We don't need to bother to add the argument wrap_in_renderUI either.

Please let me know your thoughts.

Thanks again for reaching out so that I can find the mistake I made.

@emillykkejensen
Copy link
Author

That could work. However I would like to stick to shiny's intended way of doing things, where you write the output to the server.R like: output$wordcloud <- renderUI(renderWordcloud(data = c('A', 'A', 'A', 'B', 'B', 'C', 'A')))

The main reason, is that I'm doing some work on my data before I put it into the renderWordcloud function, and I would like to do that work inside a reactive expression.

@XD-DENG
Copy link
Owner

XD-DENG commented Mar 17, 2017

Hi @emillykkejensen , for this request, I still think the current design is "good enough". My mainconcern is that it may increase the complexity if we try to have a reactive expression and do data manipulation inside it. As you may have found, the current design is quite straightforward :"translate" R data object into Javascript statements. I think it's better not to touch it.

I hope you can kindly understand this.

I'm not sure about your specific need but possibly you may want to refer to this example example-5 Use Reactive Values.

@XD-DENG XD-DENG closed this as completed Jul 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants