Skip to content

fix(scripts): Reorder bundle so bslib wins cascade ties against shiny.scss#2254

Closed
elnelson575 wants to merge 1 commit into
mainfrom
fix/cascade-order-shiny-before-bslib
Closed

fix(scripts): Reorder bundle so bslib wins cascade ties against shiny.scss#2254
elnelson575 wants to merge 1 commit into
mainfrom
fix/cascade-order-shiny-before-bslib

Conversation

@elnelson575
Copy link
Copy Markdown
Contributor

Summary

Reorder layers in bs_full_theme() so R-shiny's shiny.scss emits before bslib's bs3compat in the compiled bootstrap.min.css. This matches the R-shiny + bslib head order and lets bslib's bs3compat overrides win cascade ties.

bs_bundle() requires theme first, so drop down to sass::sass_bundle() and reapply the bs_theme class (mirroring bs_bundle()'s own implementation).

Why

Inside py-shiny's combined bundle, bslib bs3compat sat at byte ~278k and shiny.scss at ~360k. Any 0-3-1 ↔ 0-3-1 specificity tie went to shiny.scss by source order — the opposite of R-shiny + bslib, where R-shiny loads its own shiny.css <link> before bslib's bundle.

Concrete symptom: bslib PR rstudio/bslib#1308 lifts the BS5 radio/checkbox label-margin rule to .shiny-input-container.shiny-input-{kind} (0-3-1). That beat shiny.scss's base -10px rule (0-2-1) for vertical groups, but tied shiny.scss's .shiny-input-container-inline rule (0-3-1) and lost on source order for inline groups.

Ordering with related PRs

Prerequisite for #2246. The script change must land before make upgrade-html-deps is re-run, otherwise the regenerated bootstrap.min.css keeps the old layer order. Suggested sequence:

  1. Merge this PR
  2. Merge fix: elevate CSS specificity for BS5 radio/checkbox group label margin rstudio/bslib#1308
  3. Re-run make upgrade-html-deps on chore: Upgrade bslib to 0.11.0 and Bootstrap to 5.3.8, add brite theme #2246's branch, then merge chore: Upgrade bslib to 0.11.0 and Bootstrap to 5.3.8, add brite theme #2246

By itself this PR produces no compiled-output diff.

Collateral-damage audit

The only same-specificity, same-element conflict between bs3compat and shiny.scss in BS5 mode is the radio/checkbox label-margin rule. Other bs3compat rules (.well, .help-text, .glyphicon, .navbar-*, .nav-tabs > li, .dropdown-menu > li, pre.shiny-code, .float-left/right, etc.) target selectors shiny.scss does not style. shiny.scss's .radio/.checkbox usages are prefixed with .qt5 (different scope). bs3compat's .radio/.checkbox styling is wrapped in @if \$bootstrap-version == 4 and doesn't compile for BS5.

Net effect: only the inline radio/checkbox gap changes, and it changes to the value bslib PR #1308 intended.

Test plan

….scss

`bs_full_theme()` in `scripts/_functions_sass.R` previously called
`bs_bundle(theme, bslib = ..., shiny = ..., ...)`. bslib's `bs3compat`
SCSS is baked into the `theme` argument, and `bs_bundle()` always
stacks named layers on top of `theme`. This put bslib's overrides
*before* R-shiny's `shiny.scss` rules in the compiled CSS, so any
same-specificity tie went to shiny.scss by source order — the
opposite of an R-shiny + bslib app, where R-shiny's `shiny.css` is
loaded as its own `<link>` before bslib's bundle.

The visible symptom: bslib PR #1308 (which lifts the specificity of
the BS5 radio/checkbox label-margin rule to `.shiny-input-container
.shiny-input-{kind}`) fixed vertical groups in py-shiny but inline
groups still got the `-1px` from shiny.scss's
`.shiny-input-{kind}.shiny-input-container-inline` rule, which ties
at 0-3-1 and was emitted later in the file.

Fix: drop down from `bs_bundle()` to `sass::sass_bundle()` so the
theme can appear after a user layer, list `shiny` first and `theme`
second, then reapply `class(theme)` to keep the bs_theme identity
(mirrors `bs_bundle()`'s own implementation).

The only same-specificity, same-element conflict between bs3compat
and shiny.scss in BS5 mode is the radio/checkbox label-margin rule;
no other styles change behavior under the reorder.

Depends on rstudio/bslib#1308 for the visible fix.
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.

1 participant