rstudio / bslib

Tools for theming Shiny and R Markdown via Bootstrap 3, 4, or 5.
https://rstudio.github.io/bslib/
Other
443 stars 50 forks source link

Popover title isn't getting removed after being added #1011

Closed cpsievert closed 3 months ago

cpsievert commented 4 months ago

Do the following in the following to repro the issue:

  1. Click "Show popover"
  2. Click "Hide popover"
  3. Click "Show popover"
  4. Switch on "Add a popover title"
  5. Switch off "Add a popover title"

Step 5 should remove the title from popover, but it doesn't. This issue was surfaced after a recent fix in #747 by an existing popover test https://rstudio.github.io/shinycoreci/results/2024/03/15/#316-bslib-popovers-macOS-4.3

library(shiny)
library(bslib)
library(plotly)

ui <- page_sidebar(
  title = "Popover tests",
  card(
    card_header(
        popover(
          span(
            "Card title with popover",
            bsicons::bs_icon("question-circle-fill")
          ),
          "Popover message",
          id = "popover",
          placement = "right"
        )
    ),
    plotlyOutput("bars")
  ),
  sidebar = list(
    textInput("popover_msg", "Enter a popover message", "Popover message"),
    actionButton("show_popover", "Show popover", class = "mb-3"),
    actionButton("hide_popover", "Hide popover"),
    br(),
    input_switch("show_title", "Add a popover title"),
    conditionalPanel(
      "input.show_title",
      textInput("popover_title", "Enter a title", "Popover title"),
    )
  )
)

server <- function(input, output, session) {
  observe({
    update_popover(
      "popover", input$popover_msg,
      title = if (input$show_title) input$popover_title
    )
  })

  observeEvent(input$show_popover, {
    toggle_popover("popover", show = TRUE)
  })

  observeEvent(input$hide_popover, {
    toggle_popover("popover", show = FALSE)
  })

  output$bars <- renderPlotly({
    plot_ly(diamonds, x = ~cut)
  })
}

shinyApp(ui, server)
cpsievert commented 3 months ago

This particular issue is actually user error: update_popover(title = NULL) shouldn't remove the existing title.

Looking into this made realize we have some other issues, through which I'll write up in a PR.

github-actions[bot] commented 1 month ago

This issue has been automatically locked. If you have found a related problem, please open a new issue (with a reproducible example or feature request) and link to this issue. :raising_hand: Need help? Connect with us on Discord or Posit Community.