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 BS4 templates and assets (with help from @jayhesselberth). #1536

Merged
merged 67 commits into from Mar 15, 2021
Merged

Conversation

maelle
Copy link
Collaborator

@maelle maelle commented Feb 25, 2021

Fix #1409

  • headroom config inspired by quillt.
  • syntax highlighting colours come from distill. the opting-out will be documented in [draft!]starting customization docs #1490
  • darker links than Bootstrap default, to ensure sufficient contrast

Not tackled here:

DESCRIPTION Show resolved Hide resolved
R/build-home-index.R Outdated Show resolved Hide resolved
R/navbar.R Show resolved Hide resolved
R/render.r Show resolved Hide resolved
)

# Function needed for indicating where that deps folder is compared to here
transform_path <- function(x) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this and the navbar link creation might need to share logic? not sure. in any case both involve depth.

Copy link
Member

Choose a reason for hiding this comment

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

Can we use an fs function for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've now read fs docs again, and I don't see any good fit.

Copy link
Member

Choose a reason for hiding this comment

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

fs::path_rel() doesn't help?

R/render.r Show resolved Hide resolved
@maelle

This comment has been minimized.

@maelle

This comment has been minimized.

@maelle

This comment has been minimized.

@maelle

This comment has been minimized.

@maelle

This comment has been minimized.

data$headdeps <- data_deps(pkg = pkg, depth = depth)

# Potential opt-out of syntax highlighting CSS
data$needs_highlight_css <- !isFALSE(pkg$meta[["template"]]$params$highlightcss)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we could pass the code background color via a bootstrap variable but it'd make it harder to opt-out of all the syntax highlighting styling at once.

@maelle
Copy link
Collaborator Author

maelle commented Mar 11, 2021

Note to self: NOTE about not portable file paths in tests.

@maelle maelle requested a review from hadley March 11, 2021 16:27
@maelle maelle mentioned this pull request Mar 12, 2021
Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

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

I've noted a few minor code style improvements; I think you should merge soon (no need to ping me for another review) and then we can start tracking smaller problems in individual issues.

NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
R/build-news.R Outdated Show resolved Hide resolved
R/build-news.R Outdated Show resolved Hide resolved
R/html-tweak.R Outdated Show resolved Hide resolved
R/html-tweak.R Outdated Show resolved Hide resolved
R/navbar.R Outdated Show resolved Hide resolved
)

# Function needed for indicating where that deps folder is compared to here
transform_path <- function(x) {
Copy link
Member

Choose a reason for hiding this comment

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

fs::path_rel() doesn't help?

@maelle maelle changed the title BS4 work Add BS4 templates and assets (with help from @jayhesselberth). Mar 15, 2021
@maelle maelle merged commit bebcebb into master Mar 15, 2021
@maelle maelle deleted the null-or branch March 15, 2021 09:21
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.

Switch to bootstrap 4
2 participants