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

Enable usage with shiny modules #107

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

KaniaKrzysztof
Copy link

Hello, first of all thank you for such a great work creating ComplexHeatmap and InteractiveComplexHeatmap packages. They are really great tools. My heart was broken once I found out InteractiveComplexHeatmap doesn't work with shiny modules. That's a big loss for shiny community because the modules are the foundation for any shiny app that is bigger than some plot demo. I've accepted the challenge of fixing it and here is what I found out.

It's not a secret that shiny works with some ids to bind ui and server. If you work directly in shiny's ui and server element's id equals it's id in html. Things are clear and easy so far. Once we introduce modules namspaces come into play. If we take a look into ui side of simple app with numeric input and text output that displays the number from the input It'd look like this:

moduleTestUI <- function(id) {
  ns <- NS(id)
  tagList(
    numericInput(ns("number_in"), "test", 1),
    textOutput(ns("text_out"))
  )
}

ui <- fluidPage(
  moduleTestUI("module_prefix")
)

What changed is that now we wrap every shiny id within module with ns() function that basically takes the given module prefix (in our case module_prefix) and prepends it to given id making sure the elements have the unique id. In our case that would be: module_prefix-number_in and module_prefix-text_out. If we would nest some modules the prefixes from each module will carry on with each step e.g. module1-module2-module_prefix-number_in etc.

If we now take a look into server side:

moduleTestServer <- function(id) {
  moduleServer(id, function(input, output, session) {
      output$text_out <- renderText({
        input$number_in
      })
    }
  )
}
server <- function(input, output, session) {
  moduleTestServer("module_prefix")
}

We create module server with the same module prefix but inside module server function we don't have to deal with namespace at all. We directly refer to input and output elements via its non-prefixed id e.g. input$number_in. It's beacause Shiny does some shiny magic to make modules transparent to each other and allow you to not care about namespace at all. It'll behave like that no matter how many modules we nest.

That was the issue I identified that prevented InteractiveComplexHeatmap from working with shiny modules. Package referred to inputs and outputs via it's full id which works only when things are not module prefixed. I've noticed 2 cases where you have to replace whole html id with non-prefixed heatmap id:

  • Whenever you access input and output objects
  • Whenever you use functions like updateNumericInput and similiar where you provide session element as parameter

To get this non-prefixed heatmap id I've added this helper:

remove_module_prefix = function(heatmap_id, session) {
  prefix = session$ns("")

  if(nchar(prefix) == 0) {
    heatmap_id
  } else {
    substring(heatmap_id, nchar(session$ns("")) + 1)
  }
}

It leverages the fact that if we supply empty string to ns() function we will know what the prefix is and with that we can easily subtract it from our full id.

To make this code to work I've had to revert your changes with heatmap_id validation. I've seen the issue 105. The default concatenation character for shiny modules is -. I know you can change it but it's quite widely used and changing that especially in some bigger apps can be quite challengeing. I think the line that causes you trouble with dash character is this one. There's no need for making this variable name prefixed because it's only avaliable in its bracket scope. I fixed that in this PR too.

I've replaced the validation function with assertion so you can throw errors when heatmap_id is not valid i.e. starts with digits. I'd strongly suggest to not modify the ids directly because it creates confusion and makes thing harder to debug. We should make users life easier but there's a line ;)

Last but not least I've changed the way js and css templates are loaded. I've run into really strange issue on my windows machine where the javascript is loaded before the html. That caused Pickr.js to load before required div ('@{heatmap_id}_tabs-search') was loaded. I've moved it right into that div to make sure its loading in the same time (I've seen you had same approach for the other outputs).

Here's also an example app that I used to battle test my changes:

library(shiny)

m = matrix(rnorm(100), 10)
ht = Heatmap(m)

moduleTestUI <- function(id) {
  ns <- NS(id)
  tagList(
    InteractiveComplexHeatmapOutput(ns("ht_id"))
  )
}

moduleTestServer <- function(id) {
  moduleServer(id, function(input, output, session) {
      makeInteractiveComplexHeatmap(input, output, session, ht, session$ns("ht_id"))
    }
  )
}


ui = fluidPage(
  moduleTestUI("mod_prefix")
)

server = function(input, output, session) {
  moduleTestServer("mod_prefix")
}

shiny::shinyApp(ui, server)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant