search

r-lib / pkgdown

Generate static html documentation for an R package
https://pkgdown.r-lib.org/
Other
718 stars 336 forks source link

Bootstrap variables used by pkgdown #1678

Closed apreshill closed 3 years ago

apreshill commented 3 years ago

Hi @maelle šŸ‘‹

I've been working with the dev version of pkgdown for a bit, and wanted to share what I learned converting a previous CSS to a hybrid/frankenstein BS4 variable setup with additional CSS. I ran into a few issues with Bootstrap 4 variables and tried to annotate as I went.

Here are the variables I used that did seem to work:

    bg: "white"                         # background of site
    primary: "#187BA2"                  # sidebar heading colors / but not content
    headings-color: "#187BA2"           # inherits from body if not set / not sidebars though
    navbar-bg: "#f4e4e7"                # navbar bg color / inherits primary if not set
    secondary: "#595959"                # version badge bg only?
    navbar-light-brand-color: "#595959" # pkgname color / inherits navbar-light-active-color if not set

And here are the variables that do not seem to work and require some brute-force CSS:

    # these don't work
    link-color: "#767676"               # should inherit primary, but doesn't? now pkgdown overrides
    link-hover-color: "#187BA2"         # should inherit and darken link-color, but pkgdown overrides
    fg: "#595959"                       # all body text, cannot leave blank? but also bootstrap overrides?
    navbar-light-active-color: "#187BA2"
    navbar-light-hover-color: "#126180" # pkgdown overrides
    navbar-light-color: "#595959"       # pkgdown overrides

And here was the CSS I needed:

/*----Pkgdown is overriding these----*/

/* fg in YAML */
body {
    color: var(--scorpion);
}

/* navbar-light-color in YAML */
.navbar-light .navbar-nav .nav-link {
    color: var(--scorpion);
}

/* navbar-light-hover-color in YAML */
.navbar-light .navbar-nav .nav-link:hover {
    color: var(--sapphire);
}

/* navbar-light-active-color in YAML */
.navbar-light .navbar-nav .active > .nav-link {
    color: var(--matisse);
}

/* link-hover-color */
a:hover {
  color: var(--sapphire);
}

/* link-color */
a {
  color: var(--boulder);
}

From looking at some of the commits and comments to the pkgdown.css file, it looks like some changes were made to improve accessibility (which is great!), but they seem to come at the cost of allowing users to use the Bootstrap variables. Anyway, I'm happy to funnel some of my notes above which variables affect what into your current vignette (https://pkgdown.r-lib.org/dev/articles/customization.html#bslib-variables), but was hoping you might have some ideas for making these 6 variables work šŸ˜„

I'm really enjoying customizing this new template! Here is my repo for reference: https://github.com/apreshill/bakeoff

And the deployed site: https://bakeoff.netlify.app/

maelle commented 3 years ago

This is awesome @apreshill! I'll make fixes.

maelle commented 3 years ago

(so, thank you!!)

maelle commented 3 years ago

@apreshill the only variable I can't tackle is fg. Is it body-color?

maelle commented 3 years ago

And I suppose it'd make sense indeed to add a few sentences in the vignette, if you want to make a small PR?

apreshill commented 3 years ago

Ooh body-color! I'm just getting the hang of these global bs4 variables too- fg seems to be the bslib way of setting body color (https://rstudio.github.io/bslib/articles/bslib.html#main-colors), although I don't see it documented anywhere else on Bootstrap docs. This is the flexdashboard theming vignette for more context: https://pkgs.rstudio.com/flexdashboard/articles/theme.html#custom-themes

Will do PR today šŸ‘©ā€šŸ³ Are you open to an R-based interactive table (i.e., a reactable, a DT like bslib), or prefer plain markdown?

(brainstorming here)

I think it would have helped me if I could have seen:

Also- I'm not sure if you noticed, but secondary seems largely unused- only for the background of the version badge AFAIK (I've just realized I did not attempt footer elements, so it may cascade there). I was wondering if that variable might be made more useful? One idea I had (b/c it was difficult to tweak with CSS) was to make a variable that controls the hover+active states of the non-homepage TOC elements, and have that same variable affect the hover+active states of the copy button. What do you think?

So for pseudocode purposes, it is were secondary:

/* Hover/Active/focus links */

nav[data-toggle="toc"] .nav > li > a:hover, 
nav[data-toggle="toc"] .nav > li > a:focus,
nav[data-toggle="toc"] .nav-link.active,
nav[data-toggle="toc"] .nav-link.active:hover,
nav[data-toggle="toc"] .nav-link.active:focus {
    background-color: var(--secondary);
}

/*----Copy Buttons ----*/
.btn-primary:hover,
.btn-primary:focus,
.btn-primary:active:hover {
  background-color: var(--secondary);
}

Then I'd probably try to assign the background of the version badge to info? It used to be danger I believe, which didn't seem intuitive. Anyway, just spitballing!

(sorry I accidentally closed prematurely! sloppy keyboard skills :)

hadley commented 3 years ago

A reactable in https://pkgdown.r-lib.org/dev/articles/customization.html#bslib-variables-1 would be great.

We're also very receptive to any suggestions for moving logic out of pkgdown.css and into bs variables.

maelle commented 3 years ago

Thanks so much @apreshill, super useful. Regarding CSS changes before the docs improvements,

Some notes

I suppose making these changes will also make creating the dark mode much easier. I could look into making changes later this week.

Also related https://github.com/rstudio/bookdown/issues/1021

The syntax highlighting CSS would remain separate I suppose. :thinking:

apreshill commented 3 years ago

Hi- sorry for the pause, I wanted to take some time yesterday to dig a bit deeper into both Bootstrap cascades, and how the Bootswatch themes make use of variables and the cascade too. I did find an interesting candidate variable that we could map $secondary onto: $component-active-bg. Seen here:

Screen Shot 2021-05-13 at 11 38 28 AM

If we let $secondary map onto this variable, I think we should make it affect four elements (see the first reactable table here; there are 3 total: one for theme variables, one for overriding theme variables, and one for navbar light/dark variables).

https://focused-snyder-9f2e47.netlify.app/articles/customization.html#bslib-variables-1

What do you both think? (I made these tables more for us- this is not intended as a draft of user-facing documentation-yet šŸ˜„ )

Also in testing, I'm finding that the variable navbar-bg needs to be explicitly set like:

navbar:
  bg: navbar-bg

I wonder if that bslib variable should be the default navbar background, if set (i.e., you don't have to set the navbar bg variable)?

maelle commented 3 years ago

Thanks again @apreshill! I'm digesting all your feedback into the open PR #1682

Regarding the version badge, after experimenting I think the current behavior makes sense: the badge is "danger" for the in development website but "default" (hence secondary-colored) for the release version.

apreshill commented 3 years ago

I agree, "danger" definitely makes sense for in development- can you override the default cascade from $secondary if you use a more specific variable?

I'm also still digesting as well! I'll continue getting some feedback internally about what variable cascades would be least surprising šŸ¤—

maelle commented 3 years ago

Btw @apreshill when you write " I did find an interesting candidate variable that we could map $secondary onto: $component-active-bg" do you have any example of how to do that with sass? I'm trying to use grey-200 for secondary, and am failing to do so.

maelle commented 3 years ago

I agree, "danger" definitely makes sense for in development- can you override the default cascade from $secondary if you use a more specific variable?

It didn't seem so so in my PR I now use badge-info for the release mode. It's $cyan by default, but one can tweak $info to get a different color just for the badge.

maelle commented 3 years ago

@apreshill actually fg works? I've created a monster with red fg and grey bg via the config:

image

maelle commented 3 years ago

One last thing (for today), reg the navbar the default is light at the moment

https://github.com/r-lib/pkgdown/blob/48d88cbb6dcfb10eb3ae5c6abec0cb7af5249e5f/R/navbar.R#L32-L33

apreshill commented 3 years ago

Btw @apreshill when you write " I did find an interesting candidate variable that we could map $secondary onto: $component-active-bg" do you have any example of how to do that with sass? I'm trying to use grey-200 for secondary, and am failing to do so.

This is likely an oversimplification, as I've only used Sass via Hugo pipes. What I was thinking was that you could first do something like:

#pseudo-sass code
.navbar-light .navbar-nav .active > .nav-link {
    @extend #{$component-active-bg}; 
}

I did something sort of like this here: https://github.com/hugo-apero/hugo-apero/blob/05497e6e4382f861c02a52fbdcbd1bdc60cee236/assets/scaffold.scss#L475-L487

This would force the component we want (the active background color for a nav link) "listen to" the variable $component-active-bg.

Then, to make sure the that variable can be set with $secondary...

#pseudo-sass code
$component-active-bg: '$secondary' !default;

Again, this is probably oversimplified, but was just working out an existing Bootstrap variable that might make sense!

maelle commented 3 years ago

Thank you! At the moment I'm trying, before running bslib::bs_theme_dependencies(),

# map secondary to component-active-bg
secondary <- bslib::bs_get_variables(bs_theme, "secondary")
bs_theme <- bslib::bs_add_variables(
bs_theme,
"component-active-bg" = as.character(secondary)
)

which doesn't work yet.

maelle commented 3 years ago

Actually it does work for the button!

maelle commented 3 years ago

I had to keep the rules with $component-active-bg in them, I wonder whether I missed something though I'm not too surprised regarding the rules of the TOC.

maelle commented 3 years ago

mmmh it was simpler actually. :woman_facepalming:

apreshill commented 3 years ago

just waking up and excited to test your new branch out! šŸ‘Æ

maelle commented 3 years ago

wait a bit then :see_no_evil:

Surprised this does not work hence my workaround:

library("bslib")
theme <- bs_theme(
   "primary" = "#fff",
    "component-active-bg" = "$primary  !default"
  )
bslib::bs_theme_dependencies(theme)
#> Error in compile_data(sass_input, options): Error: Undefined variable: "$primary".
#>         on line 2:23 of stdin
#> >> $component-active-bg: $primary  !default;
#> 
#>    ----------------------^

Created on 2021-05-20 by the reprex package (v2.0.0.9000)

https://github.com/rstudio/bslib/issues/317

maelle commented 3 years ago

ok now ready for testing @apreshill

maelle commented 3 years ago

Am I missing something?

maelle commented 3 years ago

Current workflow for replacing values in pkgdown.sass:

Does this make sense? (i.e. have I learnt my lesson? :sweat_smile: )

apreshill commented 3 years ago

Starting off small and working my way up:

If I just use (woot!):

fg: "#982649" 
bg: "white"

The footer color doesn't inherit fg, maybe should?:

footer {
    color: #666;
}
Screen Shot 2021-05-20 at 3 59 54 PM

It looks like a nice alpha is added to fg by default šŸ’… Is that right?

Adding primary:

fg: "#ff0000" 
bg: "white"
primary: "#9B5094"

Changed:

Adding secondary:

bg: "white"                     
fg: "#982649"                    
primary: "#60B2E5"         
secondary: "#FBD989"

oh WOW- all that CSS pain taken away! This is lovely. Also changed background color for copy button when focus/active.

Screen Shot 2021-05-20 at 4 25 10 PM

Narrator's voice After making more changes to navbar-bg, I did notice that the hover background color for dropdowns can get lost, maybe both hover and focus of dropdowns should track with secondary too?

Screen Shot 2021-05-20 at 4 56 02 PM

Adding navbar-bg

bg: "white"
fg: "#982649"
primary: "#60B2E5"
secondary: "#FBD989"
navbar-bg: "#F7CCB6"                 # must set bg key in navbar

Works if I also set bg key in the navbar:

Screen Shot 2021-05-20 at 4 34 11 PM

Adding heading-color works āœ”ļø 

bg: "white"
fg: "#982649"
primary: "#60B2E5"
secondary: "#FBD989"
navbar-bg: "#F7CCB6"                 # must set bg key in navbar
headings-color: "#006DAA"         # inherits from fg if not set
Screen Shot 2021-05-20 at 4 40 16 PM

Moving more quickly now as everything works! All the link coloring works šŸŽ‰

side note: adding body-color still works at this point! Still doesn't affect footer text color though.

I cannot seem to get component-active-color to affect anything- I was expecting this to change the text color anywhere that component-active-bg affected the background color (so text of active navbar links, text of active sidebar TOC links, and focus icon color for copy/paste button?)

bg: "white"
fg: "#982649"
primary: "#60B2E5"
secondary: "#FBD989"
navbar-bg: "#F7CCB6"                            # must set bg key in navbar
headings-color: "#006DAA"                    # inherits from fg if not set
link-color: "#B33C86"                             # inherit primary if not set
link-hover-color: "#EA638C"                  # darkens primary if not set
component-active-bg: "#AADACD"       # yes!
component-active-color: "#9FFFF5"     # nope

OK going to start hitting navbar with testing, but going to start new comment for that!!

apreshill commented 3 years ago

Navbar! Here we go...

Starting from:

    bg: "white"
    fg: "#982649"
    primary: "#60B2E5"
    secondary: "#FBD989"
    navbar-bg: "#F7CCB6"                 # must set bg key in navbar
    headings-color: "#006DAA"            # inherits from fg if not set
    link-color: "#B33C86"                # inherit primary if not set
    link-hover-color: "#EA638C"          # darkens primary if not set
    component-active-bg: "#AADACD"       # yes!
    component-active-color: "#9FFFF5"    # nope

OK everything light that I tried works! I tested:

    # navbar
    navbar-light-brand-color: "#476A6F"  # pkgname color / inherits navbar-light-active-color if not set
    navbar-light-brand-hover-color: "#B7094C" # pkgname hover / inherits navbar-light-hover-color if not set
    navbar-light-active-color: "#B7094C" # active link
    navbar-light-hover-color: "#1B998B"  # hover link
    navbar-light-color: "#2A9D8F"       # link
    info: "#EE92C2"
Screen Shot 2021-05-20 at 8 28 57 PM

I'll pare down my reactable table for the vignette based on this. What do you think?

apreshill commented 3 years ago

Pulling out some notes as a tl;dr:

maelle commented 3 years ago

Thanks a ton for all of this!

Regarding the footer I've now gone with color: mix($body-color, $body-bg, 90%); which means it'll automatically be lighter than normal text. The percentage could be tweaked if someone disagrees.

maelle commented 3 years ago

It looks like a nice alpha is added to fg by default nail_care Is that right?

Not by pkgdown.sass in any case. :thinking: It comes from bslib!

theme <- bslib::bs_theme(
  fg = "#982649",
  bg = "white"
)
bslib::bs_get_variables(theme, "body-color")
#> body-color 
#>  "#A23C5B"

Created on 2021-05-21 by the reprex package (v2.0.0.9000)

maelle commented 3 years ago

I've now made it possible to set the navbar bg via navbar-bg too.

maelle commented 3 years ago

I cannot seem to get component-active-color to affect anything- I was expecting this to change the text color anywhere that component-active-bg affected the background color (so text of active navbar links, text of active sidebar TOC links, and focus icon color for copy/paste button?)

pkgdown.sass fixed for this! Good catch!

maelle commented 3 years ago

what do you think about primary -> navbar-light-active-color + component-active-color? Too much?

I'd first like to have an answer on https://github.com/rstudio/bslib/issues/317 before I add more clunky code. :sweat_smile: would you mind opening a separate issue on this specific mapping? thank you!

maelle commented 3 years ago

what do you think about having the dropdown hover/focus follow secondary if set? (dropdown variables here: rstudio/bslib@e0503c6/inst/lib/bs/scss/_variables.scss#L781)

Could you also open an issue about this? I think it's a good idea but I'd like to add it in a tidy way.

maelle commented 3 years ago

Reg docs I think it'll be better to wait until @hadley reviews the PR, to not document a moving target. Thanks again for all the extremely useful hints & tips!

maelle commented 3 years ago

what do you think about primary -> navbar-light-active-color + component-active-color? Too much?

@apreshill this is now done thanks to @cderv :tada:

maelle commented 3 years ago

Last box checked :tada:

maelle commented 3 years ago

Now border-color too is by default a mix of body-bg and body-color.

maelle commented 3 years ago

A missing mapping is probably the color of text in the navbar. :thinking:

image

apreshill commented 3 years ago

Yes- you mean the non-active/non-hover link color text? With bootswatch, it looks like it is white if dark: https://patchwork.data-imaginist.com/

Looking at bslib variable list I see:

variable value comment
navbar-light-contrast if($navbar-light-bg, color-contrast($navbar-light-bg), $black) Deepest contrasting color for navbarPage(inverse = FALSE). Defaults to $white or $black based on luminance of $navbar-light-bg
navbar-light-color rgba($navbar-light-contrast, .5)

Looks like I cannot currently change anything with navbar-light-contrast in my _pkgdown.yml. If set, it looks like it provides a default for hover/active colors too?

Screen Shot 2021-05-21 at 7 15 42 AM
maelle commented 3 years ago

@apreshill we've made component-active-bg inherit from secondary instead of primary and now I'm wondering whether this is bad for using Bootswatch themes. :thinking:

maelle commented 3 years ago

Reg navbar-light-contrast we can't use it because of

.navbar-light .navbar-nav .active > .nav-link {
   background: $component-active-bg;
   color: $component-active-color;
   border-radius: 1rem;
 }

I've however changed the default: now if navbar-light-color and navbar-light-hover-color aren't set in the configuration, they're a bit lighter than the body color. Does that seem correct? This still means navbar-light-contrast is not usable.

maelle commented 3 years ago

Just some notes for myself. The different places where we are setting/changing things.

.btn-copy-ex:active:hover {
  background-color: $component-active-bg;
  color: $component-active-color;
  border-color: $component-active-bg;
}