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.

Sign up for GitHub

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

Jump to bottom

feat: allow options in colormin #1263

Merged
merged 1 commit into from Dec 14, 2021
Merged

feat: allow options in colormin #1263

merged 1 commit into from Dec 14, 2021

Conversation

kubajmarek
Copy link
Contributor

Allow passing colord lib options to postcss-colormin.
Created from #1262

@codecov-commenter
Copy link

Codecov Report

Merging #1263 (fb67d42) into master (f930261) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1263      +/-   ##
==========================================
- Coverage   96.83%   96.83%   -0.01%     
==========================================
  Files         125      125              
  Lines        8245     8239       -6     
  Branches       15       15              
==========================================
- Hits         7984     7978       -6     
  Misses        261      261
Impacted Files Coverage Δ
packages/postcss-colormin/src/index.js 98.46% <100.00%> (+0.06%) ⬆️
packages/postcss-colormin/src/minifyColor.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f930261...fb67d42. Read the comment docs.

ludofischer
ludofischer approved these changes Dec 14, 2021
View reviewed changes
Copy link
Collaborator

@ludofischer ludofischer left a comment

Choose a reason for hiding this comment

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

I think overall it is an improvement. Should even fix minification when the browsers do not support some features.

packages/postcss-colormin/src/index.js
function pluginCreator() {
function addPluginDefaults(options, browsers) {
const defaults = {
transparent: browsers.some(hasTransparentBug) === false, // Does the browser support 4 & 8 character hex notation
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice to check for feature support!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was already in the code, I just moved it around. 🙈

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, true, but it still a bugfix because previously the options did not get passed down to colord.

@kubajmarek
Copy link
Contributor Author

I guess I should also add this new information to ReadMe... anywhere else?

@ludofischer
Copy link
Collaborator

I guess I should also add this new information to ReadMe... anywhere else?

I think it's enough to add it to the ReadMe. I'll figure out if other docs need to be updated, I think they are automatically generated though.

@ludofischer ludofischer merged commit 3b38038 into cssnano:master Dec 14, 2021
@ludofischer ludofischer mentioned this pull request Dec 15, 2021
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ludofischer ludofischer ludofischer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kubajmarek @codecov-commenter @ludofischer