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

Barbara/update #181

Merged
merged 7 commits into from
Jan 31, 2017
Merged

Barbara/update #181

merged 7 commits into from
Jan 31, 2017

Conversation

bborgesr
Copy link
Contributor

@bborgesr bborgesr commented Jan 30, 2017

This does the following:

  • update to AdminLTE 2.3.8
  • re-apply commit 73f6027 (trigger shown and hidden events for Shiny)
  • re-apply commit e9e63d1 (serve fonts from local host)
  • run grunt

I only re-applied those two commits because those are the only items left in the shiny-mods document.

Here are the two other possible mods that I think are no longer necessary (@wch correct me if I'm wrong):

  • 02dd45b (attach event handler to overall sidebar menu)
  • a31c58d (attach collapse event handler to document)

…e author does not keep things consistent, so we're overwriting his version number on the files to match the version number that the latest download refers to)
@wch
Copy link
Contributor

wch commented Jan 30, 2017

Looking good - add a NEWS item? Also, you can bump the version to 0.5.3.9000.

@bborgesr
Copy link
Contributor Author

bborgesr commented Jan 30, 2017

For convenience, I'm adding the issue-specific test apps here, so we can go about future updates more seamlessly:

For issue #42 (fixed by 73f6027) -- the goal is to make sure that the box shows the plot when it is expanded:

library(shiny)
library(shinydashboard)

ui <- dashboardPage(
  dashboardHeader(),
  dashboardSidebar(
    sidebarMenu(
      menuItem("Inputs", icon = icon("bar-chart-o"), tabName = "tabOne"
      )
    )
  ),
  dashboardBody(
    tabItems(
      tabItem("tabOne",
        box(title = "Test Box One", 
          status = "success", 
          solidHeader = TRUE, 
          collapsible = TRUE,
          collapsed = TRUE,
          plotOutput("plot", height = 250),
          verbatimTextOutput("boxOneText")
        ),
        box(
          actionButton("go", "Go")
        )
      )
    )
  )
)

server <-  function(input, output) {
  output$plot <- renderPlot({
    cat(paste("plotting", input$go, "\n"))
    plot(rnorm(1 + input$go), rnorm(1 + input$go))
  })
  output$boxOneText <- renderText({
    paste("Go counter:", input$go)
  })
}

shinyApp(ui, server)

No test needed for #32 (fixed by e9e63d1), since it just requires the fonts to be hardcoded in the css file, rather than fetched from Google fonts.


For issue #54 (fixed by 02dd45b) -- the goal is to make sure these two scripts produce an identical-looking app (namely, both should show a menuItem that can be expanded to a menuSubItem):

Static UI:

library(shiny)
library(shinydashboard)
sidebar <- dashboardSidebar(
  sidebarMenu(menuItem("foo",
                        menuSubItem("foo_"), tabName = "tabfoo"))
)


ui <- dashboardPage(
  dashboardHeader(),
  sidebar,
  dashboardBody()
)

server <- function(input, output) {}

shinyApp(ui, server)

Dynamic UI:

library(shiny)
library(shinydashboard)
library(shiny)

sidebar <- dashboardSidebar(
  sidebarMenuOutput("sbMenu")
)


ui <- dashboardPage(
  dashboardHeader(),
  sidebar,
  dashboardBody()
)

server <- function(input, output) {
  output$sbMenu <- renderMenu({
    sidebarMenu(menuItem("foo", menuSubItem("foo_"), tabName = "tabfoo"))
    })
}

shinyApp(ui, server)

For issue #17 (fixed by a31c58d) -- the goal is to make sure the box is collapsible:

library(shiny)
body <- dashboardBody(
  uiOutput("ui")
)

server <- function(input, output) {  
  output$ui <- renderUI({
    box(title = "Collapse me",
        status = "warning", solidHeader = TRUE, collapsible = TRUE
    )
  })
}

shinyApp(
  ui = dashboardPage(
    dashboardHeader(),
    dashboardSidebar(),
    body
  ), 
  server = server
)

@bborgesr
Copy link
Contributor Author

Since all three test apps above are working correctly, I think the update is good to go (and in the future, we don't have to worry anymore about commits 02dd45b and a31c58d, since it seems like those problems have now been fixed by Admin LTE itself).

@wch wch merged commit 88eff5a into master Jan 31, 2017
@wch wch deleted the barbara/update branch January 31, 2017 01:53
@wch wch removed the in progress label Jan 31, 2017
dmpe pushed a commit to dmpe/shinydashboard that referenced this pull request Mar 25, 2017
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.

None yet

2 participants