feat: Add dark-mode toggle switch

[gadenbuie]

Adds the dark mode toggle switch web component to bslib from https://github.com/wch/shinycomponent/blob/f4d8080544a273d9e41287f678f4dfe0d39e4ffb/js/src/forge/dark-mode-switch.ts

cc @wch @nstrayer

TODO

• Document and export input_dark_mode() and toggle_dark_mode()

Implementation notes

I made a few tweaks from the original implementation:

1. Moved all CSS custom properties that might be externally set up to the top of the styles array. I also tracked down their default values to ensure they work outside of shinycomponent and without open-props.style

2. The component sets a configurable attribute on the document element, which can be controlled via the (static) theme-attribute attribute on the web component.

3. The R constructor sets --text-1 and --text-2 to --bs-emphasis-color and --bs-tertiary-color respectively.

4. I also renamed --vertical_correction to --vertical-correction (the _ causes trouble with htmltools::css()) and then unset it in the R constructor. I think we'll probably want to revisit those styles in the future once we see the component used in more contexts.

5. I updated the naming of the theme-value attribute (instead of themevalue) and I tweaked the button title text.

6. A few other minor changes reflect that bslib has slightly different tooling for setting up input bindings from web components.

7. The R component wrapper allows app authors to set the color mode value. I also modified the component to reflect the current state into the webcomponent element and to respond to changes to that value, i.e. setting theme-value="dark" on the web component can be used to trigger a color mode change.

   I then tied this into a receiveMessage() method to be used by the Shiny input binding to toggle or set this attribute via toggle_dark_mode_switch() from R.

8. Other new things added as suggested during the course of the PR including having all toggle watch the document.documentElement for changes in theme and reflect that back internally.

Example

The key in this example is to add the dark mode switch to the nav inside nav_item(), pushed to the right with a preceding nav_spacer().

page_navbar(
  title = "...",
  # options and nav_panel()s
  # ....
  nav_spacer(),
  nav_item(input_dark_mode_switch("color_mode"))
)

library(shiny)
pkgload::load_all()

link_shiny <- tags$a(shiny::icon("github"), "Shiny", href = "https://github.com/rstudio/shiny", target = "_blank")
link_posit <- tags$a(shiny::icon("r-project"), "Posit", href = "https://posit.co", target = "_blank")

ui <- page_navbar(
  title = "My App",
  bg = "#0062cc",
  nav_panel(title = "One", p("First page content."), actionButton("set_light", "Set light mode")),
  nav_panel(title = "Two", p("Second page content."), actionButton("toggle_mode", "Toggle color mode")),
  nav_spacer(),
  nav_item(link_shiny),
  nav_menu(
    title = "Other links",
    align = "right",
    nav_panel("Three", p("Third tab content")),
    nav_item(link_posit)
  ),
  nav_item(input_dark_mode_switch("color_mode", mode = "dark")),
  footer = verbatimTextOutput("debug")
)

server <- function(input, output, session) {
  output$debug <- renderPrint({
    str(list(color_mode = input$color_mode))
  })

  observeEvent(input$set_light, {
    toggle_dark_mode_switch("color_mode", mode = "light")
  })

  observeEvent(input$toggle_mode, {
    toggle_dark_mode_switch("color_mode")
  })
}

shinyApp(ui, server)

Kapture.2023-09-06.at.10.21.37.mp4

[cpsievert]

I think we already importFrom for list2(), and arg_match()/names2() is becoming common enough that I'd be inclined to also importFrom

[gadenbuie]

I never remember what's imported and what isn't. I'm open to importing just these functions, but if we're adding more imported functions from rlang, I'd rather just @import rlang -- which also opens us up to using the rlang type checking functions.

[cpsievert]

Yea, I'm OK with that

[gadenbuie]

I'll do that in a separate PR then! 😄

[cpsievert]

For posterity, it'd be nice to have a link to the shinycomponent source that you worked from in the PR comments

Edit: Original source at https://github.com/wch/shinycomponent/blob/main/js/src/forge/dark-mode-switch.ts

[cpsievert]

LGTM once some of the more minor nitpicks are addressed! 🌔

[wch]

I think that this actually should be separate, and should go inside of bslibShiny.ts.

With the way that the code is currently structured, the dark mode state is part of the page. The switch component reflects the state and lets you change the state, but the <html> tag is the source of truth for the dark mode state.

If someone is programmatically setting the dark mode state from R, it's plausible that they'd want to do it even if there's no dark mode switch on the page. However, if the app doesn't contain the dark mode switch (or any other components in webComponents.js) then this code won't be loaded, and they won't be able to programmatically set light/dark mode.

[wch]

This line of reasoning also implies that the dark mode/theme state should be available via clientData, but that could be done at some point in the future.

[gadenbuie]

If someone is programmatically setting the dark mode state from R, it's plausible that they'd want to do it even if there's no dark mode switch on the page. However, if the app doesn't contain the dark mode switch (or any other components in webComponents.js) then this code won't be loaded, and they won't be able to programmatically set light/dark mode.

Unfortunately, moving this code to bslibShiny.ts doesn't solve this problem, since all components deps are individually added when each component is added to the page.

I think it makes sense to require the input_dark_mode() to be added to the app as a way for the app author to explicitly support dark mode, both to give the app user the opportunity to pick the color mode, as well as a way for us to explicitly check the Bootstrap version (which could be 3 or 4 and therefore not supporting dark mode).

Edit: clarified that component deps are individually added, even the bslibShiny dep

[cpsievert]

As a side note, we're probably going into a direction where if any bslib UI is used, then all component-specific assets (bundled into a single htmlDependency()) will come with it, which I think would "solve" this problem.

[gadenbuie]

@cpsievert true, and that would make the server side toggle work without the input, which I also think is okay. I'd rather keep as much of the implementation together in one place though, unless there a philosophically bigger reason why we would want to separate these pieces.

[wch]

I can see the reasons for keeping the code in this file... But, in the interest of having the component class encapsulate just the component's functionality, I think this message handler should be moved outside of the class.

[wch]

The default value is set both here and inside of connectedCallback(). There are two ways I can think of to simplify, both of which are defensible:

One is to use @property here:

Suggested change

	private attribute = "data-shinytheme";
	@property({ type: String})
	private attribute = "data-shinytheme";

And then you don't have to do manually set it in connectedCallback.

The other option is to modify connectedCallback so instead of this:

    this.attribute = this.getAttribute("attribute") || "data-shinytheme";

it has this:

    this.attribute = this.getAttribute("attribute") || this.attribute;

[gadenbuie]

We've add bslib-specific naming in many more places than I originally intended that at this point I think this could be a static value. Regardless, I opted for the second suggestion – a private value with an initial value that's possibly overwritten when the component is connected.