-
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
Add experiment for client-side media processing #64650
Conversation
Flaky tests detected in 7278d79. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10548153719
|
eec3eef
to
9d9848e
Compare
8fe08b7
to
fd0303e
Compare
fd0303e
to
07699c3
Compare
Note: the only PHP unit test that's failing is because of #64743. Once that PR is merged, it should be fine. |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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.
@swissspidy This looks great so far. Left a number of comments with little things to improve, but nothing major.
lib/experimental/media/class-gutenberg-rest-attachments-controller.php
Outdated
Show resolved
Hide resolved
lib/experimental/media/class-gutenberg-rest-attachments-controller.php
Outdated
Show resolved
Hide resolved
lib/experimental/media/class-gutenberg-rest-attachments-controller.php
Outdated
Show resolved
Hide resolved
lib/experimental/media/class-gutenberg-rest-attachments-controller.php
Outdated
Show resolved
Hide resolved
lib/experimental/media/class-gutenberg-rest-attachments-controller.php
Outdated
Show resolved
Hide resolved
Co-authored-by: Felix Arntz <flixos90@gmail.com>
Co-authored-by: Felix Arntz <flixos90@gmail.com>
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.
This looks good to me from a REST API perspective.
@spacedmonkey Could you please re-review after the latest changes? Your stale review is blocking the merge. |
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.
Fantastic! Left a few questions.
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.
Looks great to me!
@swissspidy Provided a review already, but I comments seem to losted in the shuffle. Can you take a look at my last comments. |
@spacedmonkey All comments were addressed and/or resolved, that's why I am asking for your re-review to unblock this merge. Further changes can be easily done in follow-up PRs, as everything here is behind an experimental flag anyway. |
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.
LGTM. Good work @swissspidy
Co-authored-by: swissspidy <swissspidy@git.wordpress.org> Co-authored-by: felixarntz <flixos90@git.wordpress.org> Co-authored-by: spacedmonkey <spacedmonkey@git.wordpress.org> Co-authored-by: TimothyBJacobs <timothyblynjacobs@git.wordpress.org> Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
What?
See #61447 for full context.
Split out from #64278, this adds:
crossorigin
attributes where requiredSubmitting this PHP-heavy change to make subsequent reviews easier.
Again, this is all behind an experiment. Iterations can be made in subsequent PRs :-)
These changes are all very well tested as part of https://github.com/swissspidy/media-experiments
All these changes are only taking in effect when the experiment is enabled on the settings page. Nothing happens otherwise.
Why?
This is just a small part towards client-side media processing in Gutenberg. See #61447 for full context.
How?
Testing Instructions
There should be a new checkbox on the settings page. No visual changes otherwise.
Testing Instructions for Keyboard
N/A
Screenshots or screencast
N/A