-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Updates Font Families and Font Faces endpoints context param #58287
Updates Font Families and Font Faces endpoints context param #58287
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/experimental/fonts/font-library/class-wp-rest-font-faces-controller.php ❔ lib/experimental/fonts/font-library/class-wp-rest-font-families-controller.php ❔ phpunit/tests/fonts/font-library/wpRestFontFacesController.php ❔ phpunit/tests/fonts/font-library/wpRestFontFamiliesController.php |
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.
-
Defaulting the context to
view
seems in line with most of WordPress core controllers. -
Having the 3 context
view
,edit
andembed
in most of the properties seems correct as we don't have reasons to 'hide' those properties in those contexts.
Looks good to me.
I just cherry-picked this PR to the release/17.6 branch to get it included in the next release: 7529726 |
What?
Updates the
wp/v2/font-families
andwp/v2/font-families/<id>/font-faces
endpointscontext
paramcontext
to view, to be consistent with other Core endpointsview
,edit
, andembed
, since viewing, editing, and embedding are valid contexts for both objectsWhy?
As pointed out by @matiasbenedetto here, the context param seems to be not about where the request is being called from (front-end vs. editor), but about what information is needed to take a specific action on the object.
How?
Updates the endpoint schema and defaults.
Testing Instructions
view
,edit
,embed
and no value for?context=
GET wp/v2/font-families
GET wp/v2/font-families
GET wp/v2/font-families/<id>/font-faces
GET wp/v2/font-families/<id>/font-faces/<id>
OPTIONS
requests to see that thecontext
param correctly defaults toview
and accepts any of those three contexts as valid