Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add page_sidebar() #588

Merged
merged 15 commits into from
May 22, 2023
Merged

Add page_sidebar() #588

merged 15 commits into from
May 22, 2023

Conversation

cpsievert
Copy link
Collaborator

@cpsievert cpsievert commented May 19, 2023

Partially addresses #569

TODO

@cpsievert cpsievert requested a review from gadenbuie May 19, 2023 21:25
R/page.R Outdated Show resolved Hide resolved
R/page.R Outdated Show resolved Hide resolved
inst/components/fill.scss Outdated Show resolved Hide resolved
R/page.R Outdated Show resolved Hide resolved
Comment on lines 29 to 36
.bslib-page-title {
background-color: $bslib-page-title-bg;
color: $bslib-page-title-color;
font-size: $h4-font-size;
font-weight: 300;
padding: $spacer * 0.5;
padding-left: $bslib-sidebar-padding;
margin-bottom: 0;
Copy link
Member

Choose a reason for hiding this comment

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

Overall this is great! Given that both Bootstrap provides three navbar color schemes and that Bootswatch highlights these in their theme previews, I think we might want to think about leaving some room for building on those classes in the future.

Maybe we could take inspiration from quarto, who IIUC add some sass variables around navbar color and hook off of that. Maybe we'd hook into a sass variable that could be set easily via bs_theme()?

I think we're on a reasonable path that would allow for this kind of expansion, but I also think we can reasonably expect users to ask for something like this soon.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, Quarto actually got the $navbar-bg from bslib :)

inst/components/fill.scss Outdated Show resolved Hide resolved
@cpsievert
Copy link
Collaborator Author

I'm going to go ahead and merge this since it's sort of blocking to the website updates, but feel free to keep commenting on things to follow up on .

@cpsievert cpsievert merged commit d7e11f2 into main May 22, 2023
@cpsievert cpsievert deleted the page-sidebar branch May 22, 2023 21:41
@github-actions github-actions bot restored the page-sidebar branch May 22, 2023 21:43
@cpsievert cpsievert deleted the page-sidebar branch May 24, 2023 18:32
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.

2 participants