-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Backwards Compatibility Improvements? #83
Comments
Hi @ryanwiemer. This is a tough one. The problem is that WordPress saves the HTML for your images as content in your posts, so we'd have to either rewrite all of the content in your database—which isn't a great idea—or filter every image in your content before if loads for your visitors to add the necessary code. The latter is probably the best approach, but I don't think it would perform well. At some point we should probably run a few test cases to see exactly how much drag this would put on the content loading. |
I have also been looking for a solution for this. Besides adding the attributes to existing images, I also wanted to be able to update and completely remove the attributes. Rewriting the content in the database (because of performance) was my first thought, but because I don't have much experience with that I decided to look into filtering the content first. My conclusion was that the performance penalty is more or less the same as for post thumbnails. In my test environment that is ~4ms per image with 5 image sizes for the Pros:
Cons:
I know that it's a goal of this plugin to become part of WordPress core one day. I am not sure what is the best approach when you keep that in mind. Rewriting content in the database will never land in core but that code can easily be separated from the rest (could even be a separate plugin), while filtering the content is a whole different way of doing things and doesn't really match anymore with how WordPress core handles images in the content (ie. creating all markup when the image is inserted). Since I need a customize version of this plugin anyway (to make it work with Lazysizes) I decided to continue to use the content filtering solution. The only con that I have to deal with (performance) can be resolved by using a cache plugin. If you want I can create a PR to share my code. @joemcgill - Although I decided to go for content filtering I am still curious why you think rewriting content in the database isn't a good idea. |
I use content filtering too with good results. The reasons why writing the srcset and the sizes to the content / database is not an option for me: image sizes are design dependent and bloat the database with redundant data, the exact same sizes attribute for every single img in every single post. The srcset contains 5 or maybe 10 times the exact same very long absolute path and the exact same image file name and only a little number suffix difference. This is a total waste. Plus: the code gets terribly unreadable. |
@jaspermdegroot I think you almost answered your own question earlier in your post when you mentioned that the goal is to become a core part of WordPress at some point and the core devs are very wary—as they should be—about rewriting user content. If we can demonstrate that we can set both the srcset and sizes in a filter on page load in a way that is performant, I'd be thrilled to not even have to deal with that rabbit hole. In the mean time, we're doing all the calculation when the post is saved and doing very little filtering on the front end so that users don't experience any lag in page load times on account of the plugin searching the DB for On the other hand, as @side777 noted, we definitely don't want to save |
@joemcgill - Ok, thanks. I was just curious if there was something that I didn't think of. I try to stay away from rewriting in the database, hence I don't have experience with it. I will create a PR to demonstrate content filtering. |
I think it's better to wait with creating a PR for this until we decided about the currently open PRs, since my code is based on the changes in those PRs. |
+1 for this feature. Visual Composer and other drag a drop site builders have proliferated but at the expense of building an extremely bloated site. Being able to add this plugin to one of those sites would be a huge improvement. Maybe if there was a checkbox to turn on content filtering so that it wouldn't be the default? |
…cset attributes Fixes ResponsiveImagesCGgh-83 Fixes ResponsiveImagesCGgh-72
…cset attributes Fixes ResponsiveImagesCGgh-83 Fixes ResponsiveImagesCGgh-72
…cset attributes Fixes ResponsiveImagesCGgh-83 Fixes ResponsiveImagesCGgh-72
…cset attributes Fixes ResponsiveImagesCGgh-83 Fixes ResponsiveImagesCGgh-72
…cset attributes Fixes ResponsiveImagesCGgh-83 Fixes ResponsiveImagesCGgh-72
This has been addressed in #177. |
Awesome plugin guys. Really great stuff. I was wondering if there are any plans for some type of bulk method to add the responsive capabilities of this plugin to previously added images. I know this can be done one by one but if anyone knows of some type of workaround to apply to all past images that would be great! I have another project with 500+ previously added images that I would like to use this plugin with.
Any thoughts?
The text was updated successfully, but these errors were encountered: