Page movement when spinner is hidden

[antpingelli]

Hello, I am seeing some page movement when the spinner is hidden and the page is scrolled to a certain location as seen below:

I think this is coming from the fact that the spinner is just hidden and not actually removed https://github.com/daattali/shinycssloaders/blob/ccb6d503c61d22ea69bea9e7475bc6e6f554fd32/inst/assets/spinner.css#L21 Is it possible to change it to display: none, or some other way to stop the spinner from being active on the page?

[daattali]

I haven't seen this issue before, can you show some small piece of code that shows the problem?

[antpingelli]

Hey Dean, thanks for responding so quickly. I was able to replicate it by adding more graphs to the example shiny application created using RStudio IDE. If I scroll down so the title of the third graph is at the top of my screen I see the same movement.

library(shiny)

ui <- fluidPage(

    # Application title
    titlePanel("Old Faithful Geyser Data"),

    # Sidebar with a slider input for number of bins 
    sidebarLayout(
        sidebarPanel(
            sliderInput("bins",
                        "Number of bins:",
                        min = 1,
                        max = 50,
                        value = 30)
        ),

        # Show a plot of the generated distribution
        mainPanel(
            shinycssloaders::withSpinner(plotOutput("distPlot1")),
            shinycssloaders::withSpinner(plotOutput("distPlot2")),
            shinycssloaders::withSpinner(plotOutput("distPlot3")),
            shinycssloaders::withSpinner(plotOutput("distPlot4")),
            shinycssloaders::withSpinner(plotOutput("distPlot5"))
        )
    )
)

# Define server logic required to draw a histogram
server <- function(input, output) {

    output$distPlot1 <- renderPlot({
        # generate bins based on input$bins from ui.R
        x    <- faithful[, 2]
        bins <- seq(min(x), max(x), length.out = input$bins + 1)

        # draw the histogram with the specified number of bins
        hist(x, breaks = bins, col = 'darkgray', border = 'white')
    })

    output$distPlot2 <- renderPlot({
        # generate bins based on input$bins from ui.R
        x    <- faithful[, 2]
        bins <- seq(min(x), max(x), length.out = input$bins + 1)

        # draw the histogram with the specified number of bins
        hist(x, breaks = bins, col = 'darkgray', border = 'white')
    })

    output$distPlot3 <- renderPlot({
        # generate bins based on input$bins from ui.R
        x    <- faithful[, 2]
        bins <- seq(min(x), max(x), length.out = input$bins + 1)

        # draw the histogram with the specified number of bins
        hist(x, breaks = bins, col = 'darkgray', border = 'white')
    })

    output$distPlot4 <- renderPlot({
        # generate bins based on input$bins from ui.R
        x    <- faithful[, 2]
        bins <- seq(min(x), max(x), length.out = input$bins + 1)

        # draw the histogram with the specified number of bins
        hist(x, breaks = bins, col = 'darkgray', border = 'white')
    })

    output$distPlot5 <- renderPlot({
        # generate bins based on input$bins from ui.R
        x    <- faithful[, 2]
        bins <- seq(min(x), max(x), length.out = input$bins + 1)

        # draw the histogram with the specified number of bins
        hist(x, breaks = bins, col = 'darkgray', border = 'white')
    })
}

# Run the application 
shinyApp(ui = ui, server = server)

[daattali]

Thanks, I was able to reproduce. After some digging around, it looks like a potential fix is to set the hidden spinner to display:none. But I'm hesitant to add that CSS rule too quickly because that may have other side effects - I'm not sure if it was hidden using visibility: hidden instead of display:none for any good reason or not. It would require a lot of testing

[andrewsali]

I think I switched to visibility instead of display: none because there were some issues with IE or old Edge, can't remember which one (if I recall correctly the loading animations didn't restart after removing display: none)

In 2020 this might not be relevant anymore.

On Wed, Aug 26, 2020, 04:49 Dean Attali notifications@github.com wrote:

Thanks, I was able to reproduce. After some digging around, it looks like a potential fix is to set the hidden spinner to display:none. But I'm hesitant to add that CSS rule too quickly because that may have other side effects - I'm not sure if it was hidden using visibility: hidden instead of display:none for any good reason or not. It would require a lot of testing

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/daattali/shinycssloaders/issues/54#issuecomment-680445052, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADTBRJ4L5BD5WWSCGRFMHMTSCRZ5DANCNFSM4QHTKCTQ .

[daattali]

Aw you still follow the issues! Thanks for the heads up. I'd still like to do enough testing before making that change, but if it really is just old IE that breaks, I would be fine with that as long as you don't have objections

[moa-incom]

I'm having the same problem in the Chrome browser here in November/December with all updated browser, R, and R packages. A fix sometime in the future would be nice. Thank you!

[andersonyyc]

A fix that worked for me without needing to change the visibility behaviour was to set a height to the loader class when using the default spinner.type. The other spinner types do not appear to have this issue.

Using the example app with some text added for scrolling length to recreate the issue:

The root cause seems to be that the div for the default spinner type does not have a minimum size and changes based on the current size of the animation's central bar:

When hidden:

Adding a style to the loader css class such as .loader { height: 40px; } keeps the loader div the same height throughout the loading animation and removed the page movement.

Of course, the height may need to vary if the default option for spinner.size has been changed. Perhaps this could be achieved similarly to how the font-size is calculated in the withSpinner function?

After adding the height: style:

As far as I can tell from some testing and running this fix on a number of apps, this does not have any unintended consequences when using the default spinner.type. Please let me know if I'm missing a painfully obvious reason not to do this!

[daattali]

Thanks for the suggestion @andersonyyc . I can't immediately think of any reason why adding a height would be bad. I would think that a safer option is to add min-height rather than height.

Is that 40px coming from somewhere specific, is it explicitly being set to 40 somewhere, or is it based on font size or something else? Or is it just what you observe in practice?

Did you check that using the proxy.height parameter isn't affected?

[andersonyyc]

@daattali Thanks for the quick reply! Fair call on using a min-height instead of just height to avoid issues. The 40px came from the max size of the animation with the default spinner.size. Using size = 2 requires the height to be 80px and so on.

The proxy.height parameter still works as expected from the testing I did.

Looking at this a bit closer, I think adding a min-height to the .load1 class that scales with the spinner.size would be a better solution. What I proposed above clips the animation slightly (didn't notice this until now) and technically applies to all the spinner types when it's only needed on spinner.type one.

I'm going to test this out over the next few days and hopefully provide a more complete fix.

[daattali]

@antpingelli feel free to send a PR that changes from visibility to display. It'll have to go through a lot of testing but if it seems to work then it'll solve the issue

[daattali]

Unfortunately after some rigorous testing, I found that changing the css from visibility: hidden to display: none works in many cases, but in some of the more complex apps it doesn't work - the spinner just never goes away. If I find more time to debug I'll try to see why that is, but for now the problem of wobbling sometimes is preferable than the problem of the spinner not working sometimes, so I won't be making that change.

[daattali]

Hi @antpingelli @mortandersen @andersonyyc I just submitted a commit that I hope fixes the wobble issue without creating any new issues. Could you all please (a) install the latest version (remotes::install_github("daattali/shinycssloaders")), (b) verify that your issue is solved, (c) test any other apps you have that use this package to make sure it didn't break anything else?

[daattali]

It seems that @phoebee-h verified in https://github.com/RinteRface/bs4Dash/issues/245 and @mvarewyck verified in https://github.com/inbo/reporting-rshiny-grofwildjacht/issues/172 that this fix works. Unless someone still sees this issue persisting, I will consider this fixed