-
Notifications
You must be signed in to change notification settings - Fork 183
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
Support Bootstrap 5 with datatable(style = "auto")
#1074
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Can we just append the version number to bootstrap
when it's greater than 3?
R/datatables.R
Outdated
# TODO: If DT adds support for BS > 5, update this logic | ||
style = if (bs_v > 4) | ||
"bootstrap5" | ||
else if (bs_v > 3) | ||
"bootstrap4" | ||
else | ||
"bootstrap" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# TODO: If DT adds support for BS > 5, update this logic | |
style = if (bs_v > 4) | |
"bootstrap5" | |
else if (bs_v > 3) | |
"bootstrap4" | |
else | |
"bootstrap" | |
style = paste0("bootstrap", if (bs_v > 3) bs_v) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't do this because I'm not familiar with the process for updating the Bootstrap-specific styles in DT. I was imagining that if/when bslib
begins offering Bootstrap 6, setting style = "bootstrap6"
might be worse than using the "bootstrap5"
styles, especially if Bootstrap 6 support in DT requires adding new CSS files to DT.
If a manual update is required, I'd personally leave this code (with all of its code smell) to avoid giving the false impression that Bootstrap 6 is automatically supported.
I defer to you though, since I haven't investigated how dependencies in DT are handled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, that makes sense to me. Thank you!
Currently, when using a Bootstrap 5 theme from bslib and
style="auto"
,datatable()
will use"bootstrap4"
for the style. This results in a somewhat incorrectly styled datatable.This PR improves the logic to use latest DT-supported Bootstrap styles corresponding to the current bslib theme. In this formulation, future Bootstrap updates would require updating this logic (so I left a todo comment), but if there's an easy way to list the bootstrap versions suppported by DT I could update the PR so that this part doesn't need to be updated when future BS versions are added.