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

Prepare client components for bootstrap 4 #764

Merged
merged 16 commits into from
Jan 24, 2022

Conversation

LordSputnik
Copy link
Member

Review time estimate: 20 minutes

Problem

There are a number of issues which it's helpful to resolve before moving the site from Bootstrap 3 to Bootstrap 4.

Solution

Before submitting the PR that moves to BS4, this PR addresses these issues, fixing a few UI bugs, speeding up webpack compilation time and cleaning up unused code.

Areas of Impact

Client components and webpack configuration. Excluding whitespace changes, the largest diffs are to src/client/components/forms/userCollection.js and src/client/components/pages/licensing.js.

When clicked, <a> font weight is 700, but in bs4, <small> gives a 400 font weight. So
<small> must wrap <a> to avoid <a> font weight being overridden.
This ensures that the padding below the contact us buttons is consistent
between desktop and mobile devices.
@@ -112,12 +111,7 @@ const clientConfig = {
new MiniCssExtractPlugin({
filename: 'stylesheets/[name].css'
}),
new CleanWebpackPlugin(cleanWebpackPluginOpts),
new ESLintPlugin({
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the goal of the removal of ESLint to make Webpack run faster?

I'm currently working on the automated Github actions we have that can annotate and even auto-fix lint issues. There are some issues with forked repos as visible here

There are some issues when it comes to PRs made from a fork repo, and we won't be able to automatically commit fixes in those PRs. Instead we are able to add annotations to the PR.

However for PR from this repo and when we merge any PR we are able to commit auto-fixed lint issues, so in combination with annotations on the PR that might be sufficient.

Copy link
Contributor

@MonkeyDo MonkeyDo Jan 24, 2022

Choose a reason for hiding this comment

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

We should probably remove the eslint-webpack-plugin package if we're not using it here anymore.

I could see a scenario where we would want to lint and autofix during production builds, so the plugin might be useful after all.

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 for these fixes !
Looking forward to the BS4 PR !

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