rstudio / shiny

Easy interactive web applications with R
https://shiny.posit.co/
Other
5.35k stars 1.87k forks source link

Slider handle is in wrong location when used with Quarto #3905

Closed wch closed 11 months ago

wch commented 11 months ago

When using the sliderInput with a different build of Bootstrap (compared to the bootstrap.css in bslib), the slider looks like this:

slider

This is because the CSS for the handle position is in bslib's copy of bootstrap.css:

slider css

This happens when using the slider in a Quarto document.

To fix this, I think the slider styling in bslib somehow needs to be put into Shiny.

cpsievert commented 11 months ago

Are you bringing the slider in via PyShiny (a reprex would be great)?

wch commented 11 months ago

Right, the slider is coming in from Shiny for Python. Sorry, I don't have a simple reprex at this point, since generating the output currently involves some things that are experimental.

I also tried reproducing with Quarto and Shiny for R, but I couldn't figure out how to get it to display sliders with the updated look (with no tick marks). If you can get Quarto and Shiny for R to display the new sliders, I suspect they will look the same.

wch commented 11 months ago

I'm not sure if this is exactly the same issue, but it seems like it's probably related: https://github.com/rstudio/bslib/issues/813

gadenbuie commented 11 months ago

@wch I suspect the issue is fundamentally the same for Python and R -- the static, precompiled CSS for ionrangeslider not using the Sass variables of the new shiny preset -- but the fix will likely be different. In the case of Shiny for Python, I'm not sure how ionrangeslider's CSS dependency is handled, but I suspect the fix will be to re-compile that CSS in the shiny preset context.

gadenbuie commented 11 months ago

I think we should move this issue into the py-shiny repo (or close it in favor of a new issue); on the R side the work needed is covered by the bslib issue.

wch commented 11 months ago

Just to make sure we're on the same page: The slider looks fine in a regular Shiny for Python app, but not when the slider from py-shiny is used in a Quarto doc.

Shiny for Python has a build of bootstrap.min.css which includes the CSS in the screenshot above, for .irs.irs--shiny .irs-handle.

In the Quarto case, the problem is that Quarto's build of bootstrap.min.css is taken from an older version of bslib, and does not have that CSS. As far as I know, there's not a way to cleanly override Quarto's version of Bootstrap. Note that Quarto will load the ion.RangeSlider HTML dependency, so the resulting document uses Shiny's version of ion.RangeSlider.css, with Quarto's version of bootstrap.min.css.

I think Shiny for Python currently simply copies over the CSS files from Shiny for R. If I run a simple example app in R, the browser inspector shows that there is some slider styling is in bootstrap.min.css.

image


And here it is with the .irs.irs--shiny .irs-handle rule in bootstrap.min.css disabled:

image

App code:

library(shiny)
library(bslib)

ui <- fluidPage(
  theme = bs_theme(preset = "shiny"),
  sliderInput("n", "N:", min = 0, max = 1000, value = 500)
)

shinyApp(ui, function(input, output) {})
gadenbuie commented 11 months ago

In the Quarto case, the problem is that Quarto's build of bootstrap.min.css is taken from an older version of bslib, and does not have that CSS. As far as I know, there's not a way to cleanly override Quarto's version of Bootstrap.

What's additionally confusing is that these bootstrap.min.css files are not equivalent. It's not that Quarto takes an older bootstrap.min.css from bslib, but rather they took the base Bootstrap layers that contain some direct patches that bslib applies to the Bootstrap source. As a result, Quarto's version doesn't include any of the rules that might get layered on top in R via a Sass compilation step.

In Shiny for Python, the bootstrap.min.css is compiled via bslib and may include even more than bslib would include in "bootstrap" because in bslib and R we can compile the Sass on the fly, whereas in Python these rules need to ship as compiled.

At this point, I think it'd be worth the effort to put together a reprex.

jcheng5 commented 11 months ago

I'd really really like us to move away from this model (that I invented, IIRC?) where the sass rules are layered in, and instead have everything that builds on top of Bootstrap use CSS variables exclusively. How far do you think we are from that reality?

Until we do that, we'll never having theming in Shiny for Python, and Quarto will always act weird like this when we put our components in it.

cpsievert commented 11 months ago

How far do you think we are from that reality?

We're not that far off, and we've made significant strides recently with the bslib components.

instead have everything that builds on top of Bootstrap use CSS variables exclusively.

I doubt this alone would be sufficient for solving the core issue here, which as @wch pointed out, seems to be related to how Quarto is managing Bootstrap. After discussing with Charles today, we talked about some of these issues, and at least for now, I'll be sending them a PR so that Quarto can better simulate bslib, which may end up fixing this issue.

Longer term, it would be great to eliminate Quarto's build-time step of copying over bslib's Sass code. I don't know if this is realistic, but I'm thinking Quarto could maybe do something like library(bslib); bs_theme() |> bs_bundle(user_sass) |> bs_theme_dependencies() via webR (and maybe, for py-shiny, could have a web service to do this?)

gadenbuie commented 11 months ago

After researching how py-shiny handles the ionRangSlider dependency, I have a much clearer understanding of this issue. py-shiny pre-renders the static ionRangeSlider.css using the preset = "shiny" theme (see here). That compiled CSS file isn't fully stand-alone, as we have additional rules in the shiny preset that end up in bootstrap.min.css.

So one possibility, for py-shiny at least, would be to include those rules in the pre-compiled ionRangeSlider.css file. Alternatively, it might be better for py-shiny to serve the standard Bootstrap styles for the ionRangeSlider when in a Quarto doc or when paired with an unknown Bootstrap.

Unfortunately, CSS variables wouldn't completely save us from this problem (even though I agree we should move in that direction with our existing components too), because we're missing an entire small stylesheet.

I do think we should at least open a py-shiny specific issue since the dynamics of the situation are different in Python and R and will need different fixes (unless we decide to make Bootstrap 5 and the Shiny preset the de facto default Shiny app look).

wch commented 11 months ago

Closing in favor of https://github.com/posit-dev/py-shiny/issues/748