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
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
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.

pbearne and others added 9 commits June 14, 2022 17:40
…r-gd.php

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
…r-gd.php

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
…r-imagick.php

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
…r-imagick.php

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
pbearne and others added 12 commits June 14, 2022 17:45
…r-gd.php

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
…r-imagick.php

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
…r-gd.php

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
…r-imagick.php

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
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