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

observeEvent triggered as many times as page_change #71

Closed
andbamp opened this issue Aug 26, 2020 · 4 comments · Fixed by #72
Closed

observeEvent triggered as many times as page_change #71

andbamp opened this issue Aug 26, 2020 · 4 comments · Fixed by #72

Comments

@andbamp
Copy link

andbamp commented Aug 26, 2020

Hello, thanks for your useful package!

I have run into an issue when it comes to changing pages. Here is a minimal example:

library(shiny)
library(shiny.router)

# This generates menu in user interface with links.
menu <- (
  tags$ul(
    tags$li(a(class = "item", href = route_link("home"), "Home page")),
    tags$li(a(class = "item", href = route_link("side"), "Side page"))
  )
)

# This creates UI for each page.
page <- function(title, content, button_name) {
  div(
    menu,
    titlePanel(title),
    p(content),
    actionButton("switch_page", "Click to switch page!"),
    actionButton(button_name, "Go!")
  )
}

# Both sample pages.
home_page <- page("Home page", uiOutput("current_page"), "test1")
side_page <- page("Side page", uiOutput("current_page"), "test2")

home_server <- function(input, output, session) {
  observeEvent(input$test1, {
    print("Foo")
  })
}

side_server <- function(input, output, session) {
  observeEvent(input$test2, {
    print("Bar")
  })
}

# Creates router. We provide routing path, a UI as
# well as a server-side callback for each page.
router <- make_router(
  route("home", ui = home_page, server = home_server),
  route("side", ui = side_page, server = side_server)
)

# Create output for our router in main UI of Shiny app.
ui <- shinyUI(fluidPage(
  router_ui()
))

# Plug router into Shiny server.
server <- shinyServer(function(input, output, session) {
  router(input, output, session)
  
  output$current_page <- renderText({
    page <- get_page(session)
    sprintf("Welcome on %s page!", page)
  })
  
  observeEvent(input$switch_page, {
    if (is_page("home")) {
      change_page("side")
    } else if (is_page("side")) {
      change_page("home")
    }
  })
})

# Run server in a standard way.
shinyApp(ui, server)

Expected behaviour:

  • Clicking the "Click to switch page!" button switches from Home page to Side page and vice versa.
  • Clicking the "Go!" button in the Home page prints "Foo", while clicking it in the Side pages prints "Bar" once.

Actual behaviour:

  • "Click to switch page!" works as intended.
  • The number of times either "Foo" or "Bar" is printed when clicking "Go!" depends on the number of times the "Click to switch page!" button has been clicked. Specifically:
    • On initial load, "Foo" is printed twice.
    • On initial page switch, "Bar" is printed once.
    • For every additional page switch, "Foo" or "Bar" is printed an addition time.

This seems to be related to issue #67. Any feedback is appreciated!

@krystian8207
Copy link
Contributor

krystian8207 commented Aug 26, 2020

@andbamp This is the behavior that happens when you use observers inside of the page. This is because shiny doesn't destroy observers when the server code is called for the n-th time but registers them again (please check my blogpost that describes the issue: https://appsilon.com/how-to-safely-remove-a-dynamic-shiny-module/).

Unfortunately in shiny.router, the main functionality is based on rerendering the page server code. We've got a few ideas how to implement this properly but this is of a low priority now, but I'll let you know when it's done.

In the meantime, you may try to force destroying observers (following instructions from the above blogpost, my solution in #69 might also help) everytime the page is switched. Please let me know if you need any help with it.

@krystian8207
Copy link
Contributor

@andbamp I decided to give my idea a try, and the issue should be fixed now. Please install the package from https://github.com/Appsilon/shiny.router/tree/krystian.stop-rerendering-whole-page branch and let me know if everything works correctly now.

@andbamp
Copy link
Author

andbamp commented Aug 27, 2020

I didn't realise shiny.router's functionality was based on rerunning the server code. This fix not only solves the issue of console output being printed multiple times (and cases such as dialogs instantiated by shinyjs appearing multiple times), but it also negates the need to set ignoreInit to TRUE to accommodate page switching. More importantly, it should also help with performance on certain applications with expensive computations within reactives.

I tested the new branch in a few scenarios and it seems to work well, but one side-effect I noticed is that the UI elements defined in shiny_ui are always rendered last. A modification of the previous example showcases the issue:

library(shiny)
library(shiny.router)

# This generates menu in user interface with links.
menu <- (
  tags$ul(
    tags$li(a(class = "item", href = route_link("home"), "Home page")),
    tags$li(a(class = "item", href = route_link("side"), "Side page"))
  )
)

# This creates UI for each page.
page <- function(title, content, button_name) {
  div(
    menu,
    titlePanel(title),
    p(content),
    actionButton("switch_page", "Click to switch page!"),
    actionButton(button_name, "Go!")
  )
}

# Both sample pages.
home_page <- page("Home page", uiOutput("current_page"), "test1")
side_page <- page("Side page", uiOutput("current_page"), "test2")

home_server <- function(input, output, session) {
  observeEvent(input$test1, {
    print("Foo")
  })
}

side_server <- function(input, output, session) {
  observeEvent(input$test2, {
    print("Bar")
  })
}

# Creates router. We provide routing path, a UI as
# well as a server-side callback for each page.
router <- make_router(
  route("home", ui = home_page, server = home_server),
  route("side", ui = side_page, server = side_server)
)

# Create output for our router in main UI of Shiny app.
ui <- shinyUI(fluidPage(
  actionButton("before_router", "Before router UI"),
  router_ui(),
  actionButton("after_router", "After router UI")
))

# Plug router into Shiny server.
server <- shinyServer(function(input, output, session) {
  router(input, output, session)
  
  output$current_page <- renderText({
    page <- get_page(session)
    sprintf("Welcome on %s page!", page)
  })
  
  observeEvent(input$switch_page, {
    if (is_page("home")) {
      change_page("side")
    } else if (is_page("side")) {
      change_page("home")
    }
  })
})

# Run server in a standard way.
shinyApp(ui, server)

Thanks for addressing the issue so fast @krystian8207! Being able to use shiny.router without modifications to the main application code (be it ignoreInit or assigning observers to variables in order to destroy them) would make the package incredibly versatile.

@krystian8207
Copy link
Contributor

@andbamp Thank you for noticing that. The current branch version should fix this behavior as well.

@dokato dokato closed this as completed in #72 Sep 6, 2020
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 a pull request may close this issue.

2 participants