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

More configurable theme; start on Bootstrap 4 support #26

Closed

Conversation

cpsievert
Copy link
Contributor

@cpsievert cpsievert commented Dec 23, 2020

This PR adds a new option, radiant.theme, which makes it possible to set a custom theme (without needing to know about the other defaults behind radiant.nav_ui).

The other CSS changes make things look slightly better when using Bootstrap 4 via the new {bslib} package. See also radiant-rstats/radiant.basics#2 which adds better support for Bootstrap 4 in {radiant.basics}.

library(radiant)
# remotes::install_github("rstudio/bslib")
opts <- options(radiant.theme = bslib::bs_theme(version = 4))
shiny::onStop(function() options(opts))
shiny::shinyAppDir(system.file("app", package = "radiant"))

…nt.nav_ui

Also update some custom styles to better accomodate Bootstrap 4
@vnijs
Copy link
Contributor

vnijs commented Dec 25, 2020

This looks really interesting @cpsievert! I'll make time to review asap. I did notice that bslab is not yet on CRAN. Is there a (rough) timeline for that?

@vnijs
Copy link
Contributor

vnijs commented Dec 26, 2020

I just tried this out locally and I'm not seeing any differences. I also tried the my_theme example from the bslib page and see no differences when starting radiant.data. I installed bslib and updated htmltools to the github version as required. Seems I'm missing something.

image

@vnijs
Copy link
Contributor

vnijs commented Dec 26, 2020

Seems like this also requires the development version of shiny. the my_theme example looks great! The main issue I see is that navbar-right is being wrapped to the next line for some reason. Also happens with basic bs4.

image

There are few other minor issues in basic bs4 that I see so far. See below paging for the DT table and the "store" button taking up two lines. I assume giving the button a bit more space (or reducing size of the +) would address the 2nd issue. Not sure about DT paging though.

image

@cpsievert
Copy link
Contributor Author

Good catches, thanks. cd88f5d solves the DT issues. I don't have a great solution in mind for the "+ Store" button (you could add back in the white-space: nowrap that BS3->BS4 removes on the .btn class, but that'll make the label on the input span 2 lines...it seems a "real" fix might be rather involved without a bunch of benefit, so I'd suggest leaving it)

As for the navbar issue, this is what I see when I run shiny::runApp(system.file("app", package = "radiant"), launch.browser = TRUE) (without bslib), is the expected result?

Screen Shot 2021-01-04 at 1 05 31 PM

@vnijs
Copy link
Contributor

vnijs commented Jan 4, 2021

Thanks for the update and suggestions. I'll look into this next weekend. I think the change in with navbar-right might have been introduced in the dev version of shiny. Below what it should look like with shiny 1.5.0. FYI navbar-ight is used to indicate if you are using an Rstudio project or if not what the base directory is that Radiant will use.

image

@cpsievert
Copy link
Contributor Author

cpsievert commented Jan 4, 2021

Ah, yes, code like this is always gonna be susceptible to changes in shiny:

## based on: https://stackoverflow.com/a/40755608/1974918
navbar[[3]][[1]]$children[[1]]$children[[2]] <- htmltools::tagAppendChild(
navbar[[3]][[1]]$children[[1]]$children[[2]],
proj
)

I think {shiny} can do better to not break this code if {bslib} isn't being used, but I'll send you some changes to better accommodate {bslib} once rstudio/shiny#3235 is closed

@vnijs
Copy link
Contributor

vnijs commented Feb 23, 2023

@cpsievert Thanks again for your help transitioning. Closing.

@vnijs vnijs closed this Feb 23, 2023
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