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

Create shiny sass variable default files specific to Bootstrap Version #3914

Merged
merged 5 commits into from
Oct 19, 2023

Conversation

gadenbuie
Copy link
Member

I reverted inst/www/shared/shiny_scss/bootstrap.scss back to its state before the BS 5.3 updates. Those updates tried to support BS 3, 4 and 5.3 in the same bootstrap.scss file.

Then I created variants for each major version of Bootstrap. This allows us to confidently use features of each version of Bootstrap without a lot of checking for variable existence or having to backport Sass variables for older versions of Bootstrap.

The original bootstrap.scss had a small amount of backporting to simultaneously support BS3 and BS4; I removed the backports in each version.

@gadenbuie gadenbuie self-assigned this Oct 11, 2023
Copy link
Collaborator

@cpsievert cpsievert left a comment

Choose a reason for hiding this comment

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

LGTM, thanks. Would also be great to have a short explanation of what this PR changes in terms of default Bootstrap 5 styles. Please also update news.

@cpsievert
Copy link
Collaborator

@gadenbuie it'd be great to get this in fairly soon, since it seems like it could impact testing screenshots.

To that end, I am a bit surprised that inst/www/shared/shiny.min.css hasn't changed as a result of these changes, so before merging, let's make sure that is updated (if need be)

@gadenbuie
Copy link
Member Author

To that end, I am a bit surprised that inst/www/shared/shiny.min.css hasn't changed as a result of these changes, so before merging, let's make sure that is updated (if need be)

Is that file compiled against BS3? In which case it'd be a good thing we don't see changes.

@gadenbuie
Copy link
Member Author

gadenbuie commented Oct 19, 2023

@cpsievert Sorry, I forgot this was waiting on changes from me. I've removed the comments and added a news item.

@cpsievert
Copy link
Collaborator

cpsievert commented Oct 19, 2023

Is that file compiled against BS3? In which case it'd be a good thing we don't see changes.

Ah, good point, thanks!

@cpsievert cpsievert merged commit a0a83d5 into main Oct 19, 2023
12 checks passed
@cpsievert cpsievert deleted the chore/shiny-sass-bs-3-4-5 branch October 19, 2023 21:41
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