Skip to content

Add option for users to generate minified stylesheets using the build:minify command#146

Merged
jackdomleo7 merged 20 commits into
jackdomleo7:masterfrom
tannerdolby:144-minify-css
Nov 9, 2021
Merged

Add option for users to generate minified stylesheets using the build:minify command#146
jackdomleo7 merged 20 commits into
jackdomleo7:masterfrom
tannerdolby:144-minify-css

Conversation

@tannerdolby

@tannerdolby tannerdolby commented Nov 6, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

tannerdolby commented Nov 6, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

jackdomleo7
jackdomleo7 requested changes Nov 6, 2021
View reviewed changes

@jackdomleo7 jackdomleo7 left a comment

Copy link
Copy Markdown
Owner

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 🙂

Comment thread checka11y-minified.css
Comment thread minify.js Outdated
Comment thread package.json Outdated
@jackdomleo7 jackdomleo7 added the project enhancement Enhancement to improving the overall project label Nov 6, 2021
@tannerdolby

tannerdolby commented Nov 7, 2021

Copy link
Copy Markdown
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
View reviewed changes

@jackdomleo7 jackdomleo7 left a comment •
edited
Loading

Copy link
Copy Markdown
Owner

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

tannerdolby commented Nov 8, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

jackdomleo7
jackdomleo7 previously approved these changes Nov 9, 2021
View reviewed changes

@jackdomleo7 jackdomleo7 left a comment

Copy link
Copy Markdown
Owner

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

jackdomleo7 commented Nov 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Owner

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

tannerdolby commented Nov 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

Reviewers

@jackdomleo7 jackdomleo7 jackdomleo7 approved these changes

Assignees

@tannerdolby tannerdolby

Labels

project enhancement Enhancement to improving the overall project

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT]: Generate minified CSS files too

2 participants

@tannerdolby @jackdomleo7