mindkomm / timmy

Advanced image handling for Timber.
MIT License
171 stars 13 forks source link

Responsive images for gutenberg editor #26

Closed sque89 closed 4 years ago

sque89 commented 4 years ago

We would like to use the great content responsive image functionality (https://github.com/mindkomm/timmy/blob/master/docs/responsive-content-images.md), but we are using the gutenberg editor. I tried it out and there is one problem:

When selecting a defined custom image size in the gutenberg editor, it gets just not recoginzed. I dont see any javascript errors in the console. Just really nothing is happening.

deepin-screen-recorder_Select area_20200313161745

gchtr commented 4 years ago

Yeah, honestly I didn’t test this yet with the new editor. We’re using Gutenberg ourselves a lot, but not with the default image block.

I hope I can have a look at this in the next few days, because this is really something that we want to support.

gchtr commented 4 years ago

Hey @sque89

I quickly tried it out with a very basic setup of WordPress and Timmy. And for me, changing the size in the editor seems to work for me. So, I have the following questions:

Making the image responsive in the frontend isn’t working yet, but that’s another thing to look at.

sque89 commented 4 years ago

Hi @gchtr

[
        'thumbnail'              => [
            'resize'     => [ 500 ],
            'name'       => 'Thumbnail',
            'post_types' => [ 'all' ],
        ],
        'medium'                 => [
            'resize' => [ 300 ],
            'name'   => 'Medium',
        ],
        'large'                  => [
            'resize' => [ 1024 ],
            'name'   => 'Large',
        ],
        'post'                  => [
            'resize' => [ 1024 ],
            'name'   => 'Post',
        ],

        'featured-row-img-33'    => create_timmy_size('(min-width: 801px) 33.33vw, 100vw'),
        'featured-row-img-50'    => create_timmy_size('(min-width: 801px) 50vw, 100vw'),
        'featured-row-img-66'    => create_timmy_size('(min-width: 801px) 66.66vw, 100vw'),

        'callouts-row-3-desktop' => create_timmy_size('(min-width: 801px) 33.33vw, 1px'),
        'callouts-row-3-mobile'  => create_timmy_size('(max-width: 800px) 100vw, 1px'),
        'callouts-row-4-desktop' => create_timmy_size('(min-width: 1009px) 25vw, 1px'),
        'callouts-row-4-mobile'  => create_timmy_size('(max-width: 1008px) 100vw, 1px'),
        'callouts-row-5-desktop' => create_timmy_size('(min-width: 1009px) 20vw, 1px'),
        'callouts-row-5-mobile'  => create_timmy_size('(max-width: 1008px) 100vw, 1px'),

        'hero'                   => create_timmy_size('100vw'),
        'latest-update'          => create_timmy_size('(min-width: 801px) 16vw, (min-width: 496px) 50vw, 15rem'),
        'event-list'             => create_timmy_size('(max-width: 496px) 100vw, (max-width: 688px) 50vw, (max-width: 1008px) 33vw, (max-width: 1200px) 25vw, calc(1200px / 4)'),
        'staff-row'              => create_timmy_size('240px'),
        'quotes-row'             => create_timmy_size('240px'),
        'timeline'               => create_timmy_size('(max-width: 496px) calc(100vw - 4rem), (max-width: 900px) calc(50vw - 4rem), (max-width: 1200px) 25vw, calc(1200px / 4)'),
        'upcoming-events-row'    => create_timmy_size('240px'),
        'investigation-item'     => create_timmy_size('(max-width: 688px) 100vw, (max-width: 1008px) 50vw, (max-width: 1200px) 33vw, calc(1200px / 3)'),
        'footer-signup'          => create_timmy_size('(max-width: 496px) 1px, (max-width: 33rem) calc(5vw/12), calc((5 / 12) * 33rem)'),
        'media-coverage-row'     => create_timmy_size('12rem'),
        'press-release-row'      => create_timmy_size('(max-width: 801px) calc(100vw - 4rem), (max-width: calc((45rem + 12rem) * (1 + 5 / 7))) calc((7 / 12) * 100vw - 12rem), 45rem'),
        'blog-list'              => create_timmy_size('500px'),
        'news-list'              => create_timmy_size('820px'),

        'container-33-50-100'    => create_timmy_size('(max-width: 496px) 100vw, (max-width: 801px) 50vw, (max-width: 1200px) 33vw, calc(1200px / 3)'),
        'container-33-100-100'   => create_timmy_size('(max-width: 801px) 100vw, (max-width: 1200px) 33vw, calc(1200px / 3)'),
        'container-66-100-100'   => create_timmy_size('(max-width: 801px) 100vw, (max-width: 1200px) 66vw, calc(1200px * (2/3))'),

        'full-25-50-100'         => create_timmy_size('(max-width: 496px) 100vw, (max-width: 801px) 50vw, 25vw'),
        'full-66-50-100'         => create_timmy_size('(max-width: 496px) 100vw, (max-width: 801px) 50vw, 66vw'),
    ]
function create_timmy_size($sizes)
{
    return [
        'resize'     => [ 3000 ],
        'srcset'     => [ [ 375 ], [ 500 ], [ 750 ], [ 800 ], [ 1000 ], [ 1125 ], [ 1500 ], [ 1800 ], [ 2100 ], [ 2400 ], [ 2700 ], [ 3000 ] ],
        'sizes'      => $sizes,
        'show_in_ui' => false,
        'oversize'   => [
            'allow'      => false,
            'style_attr' => false,
        ],
    ];
}
gchtr commented 4 years ago

Alright! I assume that the new image works, because of the way meta data is handled since since version 0.14.0.

Could you try and run Regenerate Thumbnails in a test environment to check whether this fixes the issue that the image size doesn’t change for existing images?

I’ll still have to look at how to make it work in the frontend.

By the way, cool approach with the create_timmy_sizes() helper function ;).

sque89 commented 4 years ago

Regenerating media solved the problem. Perfect, thanks a lot for your time to investigate! I assume this bug can be closed then.

Is there another ticket to track the progress of the actual implementation?

Thanks!

gchtr commented 4 years ago

@sque89 I just added a pull request to add support for Responsive Content Images in #29.

You should be able to test it out by loading that branch:

composer require mindkomm/timmy:dev-block-images

I’d be glad to get some feedback on whether this works for you. I had to add some special handling for cases like

sque89 commented 4 years ago

@gchtr comment in the PR

gchtr commented 4 years ago

This is fixed with the 0.14.5 release.