Skip to content

Conversation

@tannerdolby
Copy link
Contributor

@tannerdolby tannerdolby commented Nov 6, 2021

Description

Fixes #144

Why is this important?

This will allow users to generate minified stylesheets with npm run build:minify, it currently generates the minified files at the root directory as <original_filename>-minified.css. If a user wanted to only generate one of the stylesheets or two, we could add that logic.

Covered test cases

No test cases have been added yet.

Did you test on all major browsers?

  • Chrome
  • Firefox
  • Edge
  • Safari

T&Cs

  • I confirm I have read and understand the contributing guidelines
  • I understand the work in this pull request will not be released straight away and will appear in a future release (if approved)
  • I confirm the work in this pull request is true and valid to the best of my knowledge
  • I have updated the README, features.md and codes.md files where applicable

@tannerdolby
Copy link
Contributor Author

tannerdolby commented Nov 6, 2021

This PR cleans up my mistakes in #145 of committing the package-lock.json which created a very large 6k diff.

Copy link
Owner

@jackdomleo7 jackdomleo7 left a comment

Choose a reason for hiding this comment

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

Thank you for picking this up @tannerdolby! Just left a few comments, then we'd be good to go with this PR 🙂

@jackdomleo7 jackdomleo7 added the project enhancement Enhancement to improving the overall project label Nov 6, 2021
@tannerdolby
Copy link
Contributor Author

You're welcome @jackdomleo7! I'm glad you mentioned using the clean-css-cli package because it much better suits our needs here. We can now minify stylesheets straight from the command line using build:minify and without any JS added to the project 🙂

jackdomleo7
jackdomleo7 previously approved these changes Nov 8, 2021
Copy link
Owner

@jackdomleo7 jackdomleo7 left a comment

Choose a reason for hiding this comment

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

Looks brilliant! Thank you. Just 1 merge conflict.

@tannerdolby
Copy link
Contributor Author

tannerdolby commented Nov 8, 2021

Ok great! You're welcome. Ah, gotcha I will resolve that merge conflict real quick.

jackdomleo7
jackdomleo7 previously approved these changes Nov 9, 2021
Copy link
Owner

@jackdomleo7 jackdomleo7 left a comment

Choose a reason for hiding this comment

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

Thank you! Sorry, I didn't realise I could push changes to your branch and I only just thought about the things I committed and didn't want to have this PR hanging around for longer 🙂

@jackdomleo7
Copy link
Owner

jackdomleo7 commented Nov 9, 2021

Sorry @tannerdolby... I added some commits that I forgot when reviewing, then the tests wouldn't run. Even though I didn't touch the tests. Then I had to add a few more commits, including adding clean-css as a dev dependency because it wasn't liking it for some reason. Strange how it passed for you though. 🤔

Anyway, thank you for this!

@jackdomleo7 jackdomleo7 merged commit b4e10e5 into jackdomleo7:master Nov 9, 2021
@tannerdolby tannerdolby deleted the 144-minify-css branch November 9, 2021 16:25
@tannerdolby
Copy link
Contributor Author

tannerdolby commented Nov 10, 2021

No worries @jackdomleo7! You're welcome :)

Thanks for adding the commits needed to push this through. Yeah I didn't think clean-css-cli needed clean-css at all since the cleancss command in package.json is directly coming from clean-css-cli. But maybe like you mentioned, clean-css-cli needs clean-css as a peer dependency. Either way, I'm glad to see the project now gives users the option to use minified stylesheets.

Thank you for reviewing and seeing this through! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

project enhancement Enhancement to improving the overall project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT]: Generate minified CSS files too

2 participants