ResponsiveImagesCG / wp-tevko-responsive-images

Fully responsive image plugin for wordpress
449 stars 53 forks source link

Filtering function updates #206

Closed joemcgill closed 9 years ago

joemcgill commented 9 years ago

Now that we're no longer using a preg_replace_callback function in tevkori_filter_content_images(), it doesn't make as much sense to use the callback function the way we originally set it up. However, rather that merging the two function, this turns the callback into a useful utility function that accepts an img element and returns the HTML including srcset and sizes attributes if we can.

Additionally, since we only add srcset and sizes when we're able to parse the attachment id from the markup, we also no longer need to limit our matches in the content filter by files containing the path to our uploads folder. This resolves #190.

jaspermdegroot commented 9 years ago

The code looks good to me.

bradt commented 9 years ago

Wow, this is a huge shift: no longer matching the src. Is there a conversation on Slack somewhere about this decision? I do think it's right, as querying attachments by image path is slow as hell compared to using IDs, but I'm just curious how the conversation went.

joemcgill commented 9 years ago

Hi @bradt. We removed that custom query recently in order to speed up the display filter. Some of the relevant conversation can be found in the meeting logs from Sept. 25. Once we no longer needed the src it made sense to remove the code that was filtering images based on the path.

At some point, it would be nice to handle cases where we're not able to get the attachment_id by parsing the markup, at which point we will need to reconsider how we filter on image sources that contain a set of accepted paths.

bradt commented 9 years ago

Awesome, great work pushing this forward! :smiley:

joemcgill commented 9 years ago

Fixed the inline docs as noted by @jaspermdegroot and rebased to a single commit so it can be merged cleanly.