Skip to content
This repository was archived by the owner on Apr 2, 2020. It is now read-only.

Conversation

@davisshaver
Copy link
Member

Reworks the callback to use WordPress 4.4's new responsive image feature. Images without associated attachment are still generated with the previous method.

@goldenapples
Copy link
Contributor

Related issue #66

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect it'd be easier to array_intersect_key with a whitelist of allowed attributes than try to unset any potential attributes that wouldn't work here.

Copy link
Member Author

Choose a reason for hiding this comment

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

@goldenapples Agreed, but we would nullify any new attributes added in img_shortcode_attrs. Kosher?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yeah. true. I can't think offhand what custom attributes you'd want to enable on an image, but in case there are any, we should allow them to pass through. At the very least, you could clean up this logic using array_diff_key.

@davisshaver
Copy link
Member Author

Cleaned up based on @goldenapples' feedback.

The tests are failing due to the srcset seemingly affected by TwentySixteen filter, here:

function twentysixteen_content_image_sizes_attr( $sizes, $size ) {
    $width = $size[0];
    840 <= $width && $sizes = '(max-width: 709px) 85vw, (max-width: 909px) 67vw, (max-width: 1362px) 62vw, 840px';
    if ( 'page' === get_post_type() ) {
        840 > $width && $sizes = '(max-width: ' . $width . 'px) 85vw, ' . $width . 'px';
    } else {
        840 > $width && 600 <= $width && $sizes = '(max-width: 709px) 85vw, (max-width: 909px) 67vw, (max-width: 984px) 61vw, (max-width: 1362px) 45vw, 600px';
        600 > $width && $sizes = '(max-width: ' . $width . 'px) 85vw, ' . $width . 'px';
    }
    return $sizes;
}
add_filter( 'wp_calculate_image_sizes', 'twentysixteen_content_image_sizes_attr', 10 , 2 );

This is especially odd to me given the explicit switch and test of TwentyFiteen being the theme, here.

    public function setUp() {
        parent::setUp();
        switch_theme('twentyfifteen');
    function test_theme() {
        // replace this with some actual testing code
        $this->assertContains( 'Twenty Fifteen', wp_get_theme()->name );
    }

I tried setting the theme again inline the test, no dice.

Stumped on the mysterious behavior here.

@goldenapples
Copy link
Contributor

Stumped on the mysterious behavior here.

That is really weird. I think the issue is that wp_make_content_images_responsive, which is called the first time the_content is run, has some caching logic which caches the responsive html representation of all images in a post content.

I was able to get around this behavior by adding

remove_all_filters( 'wp_calculate_image_srcset' );
remove_all_filters( 'wp_calculate_image_sizes' );
wp_cache_flush();

to the setUp() method in that test suite.

I'm not sure where you got the sizes you're using in the srcset test, so I'm not sure whether the test assertation is wrong, or if there's another filter that needs to be removed as well...?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants