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

Use the core metalsmith postcss plugin #2258

Merged
merged 1 commit into from
Jul 21, 2022
Merged

Conversation

domoscargin
Copy link
Contributor

@domoscargin domoscargin commented Jul 20, 2022

Metalsmith is being updated fairly regularly now, and have a core PostCSS plugin.

We've been blocked on bumping some dependencies prior to this (#2016), but are now able to fully bump postcss.

I've run npm build and there is one small prefixing change in the resulting CSS, but otherwise it's exactly the same.

In the generated IE8 CSS we go from:
.govuk-template{background-color:#f3f2f1;-webkit-text-size-adjust:100%;-ms-text-size-adjust:100%;text-size-adjust:100%;overflow-y:scroll}

to:
.govuk-template{background-color:#f3f2f1;-webkit-text-size-adjust:100%;-moz-text-size-adjust:100%;-ms-text-size-adjust:100%;text-size-adjust:100%;overflow-y:scroll}

and in the standard generated CSS we go from:
.govuk-template{background-color:#f3f2f1;-webkit-text-size-adjust:100%;-ms-text-size-adjust:100%;text-size-adjust:100%

to:
.govuk-template{background-color:#f3f2f1;-webkit-text-size-adjust:100%;-moz-text-size-adjust:100%;-ms-text-size-adjust:100%;text-size-adjust:100%

PostCSS major changelogs

8.0.0
7.0.0

@domoscargin domoscargin marked this pull request as ready for review July 20, 2022 14:28
@netlify
Copy link

netlify bot commented Jul 20, 2022

You can preview this change here:

Name Link
🔨 Latest commit ee324be
🔍 Latest deploy log https://app.netlify.com/sites/govuk-design-system-preview/deploys/62d94caa4c5c5e00089b0cff
😎 Deploy Preview https://deploy-preview-2258--govuk-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@vanitabarrett
Copy link
Contributor

I wonder if this will unblock #1663 🤔

@domoscargin
Copy link
Contributor Author

domoscargin commented Jul 20, 2022

Ooh, interesting. Seems like it probably would.

Edit: Yeah, I did a quick swap for stylelint and looked good (just a bunch of linting failures). Didn't spend long on it, but if I get time I might do a proper PR.

@domoscargin domoscargin changed the title Use the official metalsmith postcss plugin Use the core metalsmith postcss plugin Jul 20, 2022
@domoscargin domoscargin added dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code tooling labels Jul 21, 2022
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Changes make sense, but this also bumps metalsmith to v2.5.0 which isn't currently mentioned in the PR or commit message.

Suggest either splitting in to a separate commit or updating the commit message to cover all the changes.

@domoscargin
Copy link
Contributor Author

Done! metalsmith bump here: #2259

@36degrees
Copy link
Contributor

Thanks for that! I think the changes to the package-lock.json still include Metalsmith 2.5.0 (take a look at the 'rich diff' for the lock file), but suspect you'll need to resolve that once #2259 has been merged as it'll conflict anyway…

Metalsmith is being updated fairly regularly now, and have an [official PostCSS plugin](https://github.com/metalsmith/postcss).

We've been blocked on bumping some dependencies because of this (#2016).

I've run `npm build` and there is one small prefixing change in the resulting CSS, but otherwise it's exactly the same.
@domoscargin domoscargin merged commit e468190 into main Jul 21, 2022
@domoscargin domoscargin deleted the bk-bump-metalsmith-postcss branch July 21, 2022 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code tooling
Projects
Development

Successfully merging this pull request may close these issues.

3 participants