Skip to content

Conversation

@pbearne
Copy link
Contributor

@pbearne pbearne commented Apr 5, 2022

Summary

This stores a dominant color of an image to image meta.
We then use this as a background color for images.

Checklist

fixes: #19

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

pbearne and others added 30 commits March 18, 2022 14:58
we have code
I have no idea if the imagick code path works
not sure the js to remove bg works
created classes for image functions
Updated tests

NOT all tests are passsing
got get_has_transparency() working
enable_dominant_color_for_image
}

try {
// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged
// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged

Do we need this line anymore?

Copy link
Member

Choose a reason for hiding this comment

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

+1, let's remove this line.


'red_png' => array(
'image_path' => TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/dominant-color/red.png',
'expected_color' => array( 'ff0505', 'ff55', 'ff0506', 'ff56' ),
Copy link
Member

Choose a reason for hiding this comment

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

4 different options seems like a lot here. Why are so many different options? Why are there 4 digital hex values?

Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@pbearne @spacedmonkey Awesome work, LGTM!

My remaining comments are all super minor, mostly related to documentation. Please fix these, and this should be good to go.

}

try {
// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged
Copy link
Member

Choose a reason for hiding this comment

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

+1, let's remove this line.

Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Awesome - thanks to everyone involved! 🎉

@felixarntz felixarntz changed the title Add Dominant Color module to provide color background Add Dominant Color module to provide color background for loading images Jun 14, 2022
@felixarntz felixarntz merged commit 11bf31c into WordPress:trunk Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Plugin] Image Placeholders Issues for the Image Placeholders plugin (formerly Dominant Color Images) [Type] Feature A new feature within an existing module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add low quality image placeholders using background color