humanmade / asset-manager-framework

A framework for overriding the WordPress media library with an external asset provider, such as a DAM
GNU General Public License v2.0
155 stars 7 forks source link

Block-Editor Sidebar Image Size Select Issue #65

Open ouun opened 2 years ago

ouun commented 2 years ago

Changing the Image Size from the Block Options panel uses the wrong image url.

2Kapture 2022-10-11 at 12 22 27

robindevitt commented 2 years ago

Hey @ouun thanks for reporting this bug. I haven't been able to replicate the issue myself. Please may you provide some details that can assist in replicating the issue?

ouun commented 2 years ago

Hi @robindevitt, thanks for getting back to me. I am using AMF together with Tachyon (humanmade/tachyon-plugin) & Smart Media (humanmade/smart-media). Now thinking about that again, I might have ended in the wrong repo for this report.

The issue is that using the switch replaces the img src with the default links that have the size as suffix. E.g. filename-1200x500.jpg. But it should be the original filename with the requested size as parameters: filename.jpg?w=1200&h=500

This is Tachyon related, isn't it? If yes, please let me know and I will close the ticket here and reopen it at the othe repo.

Thanks! :)

robindevitt commented 2 years ago

@ouun I have tried testing this further from my side, however I haven't been able to replicate your bug yet.

amf-bug-65

Can you confirm the versions of of AMF, Tachyon and Smart Media you are using so I can try replicate the bug? As well as potential info of your install/ setup.

We can always move this issue to the appropriate repo once the bug is able to be replicated.

ouun commented 2 years ago

Strange. The versions I am using seem to be the most up-to-date:

AMF: 0.13.4 Tachyon: 0.11.5 Smart Media: 0.4.3

I am loading tachyon and smart-media right after each other very similiar to this: https://github.com/humanmade/altis-media/blob/98aa4941aea7a7b7a558129a1fc577f1259a6a5f/inc/namespace.php#L41

Clicking on the block option "replace" and choosing another image size in the media modal works as expected. So it currently seems to me, that for the select field the used Rest API output is not modified correctly. I approved that by logging the output of $data['media_details']['sizes'] here: https://github.com/humanmade/smart-media/blob/d5e22269200b41aaf480b20c469b5fbd9aa7a56f/inc/cropper/namespace.php#L176

Using the "replace" button and choosing another image size from the media library (Smart Media UI) outputs the correct image URLs such as /image.jpg?fit=1024%2C1024. However in the Rest API response, the image URLs logged from $data['media_details']['sizes'] are the regular WordPress /image-1024x500.jpg.

Hope that helps and thanks a lot again!

ouun commented 2 years ago

@robindevitt I found the issue, please stop debugging! It is produced by a filter I added a while ago:

    // Support wp_get_attachment_url usage for images
    if (!is_admin()) {
        add_filter('wp_get_attachment_url', function ($url, $attachment_id) {
            // Rewrite Image URLs to Tachyon Endpoint
            if (wp_attachment_is_image($attachment_id) && get_post_mime_type($attachment_id) !== 'image/svg+xml') {
                return tachyon_url($url);
            }

            return $url;
        }, 10, 2);
    }

I am using the above to modify/fix image URLs that are not handled via Tachyon by default, as my theme uses "wp_get_attachment_url()" to reference images. Any idea how to keep that filter intact while not breaking other parts?

Thanks and kind regards!

robindevitt commented 2 years ago

Hey @ouun

Great to hear!

By default the plugin handles the following extensions

This would imply that the SVG's wont be included through Tachyon.

If you removed the filter does that not give you the desired result?