WordPress.org

Make WordPress Core

Opened 9 months ago

Last modified 8 months ago

#41675 reviewing enhancement

Add filter to bypass display/query for Media Months filter in media modal

Reported by: CrazyJaco Owned by: joemcgill
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Media Keywords: good-first-bug needs-patch
Focuses: performance Cc:

Description

I am investigating slow running queries to try and cut down the page load time for edit pages in the admin.

The media months dropdown filter runs a somewhat expensive query for what it is. On our production database, it adds about 500 ms of query execution time to the page load.

There doesn't seem to be a way to be able to disable this drop down. The query that is run populates the field. But when there are a mass quantity of media, this is not an optimal method of search.

There should be an option/filter to disable the display and query for this.

Attachments (1)

41675.diff (7.2 KB) - added by herregroen 9 months ago.
Adds a 'media_library_show_months_select' which returns a boolean.

Download all attachments as: .zip

Change History (7)

#1 @CrazyJaco
9 months ago

This filter hook appears to exist: media_library_months_with_files

If the returned value is anything other than an array, it will execute default behavior (running the query and displaying the drop down)

If the returned value is an array, it will skip the query and populate the drop down with whatever is in the array. If an empty array is passed, the drop down will still appear with no options to select.

#2 @johnbillion
9 months ago

  • Focuses performance added
  • Keywords needs-patch good-first-bug added

Thanks for the report, @CrazyJaco .

This needs a patch to clarify the media_library_months_with_files filter description, and to change the dropdown behaviour so it's not shown when there are no elements in the corresponding array.

#3 @CrazyJaco
9 months ago

Thanks!

I spoke with @joemcgill at contributor day for WordCamp for Publishers about this. I would like to work on this with some guidance from him.

Last edited 9 months ago by CrazyJaco (previous) (diff)

@herregroen
9 months ago

Adds a 'media_library_show_months_select' which returns a boolean.

#4 @herregroen
9 months ago

  • Keywords has-patch added; needs-patch removed

Just added a patch which adds a 'media_library_show_months_select' filter which returns a boolean determining whether or not to show the months select. Returning false will not mean 'media_library_months_with_files' filter is not run and the default query is not run either.

In the browser the months select and it's accompanying accessibility label are not rendered if false is returned.

I felt it was clearer to add a new filter in the line of media_library_show_video_playlist and media_library_show_audio_playlist with a singular behaviour than to expand the behaviour of the media_library_months_with_files filter to include new behaviour.

Patch could also serve as an example for a patch that does change the behaviour of media_library_months_with_files.

Last edited 9 months ago by herregroen (previous) (diff)

#5 @joemcgill
9 months ago

  • Owner set to joemcgill
  • Status changed from new to reviewing

Thanks for the great start on this @herregroen!

I could see how a separate filter for this case might be helpful, but I think we can support this right now with the current filter if, as @CrazyJaco suggests, someone were to return an empty array, then the UI should not be shown.

There are a few places in the AttachmentsBrowser view where a new wp.media.view.DateFilter view is rendered, which will need to be accounted for — the grid view in the media library, and the default one used by the wp.media.frame. In the latter, there is already a check to se if this.options.date is true before rendering the DateFilter view. I think we could add a check for the length of wp.media.view.settings.months as well before rendering that view.

Additionally, in the list view of the media library, you can disable the months dropdown using the disable_months_dropdown filter. Seems like we could take advantage of this opportunity to unify the process for disabling the months dropdown between the media views and the WP_List_Table.

#6 @joemcgill
8 months ago

  • Keywords needs-patch added; has-patch removed

@herregroen or @CrazyJaco – either of you interested in working on an update on the initial patch? We could still get this in 4.9 if there's a new patch in the next week.

Note: See TracTickets for help on using tickets.