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

Update to Bootstrap v4 #958

Merged
merged 5 commits into from
Jul 29, 2024
Merged

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Jul 26, 2024

preview

Description of proposed changes

below is from the main commit, but there are additional smaller commits

It's not clear to me where this bootstrap.css file came from, but it seems to be roughly a small subset of the bootstrap.css file distributed¹ in v3.4.1. This commit replaces it with the bootstrap-grid.css file distributed in v4.6.2.

Bootstrap v4 introduced more precise breakpoints and shifted the xs/sm/md/lg classes one level down from the previous version.² Usage has been adjusted accordingly.

¹ https://getbootstrap.com/docs/3.4/getting-started/#download
² https://getbootstrap.com/docs/4.0/migration/#responsive-utilities

Related issue(s)

Checklist

Previously these were set in two places:

1. the modified styled component
2. bootstrap styles on `.container`

Move all the relevant bootstrap styles into the modified styled
component.
The previous navbar max width looked too small on large screens with
page content extending beyond the navbar width. This common width should
really be defined in one place, but that's not trivial as described in
the TODO comment.
@victorlin victorlin self-assigned this Jul 26, 2024
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-victorlin--ywrpjq July 26, 2024 23:56 Inactive
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--ywrpjq July 27, 2024 00:57 Inactive
Future-proofing for Bootstrap v4 change to use flexbox.
It's not clear to me where this bootstrap.css file came from, but it
seems to be roughly a small subset of the bootstrap.css file
distributed¹ in v3.4.1. This commit replaces it with the
bootstrap-grid.css file distributed in v4.6.2.

Bootstrap v4 introduced more precise breakpoints and shifted the
xs/sm/md/lg classes one level down from the previous version.² Usage has
been adjusted accordingly.

¹ https://getbootstrap.com/docs/3.4/getting-started/#download
² https://getbootstrap.com/docs/4.0/migration/#responsive-utilities
… updating to Bootstrap v4

These seem to be more appropriate. Not sure why these weren't necessary
before.
@victorlin victorlin force-pushed the victorlin/update-responsive-styling branch from 0e09c3b to 6177e27 Compare July 27, 2024 02:01
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--ywrpjq July 27, 2024 02:02 Inactive
@victorlin victorlin marked this pull request as ready for review July 27, 2024 02:07
Copy link
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

Nice, I've definitely wondered why we don't use more Bootstrap in this repo...

@victorlin
Copy link
Member Author

Nice, I've definitely wondered why we don't use more Bootstrap in this repo...

The choice of styling framework is also unclear to me. I've created an issue to continue discussion: #963

@victorlin victorlin merged commit c1ce1c0 into master Jul 29, 2024
7 checks passed
@victorlin victorlin deleted the victorlin/update-responsive-styling branch July 29, 2024 21:51
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.

4 participants