refactor(preset-shiny): Style updates and nudges

[gadenbuie]

I'll fill out this description with screenshots Monday, in the mean time, here's a tl;dr. These mostly affect only the preset="shiny" theme:

1. Use a smaller, tighter $box-shadow-sm
2. Apply $box-shadow-sm to popovers (previously unshadowed)
3. Use this shadow as the default card shadow
4. I added $bslib-spacer that defaults to $spacer, so that we can change --bslib-spacer without affecting Bootstrap spacing.
5. Then I bumped $bslib-spacer for the shiny preset to 1.5rem. This lets most things breathe a bit more and ensures that spacing between elements in a sidebar layout match the padding in the main and sidebar areas. This did require separating one or two things from --bslib-spacer that were using it as a stand-in for 1rem.
6. The title of page_sidebar() now matches page_navbar() exactly by using similar markup and .navbar classes to wrap the title element.

Sidebar Page

Navbar Page

[gadenbuie]

I'm tempted to rename this --bslib-gap-spacer but that's definitely too big of a change for right now. In general, though, I think we should be careful moving forward to use this variable for adjusting spacing between elements.

I know I used it in some sidebar CSS (that I've adjusted in this PR) for something other than inner-item spacing. I don't think there are remaining places where this happens, but I did want to call it out for future reference.

[cpsievert]

Maybe this should inherit from card shadow box?

[gadenbuie]

That's a good thought, but we want popovers to always use the small shadow box. Also the --bslib-card-box-shadow is unset at nested levels and we'd want to keep shadows on popovers in all cases.

[gregswinehart]

I agree with you about always using the small shadow box, @gadenbuie. I did a little test to see what the other shadows would look like, and a nice thing is that it didn't bother me as much as I thought it would:

Small:

Medium:

Large:

[gregswinehart]

...Nice that it isn't an aesthetically breaking change if someone accidentally does that.

[gadenbuie]

I actually like the larger shadow for the popover, since popovers appear on top of all other UI elements, so they're sort of "closer" to the viewer than the rest of the app.

[gadenbuie]

Another popover shadow comparison in a dashboard context

Large

Medium

Small

[cpsievert]

Then I bumped $bslib-spacer for the shiny preset to 1.5rem.

I'm slightly concerned that this decision will look weird/inconsistent in practice since a somewhat arbitrary set of things will have 1rem vs 1.5rem spacing.

Is the motivation for increasing mainly just for spacing in the main content area of a page_sidebar() with cards? Could we restrict the increased padding to this context?

[gadenbuie]

I'm slightly concerned that this decision will look weird/inconsistent in practice since a somewhat arbitrary set of things will have 1rem vs 1.5rem spacing.

Agreed, which is why I mentioned that --bslib-gap-spacer would be a better name, since that's basically how we're using the spacer variable in bslib. Internally, after this PR, we only use it for gap, padding, and margin in places where the spacing is consistent. I do think there is an internal consistency to how we're using $bslib-spacer where it covers "external" padding/gap/margin. We should stick with $spacer for the "internal" spacing.

There is one place I should check, which is in the --bslib-spacer usage in the shiny preset datatables styles.

Is the motivation for increasing mainly just for spacing in the main content area of a page_sidebar() with cards? Could we restrict the increased padding to this context?

Yes that but also in layout_columns() and layout_column_wrap() and page_navbar(), etc. If we increased this value but only in the context of page_sidebar(), then we would be in a weird place where certain layouts/elements have more spacing in certain places and not others.

[gregswinehart]

Navbar title looks great! I just noticed one little discrepancy in the height between the two (I prefer the taller one.) But this shouldn't block its release at all, I can file an issue later.

[gadenbuie]

just noticed one little discrepancy in the height between the two (I prefer the taller one.) But this shouldn't block its release at all, I can file an issue later.

I looked into this enough to determine that it's the fault of the .nav-underline class (used in page_navbar()) and that it's exactly 1px shorter in height.

[gadenbuie]

@gregswinehart and I talked and decided to use the lower-key shadows and take away borders on top-level cards and value boxes, adding them back for nested cards/value boxes or under the .bslib-card-box-shadow-none class.