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

Switch from Bootstrap 3 to Bootstrap 4 #769

Merged
merged 64 commits into from
Jan 31, 2022

Conversation

LordSputnik
Copy link
Member

Problem

The site runs on Bootstrap 3, which means that it's tied to react-boostrap 0.x. This also means that we're unable to upgrade to react 17, react-select 4, and other recent versions of packages within the react ecosystem.

Solution

This PR upgrades the whole site to Bootstrap 4 through a series of transformations and tweaks. The stylesheet build system is changed from LESS to SASS, bootstrap and react-bootstrap packages are updated and the new lobes4 npm package is added as a dependency.

Areas of Impact

Just about every client-side JSX or TSX file. I'd suggest that in addition to running through the commits as a sanity check, it would be good to spin this PR up on test.bookbrainz.org and have a couple of volunteers run through the pages to make sure things work on mobile and desktop at least as well as the current bookbrainz.org.

In bootstrap 4, badge and label have been merged, so it's now necessary to use
the pill attribute in react-boostrap.
Some additional hand-tweaks needed, but script initially used:

sed -i "s|<Panel.Body|<Card.Body|" src/**/*.js*
sed -i "s|</Panel.Body|</Card.Body|" src/**/*.js*
sed -i "s|<Panel.Footer|<Card.Footer|" src/**/*.js*
sed -i "s|</Panel.Footer|</Card.Footer|" src/**/*.js*
sed -i "s|<Panel.Heading|<Card.Header|" src/**/*.js*
sed -i "s|</Panel.Heading|</Card.Header|" src/**/*.js*
sed -i "s|<Panel|<Card|" src/**/*.js*
sed -i "s|</Panel|</Card|" src/**/*.js*
sed -i "s|<Panel.Body|<Card.Body|" src/**/*.ts*
sed -i "s|</Panel.Body|</Card.Body|" src/**/*.ts*
sed -i "s|<Panel.Footer|<Card.Footer|" src/**/*.ts*
sed -i "s|</Panel.Footer|</Card.Footer|" src/**/*.ts*
sed -i "s|<Panel.Heading|<Card.Header|" src/**/*.ts*
sed -i "s|</Panel.Heading|</Card.Header|" src/**/*.ts*
sed -i "s|<Panel|<Card|" src/**/*.ts*
sed -i "s|</Panel|</Card|" src/**/*.ts*
@MonkeyDo
Copy link
Contributor

MonkeyDo commented Jan 28, 2022

Am I correct in assuming we can remove the dependency on the lobes git submodule?
If so I can take care of that.

Edit: I did build the project after removing the submodule, and checked that everything worked correctly, just looking for second opinion :)

lobes4 is now deployed as an npm package and does not need to be checked out as a git submodule
@LordSputnik
Copy link
Member Author

Yes that's right, now that it's an npm module we pull in the dependency that way, so no need for the submodule.

On a slightly unrelated note, we do need to keep the bootstrap module even though it's a dependency in lobes4. I'm thinking I'll change that to a peer dependency in lobes to give projects a bit of freedom over which exact bs4 version is used.

@MonkeyDo
Copy link
Contributor

I can only spot very few remaining changes !

On pages with a search input in the navbar, between the size where the navbar collapses and enough space to fit all menu items comfortably, the content of the menu items wraps:
image

I suppose it's not any worse than what's currently in prod where the search input and menu items wrap to another line (screenshot below), but I'll have a go at making it a bit tidier.
image

Also the BB logo's alignment needs tweaking, as seen on the first screenshots, that's an easy one.

One other small change is the green color of valid input label, but if I'm honest I don't mind the change… The old one looks drab when they're side by side.
Current color:
image

New color:
image

Copy link
Contributor

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Thanks again a lot for this big PR !

@LordSputnik LordSputnik merged commit a0af29e into metabrainz:master Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants