-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[Block Library - Image]: Try Image compression #56156
base: trunk
Are you sure you want to change the base?
Conversation
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/compat/wordpress-6.5/rest-api.php |
Size Change: +6.22 kB (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
I mentioned it on #55106 already but such a simple approach is not enough IMHO, which is why something like https://github.com/swissspidy/media-experiments is more complex. Just a couple of notes after a quick glance:
|
} | ||
// There is a cached value for the file size, but it seems that it's | ||
// not reliable (@see https://core.trac.wordpress.org/ticket/57459). | ||
$response->data['filesize'] = filesize( get_attached_file( $post->ID ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File size information is already stored in post meta and is part of the rest api response. Please don't add this.
Also use wp_filesize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mention in the code comment that this value is not reliable and might be removed from core.
1587cdb
to
571e4a2
Compare
Hey @swissspidy. As I mentioned in the issue, you plugin does really cool stuff, but I think it's okay to start adding some of the functionality iteratively, and it would be really cool if we could add this functionality for 6.5(if we can). The issue had no activity for a month, that's why I explored this prototype. I'm more than happy if you think we can do this from your plugin code, and of course I'd love to help to land it. Regarding the safari issue the docs might be misleading saying it's supported, but TBH with the prototype I hadn't tested it.
Now that I tested safari, the image seems to get compressed in lower quality, although the results vary quite a lot in safari(it seems to compress less than the other browsers). That's an issue yes. You have overcome this in your code by doing it server side I assume? Or you're using a different JS library? |
I'm doing it client-side in WASM |
What?
Related: #55106
This PR is a prototype for client side image compression through the
Image
block.There are many scenarios here that would need to be handled and tested and there might be lots of nuances with many things, like user capabilities, what info we preserve from the original image, how we 'link' them, etc.. As soon as I'm figuring something, I'm going to add this in the description to be tracked and discussed.
Noting that this is a prototype and not all functionality is implemented and tested. Same goes for the design of course, as I took the liberty to do something partly from @jameskoster design in the issue, and partly from me(which could change altogether :)).
How
Currently the image compression is done at the client and we upload the compressed image to the server only when the button(
compress
) is clicked. This is done mostly because why can't rely/know if a host has disabled the needed extensions for that. Additionally we have the previous on the fly that help the user decide before committing.I'm very wary of the PHP side and I'll to ask more feedback and suggestions soon. For example could we use the
parent
relationship of attachments to keep the compressed versions?Current notes:
meta
when creating the attachment. I could use some help/insights. Ideally we should pass the meta values in the REST API and not handle the update ingutenberg_rest_after_insert_attachment_update_meta
.Testing Instructions
Compress image
section.Screenshots or screencast
Screen.Recording.2023-11-15.at.5.52.39.PM.mov