-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 preload paths for media categories #3894
Add preload paths for media categories #3894
Conversation
@@ -72,6 +72,9 @@ static function( $classes ) { | |||
sprintf( '%s/autosaves?context=edit', $rest_path ), | |||
'/wp/v2/settings', | |||
array( '/wp/v2/settings', 'OPTIONS' ), | |||
'/wp/v2/media?context=edit&per_page=1&orderBy=date&media_type=image', | |||
'/wp/v2/media?context=edit&per_page=1&orderBy=date&media_type=video', | |||
'/wp/v2/media?context=edit&per_page=1&orderBy=date&media_type=audio', |
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'm not sure if it's preferred in this format:
add_query_arg(
array(
'context' => 'edit',
'per_page' => 1,
'orderBy' => 'date',
'media_type' => 'image',
),
rest_get_route_for_post_type_items( 'attachment' )
),
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.
Yeah this would be better.
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.
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.
@ntsekouras I see this implemented in Gutenberg, however in terms of performance considerations I would like to understand why these requests are being preloaded. Will they be made anyway as soon as the block editor loads?
We're talking about WP Admin here, so performance is not as critical as for the frontend, but I would still want to ensure we don't add these queries unnecessarily.
Since I don't have the context here, can you outline the use-case for what these endpoints are used? And most importantly: Is this relevant for every block editor page load, or do you have to do something specific in the block editor for that data to become relevant?
@felixarntz yes, they need to, and they are part of the inserter If we don't make these requests, the The |
I agree with @felixarntz and @spacedmonkey.
I don't think these need to be completed when the editor is initially loaded. Instead, they could be loaded asynchronously when the editor loads, but before the user opens the inserter. |
Given this feedback, what needs to change in this PR? @ntsekouras is this something being worked on? |
@hellofromtonya I believe this is currently being discussed further in WordPress/gutenberg#47503. |
@hellofromtonya I've opened a GB PR to see if it makes any difference to preload these client side in JS. The discussions there are mostly about whether we can avoid making the requests as soon as the editor loads, but I don't think that's an option. This is because these requests are needed for a user before opening the inserter, which is something that could be their first actions as soon as the editor loads.. |
Thanks @felixarntz and @ntsekouras for the update on where the discussion is happening. |
I'm closing this since we'll back port the following changes here: WordPress/gutenberg#47503 which handle this issue client side. The approach is:
|
Trac ticket: https://core.trac.wordpress.org/ticket/57533
This PR adds the new preload paths for the inserter media categories for post and site editors.
Related GB PR: WordPress/gutenberg#46251
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.