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

feat(fill): Use a @layer htmltools #425

Merged
merged 3 commits into from
Mar 19, 2024
Merged

feat(fill): Use a @layer htmltools #425

merged 3 commits into from
Mar 19, 2024

Conversation

gadenbuie
Copy link
Member

@gadenbuie gadenbuie commented Mar 17, 2024

This PR wraps the fill.css rules in a @layer htmltools.

In bslib we have the following CSS rule using the .bslib-flow-mobile class:

@include media-breakpoint-down(sm) {
  .bslib-flow-mobile > .html-fill-item {
    flex: 0 0 auto;
  }
}

This rule is intended to undo the fill.css rule

.html-fill-container > .html-fill-item {
  flex: 1 1 auto;
}

but it has a specificity problem where whether or not it is applied as expected depends on the order in which dependencies are loaded.

In fact, it appears that currently .bslib-flow-mobile is applied correctly in Shiny for Python and not in Shiny for R where page_fillable() appears to be always filling on mobile with dev bslib (we started including the component CSS in bs_theme_dependency() in rstudio/bslib#810).

One obvious fix is to increase the specificity of the .bslib-flow-mobile rule (and we may still want to do this to avoid version coordination between bslib and htmltools).

@include media-breakpoint-down(sm) {
  .bslib-flow-mobile.html-fill-container > .html-fill-item {
    flex: 0 0 auto;
  }
}

But we generally intend for fill.css to be applied at a lower level than other CSS rules. We've also talked about adopting CSS layers in our dependencies. Fortunately, we can start using a layer for fill.css without having to completely work out how we'll use layers overall.

I'm proposing we use the package name, @layer htmltools, for this layer, rather than trying to guess what functional-oriented name we might want in the long run (e.g. utilities or layout, etc.). We also don't need to declare layer order elsewhere, simply using a layer is enough to at least reduce the importance of these rules. At some point we may decide to have bslib coordinate some of the layer ordering, but we don't need to do that now.

@@ -1,8 +1,12 @@
# htmltools (development version)

## Improvements
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went back and forth on putting this in "possible breaking changes". It's more likely this change will un-break unintentionally broken styles and less likely to break working styles, so I settled on calling it an "improvement".

@gadenbuie gadenbuie merged commit f8e0591 into main Mar 19, 2024
15 checks passed
@gadenbuie gadenbuie deleted the use-css-layer branch March 19, 2024 14:36
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.

2 participants