Skip to content

Conversation

@gadenbuie
Copy link
Member

@gadenbuie gadenbuie commented Oct 25, 2023

Sidebar transition minimum time

The main goal of this PR started out as enforcing a minimum transition time, but the pattern was useful and I decided to apply it everywhere.

Previously we set the sidebar transition duration like this:

.bslib-sidebar-layout {
  --bslib-sidebar-transition-duration: 500ms;
}

But this has two major downsides:

  1. The (sort-of) "public" property default can't be overwritten, instead we'd have to use max(var(--bslib-sidebar-transition-duration, 500ms), 5m) every single time we want to use this value (annoying and obfuscating).

  2. The "public" CSS property is scoped to the .bslib-sidebar-layout class, which is part of why we can't overwrite it. Users wanting to set the transition duration would end up clobbering our max default calculation.

The new approach creates a pseudo-private property--_transition-duration – whose value inherits the public property – --bslib-sidebar-transition-duration.

This means that we can store the default calculation in the psuedo-private variable, and internally reference the private variable directly.

.bslib-sidebar-layout {
  --_transition-duration: max(var(--bslib-sidebar-transition-duration, 500ms), 5ms);

  > .main {
    transition: padding var(--_transition-easing-x) var(--_transition-duration);
  }
}

Because --bslib-sidebar- is already a lot for a prefix, I dropped it from the private properties. This makes the CSS easier to read where these properties are used. Also, because these are scoped to .bslib-sidebar-layout, the class acts as a prefix, so internally we can be more succint.

All other properties

After doing this for the --_transition-duration I decided to go all in and convert the rest of the properties. The end result is that it's now a lot easier to theme sidebar layouts using custom CSS properties on :root:

page_fixed(
  h2("Demo"),
  tags$style("
  :root {
    --bslib-sidebar-main-bg: #fff4e6;
    --bslib-sidebar-main-fg: #ff7733;
    --bslib-sidebar-bg: hsl(336 100% 50%);
    --bslib-sidebar-fg: #fff4e6;
  }"),
  layout_sidebar(
    sidebar = sidebar(title = "Sidebar title!"),
    lorem::ipsum(2, 1)
  )
)

image

Currently, users could globally use .bslib-sidebar-layout instead of :root, but it's still tricker to get right than it could be. For example, currently the following doesn't work:

page_fixed(
  h2("Demo"),
  tags$style("
  #this-area {
    --bslib-sidebar-main-bg: #fff4e6;
    --bslib-sidebar-main-fg: #ff7733;
    --bslib-sidebar-bg: hsl(336 100% 50%);
    --bslib-sidebar-fg: #fff4e6;
  }"),
  div(
    id = "this-area",
    style = css(
      "--bslib-sidebar-bg" = "hsl(189 100% 42%)" # turquoise
    ),
    layout_sidebar(
      sidebar = sidebar(title = "Sidebar title!"),
      lorem::ipsum(2, 1)
    )
  )
)

In the above, the goal is to set the defaults for a region of the DOM, e.g. under the #this-area. To make that work currently, you'd have to use #this-area .bslib-sidebar-layout. Also note that inline CSS properties set on a parent would similarly not be passed down, so the inline style on the parent div isn't used by the sidebar layout.

With the pseudo-private properties, both approaches work. By using the private properties, we also have a layer of separation; properties that shouldn't inherit from the cascade don't consult the public CSS properties.

@gadenbuie gadenbuie marked this pull request as ready for review October 25, 2023 19:32
@gadenbuie gadenbuie requested a review from cpsievert October 25, 2023 19:32
Copy link
Collaborator

@cpsievert cpsievert left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@gadenbuie
Copy link
Member Author

gadenbuie commented Oct 25, 2023

@cpsievert Thanks, do you think this is newsworthy (it feels pretty internal to me)?

@cpsievert
Copy link
Collaborator

cpsievert commented Oct 25, 2023

I'm ok with not mentioning it

@gadenbuie gadenbuie merged commit 1264bfd into main Oct 25, 2023
@gadenbuie gadenbuie deleted the sidebar/enforce-transition-time branch October 25, 2023 21:07
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.

3 participants