quarto-dev / quarto-cli

Open-source scientific and technical publishing system built on Pandoc.
https://quarto.org
Other
3.77k stars 309 forks source link

Quarto dashboard value boxes doesn't adhere to theme changes #9453

Closed AlbertRapp closed 4 months ago

AlbertRapp commented 5 months ago

Bug description

It seems like the value boxes do not understand it when a theme preset like flatly is modified with an additional scss file. In the example below, you will see that on top of the flatly theme I add a scss file that overwrites the danger color to green (obviously a bad danger color but it's just for demo purposes).

Using elements with .bg-danger class manually uses this new green color but using color = "danger" in the list() to create a value box uses the original flatly danger color. Commenting out the flatly theme does the correct thing, though.

Steps to reproduce

---
format: dashboard
theme: 
  - flatly # comment out for correct behavior
  - quarto_dash_reprex.scss
---

### Row 

::: {.bg-danger}

This is green and uses the specified danger color in the scss file.

:::

```{r}
#| content: valuebox
#| title: "This uses the original flatly danger color"
list(
  icon = "trash",
  color = "danger",
  value = '24334'
)

And the content of the scss file:

````scss
/*-- scss:defaults --*/
$danger: #009E73 !default;

Expected behavior

image

Actual behavior

image

Your environment

Quarto check output

Quarto 1.4.526
[✓] Checking versions of quarto binary dependencies...
      Pandoc version 3.1.9: OK
      Dart Sass version 1.69.5: OK
      Deno version 1.37.2: OK
[✓] Checking versions of quarto dependencies......OK
[✓] Checking Quarto installation......OK
      Version: 1.4.526
      Path: /opt/quarto/bin

[✓] Checking tools....................OK
      TinyTeX: v2024.03
      Chromium: (not installed)

[✓] Checking LaTeX....................OK
      Using: TinyTex
      Path: /home/albert/.TinyTeX/bin/x86_64-linux
      Version: 2023

[✓] Checking basic markdown render....OK

[✓] Checking Python 3 installation....OK
      Version: 3.11.5 (Conda)
      Path: /home/albert/anaconda3/bin/python
      Jupyter: 5.3.0
      Kernels: python3

[✓] Checking Jupyter engine render....OK

[✓] Checking R installation...........OK
      Version: 4.3.3
      Path: /usr/lib/R
      LibPaths:
        - /home/albert/R/x86_64-pc-linux-gnu-library/4.3
        - /usr/local/lib/R/site-library
        - /usr/lib/R/site-library
        - /usr/lib/R/library
      knitr: 1.45
      rmarkdown: 2.25

[✓] Checking Knitr engine render......OK
mcanouil commented 5 months ago

Sam as your other issue:

Could you try with at least the latest stable version? You are a little bit behind, i.e., the current is 1.4.553 (1.4.526 was a pre-release from last December). 1.4 stable was released on the 15th of February (1.4.550)

(In current 1.5 pre-release, there are no changes made regarding Dashboard.)

AlbertRapp commented 5 months ago

Oh my bad I thought my system auto-updates. In any case I tried this now with v.1.4.554 and the issue still persists.

gadenbuie commented 5 months ago

It turns out Quarto doesn't use the original flatly danger color for value boxes with color = "danger". If you comment out the custom Sass setting $danger, you get this result (flatly's original, redder, danger color on the left; Quarto's custom danger color for value boxes on the left):

image

What's happening

This gives a hint at what's happening: Quarto overrides the default colors for value boxes of certain Bootswatch themes, one of which is flatly. In the value box Sass Quarto sets $value-box-bg-{color} variables, which take from ${color} variables. I'll switch to showing the code for $primary because it's easier to include relevant context, but the process below applies to all theme colors.

https://github.com/quarto-dev/quarto-cli/blob/466d959674507930685b2c977e878495932152ba/src/resources/formats/dashboard/quarto-dashboard.scss#L342-L345

It then creates an array of value box colors

https://github.com/quarto-dev/quarto-cli/blob/466d959674507930685b2c977e878495932152ba/src/resources/formats/dashboard/quarto-dashboard.scss#L354-L360

Where "primary" in $valuebox-colors is set to theme-override-value("flatly", "valuebox-bg-primary", $valuebox-bg-primary). The theme-override-value() function is defined in _bootstrap-functions.scss:

https://github.com/quarto-dev/quarto-cli/blob/466d959674507930685b2c977e878495932152ba/src/resources/formats/html/bootstrap/_bootstrap-functions.scss#L72-L75

The goal of this function is to use the theme override value for {color} if one exists for the theme instead of $valuebox-bg-{color}.

The end result is that both $primary and $valuebox-bg-primary are ignored if a theme override color is present.

Solutions

I have two thoughts on how to fix this. The first would be to move the theme override call into the variable defintion

- $valuebox-bg-primary: $primary !default;
+ $valuebox-bg-primary: theme-override-value($theme-name, "valuebox-bg-primary", $primary) !default;

In flatly, the color="primary" value box still wouldn't pick up the $primary color (because it's overridden), but at least users could set $valuebox-bg-primary directly with something like

/*-- scss:defaults --*/
$primary: #4CAF50 !default;
$valuebox-bg-primary: #4CAF50 !default;

The second option would be to update theme-override-value() to take both the user variable and the default value. That pattern would look like this:

$valuebox-bg-primary: null !default;

$valuebox-colors: (
  "primary": theme-override-value($theme-name, "valuebox-bg-primary", $valuebox-bg-primary, $primary).
  // ...
);

In this scenario, theme-override-value() would return early if $valuebox-bg-primary is set (returning its value), otherwise it would continue with the same logic, using $primary as the default value if there isn't otherwise a default value. In places where this insertion of user preferences isn't needed, calls to theme-override-value() would become theme-override-value($theme-name, "primary", null, $primary). Edit: actually, we could put the $user-value last in the function signature to avoid having to update any other uses.

@cscheid I'd be happy to contribute a fix if either of the above sound good to you.

gadenbuie commented 5 months ago

several hours later 🤦 my two suggestions are functionally equivalent – both require that the user set $valuebox-bg-danger to reliably change the value box background color. I'll PR the first suggestion.

AlbertRapp commented 5 months ago

Thanks for all of that work @gadenbuie :tada: Just a short question: Is $valuebox-bg-danger documented somewhere? I feel like I haven't seen it in the docs anywhere yet :thinking:

gadenbuie commented 5 months ago

I don't think they are. There are only a couple of dashboard-related variables (see here). Seems like Dashboard theming - Sass variables would be a good place for these to be documented.