Make WordPress Core

Opened 13 months ago

Closed 12 months ago

Last modified 12 months ago

#57533 closed task (blessed) (fixed)

Add preload paths for media categories

Reported by: ntsekouras's profile ntsekouras Owned by: flixos90's profile flixos90
Milestone: 6.2 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch gutenberg-merge changes-requested
Focuses: performance Cc:

Description (last modified by hellofromTonya)

To provide a better user experience when opening the inserter media tab, asynchronously load the media categories.

References:

Change History (17)

This ticket was mentioned in PR #3894 on WordPress/wordpress-develop by @ntsekouras.


13 months ago
#1

  • Keywords has-patch added

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: https://github.com/WordPress/gutenberg/pull/46251

#2 @hellofromTonya
13 months ago

  • Keywords gutenberg-merge added
  • Milestone changed from Awaiting Review to 6.2

Moving into milestone and adding the keyword for tracking.

@ntsekouras commented on PR #3894:


13 months ago
#3

@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?

Yes, they need to, and they are part of the inserter media category. When a user opens the inserter we need to make these requests to check if there is at least one media item per category(audio, image, video). If there is no result we don't need to show the inserter menu item for that category.

If we don't make these requests, the media tab would need to wait for the resolution of these calls, making it display afterwards. This can be a jarring experience and we want to avoid it.

The media inserter tab is implemented for post and site editors.

@TimothyBlynJacobs commented on PR #3894:


13 months ago
#4

I agree with @felixarntz and @spacedmonkey.

If we don't make these requests, the media tab would need to wait for the resolution of these calls, making it display afterwards. This can be a jarring experience and we want to avoid it.

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.

@hellofromTonya commented on PR #3894:


13 months ago
#5

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?

#6 @hellofromTonya
13 months ago

  • Description modified (diff)
  • Keywords changes-requested added
  • Summary changed from Backport media categories preload paths to Add preload paths for media categories

Adjusted summary and description for what the enhancement is for Core. Removing the "backport" reference.

Adding changes-requested to reflect the requested change in PR 3894 to change when these load:

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.

Last edited 13 months ago by hellofromTonya (previous) (diff)

@flixos90 commented on PR #3894:


13 months ago
#7

@hellofromtonya I believe this is currently being discussed further in https://github.com/WordPress/gutenberg/pull/47503.

@ntsekouras commented on PR #3894:


13 months ago
#8

Given this feedback, what needs to change in this PR? @ntsekouras is this something being worked on?

@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..

@hellofromTonya commented on PR #3894:


13 months ago
#9

Thanks @felixarntz and @ntsekouras for the update on where the discussion is happening.

#10 @flixos90
13 months ago

@ntsekouras Is this something that we need to get into 6.2? I believe it'll still take a bit to address the remaining concerns here, so I think we need to either make this a "task (blessed)" or punt it to the 6.3 milestone, depending on whether it is critical or not.

Are there other tickets that depend on this?

#11 @flixos90
13 months ago

  • Focuses performance added

#12 @ntsekouras
13 months ago

@ntsekouras Is this something that we need to get into 6.2?

I think we need to either preload php side or client side with https://github.com/WordPress/gutenberg/pull/47503 or something similar. Media tab still works without it, but the feedback on the original GB PR was to preload these paths. There are no tickets that depend on this.

What I don't understand is if there is any difference doing it client or server side, if we have to do it. Is the discussion here more about whether we need to do it at all?

#13 @hellofromTonya
13 months ago

  • Owner changed from ntsekouras to flixos90
  • Status changed from assigned to reviewing
  • Type changed from enhancement to task (blessed)

Blessing this enhancement in case there's resolution in time for 6.2. As the blocker is for performance concerns, reassigning ownership to @flixos90.

#14 @flixos90
13 months ago

I'm still questioning whether this is a good idea based on the use-case. I left a follow up comment about this in https://github.com/WordPress/gutenberg/pull/47503#issuecomment-1435351114.

Based on my understanding, my vote would be to instead show the "Media" tab unconditionally and only issue the requests once the user opens the inserter, as in that situation they would not see the media categories anyway (as that is not the default tab).

@ntsekouras commented on PR #3894:


12 months ago
#15

I'm closing this since we'll back port the following changes here: https://github.com/WordPress/gutenberg/pull/47503 which handle this issue client side.

The approach is:

  1. If enableOpenverseMediaCategory setting is set to true(which is the default) show the media tab and also init the requests for the media types in library. This ensures that the media tab will be there right after we open the inserter and since it's not the default tab for any editor, it will almost certainly have resolved the requests when a user selects that tab.
  2. If enableOpenverseMediaCategory is set to false, the media tab will wait for the resolution of the requests and will be displayed if there are any available media categories. This is not ideal, but it's less common and it's better than showing the media tab where there could be a chance to remove that tab after the requests' resolution.

#16 @ntsekouras
12 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

#17 @hellofromTonya
12 months ago

This ticket is now fixed with [55392] which includes a client side fix in the @wordpress package updates.

Note: See TracTickets for help on using tickets.