WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 24 hours ago

#50105 assigned defect (bug)

Remove infinite scrolling behavior from the Media grid

Reported by: afercia Owned by: afercia
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch has-screenshots needs-dev-note needs-testing
Focuses: ui, accessibility, javascript Cc:

Description

Splitting this out from #40330.

This ticket aims to discuss technical implementation details for the removal of infinite scrolling from the Media grid. For general discussion on the infinite scrolling behavior, please head over to the main ticket #40330.

Attachments (4)

50105.diff (16.6 KB) - added by afercia 2 months ago.
50105.jpg (90.8 KB) - added by afercia 2 months ago.
50105.2.diff (18.6 KB) - added by afercia 2 weeks ago.
50105 buttons spinner.png (152.2 KB) - added by afercia 2 weeks ago.

Download all attachments as: .zip

Change History (26)

@afercia
2 months ago

@afercia
2 months ago

#1 @afercia
2 months ago

  • Keywords has-patch has-screenshots added

50105.diff is a proof of concept meant for initial testing.

Still to do:

  • update the number of total attachments when adding / removing attachments
  • more testing

It would be great to run extended testing on different installations, with a varying mount of attachments stored in the media library. I'd suggest to focus on:

  • general behavior
  • whether the default "per page" of 40 attachments should be increased
  • filtering by type and date, keeping in mind there are existing bugs when filtering see for example #50025.

Suggestions for testing:

  • styling of the button / spinner / etc. is temporary, please ignore it for now
  • throttle the network via your browser's dev tools: this helps in observing the UI behavior e.g. the spinner visibility
  • test without and with media trash enabled (by setting define( 'MEDIA_TRASH', true ); in your wp-config.php file)
  • test the filter add_filter( 'media_library_infinite_scrolling', '__return_true' ); to restore the infinite scrolling behavior
  • test the Media Library
  • test the Media modal with Classic Editor (Add Media, featured image)
  • test the Media modal with Gutenberg (image, gallery, featured image...)
  • optionally: test the Media modal with plugins that implement custom frames to upload / select attachments
  • inspect the queries array of cached collections

Accessibility:
For now, clicking the "load more" sets focus on the first added attachment. I think it would be best to explore other options, as moving focus unexpectedly should preferably be avoided.

Questions:

1
The patch changes the data returned by the WP_Query used to retrieve attachments. Previously, the response only held the array of attachments. Now, it holds 2 items:

  • the array of attachments
  • the number of total attachments, that is: found_posts

Later, the response is parsed so that it passes the attachments array to the views and sets a new property for the number of total attachments. Basically, the response and the cached queries use a different format. An interesting question in this regard is: should the old format be kept when the media_library_infinite_scrolling filter restore infinite scrolling? The media grid works regardless but I guess this should be explored for potential backwards compatibility issues?

2
One more interesting question for anyone familiar with the history of the media views is what are the various "compat" methods meant for? This part is a bit obscure to me. See things like:

  • Attachment.saveCompat
  • the AJAX call save-attachment-compat and related wp_ajax_save_attachment_compat
  • the attachment_fields_to_edit things (I think this was used for the old upload method?)

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


2 months ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


2 months ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


8 weeks ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


7 weeks ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


6 weeks ago

#8 @afercia
5 weeks ago

  • Owner set to afercia
  • Status changed from new to assigned

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


5 weeks ago

This ticket was mentioned in Slack in #accessibility by nrqsnchz. View the logs.


4 weeks ago

#11 @afercia
3 weeks ago

Related: #50410 which is a blocker for this ticket, at least for the part concerning the text with the media items count / total.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 weeks ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


2 weeks ago

@afercia
2 weeks ago

#14 @afercia
2 weeks ago

50105.2.diff refreshes the patch and:

  • adds a "Jump to first new item" button
  • the button is only displayed on focus for keyboard users
  • uses more wp.i18n

This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.


2 weeks ago

#16 @audrasjb
2 weeks ago

  • Keywords needs-dev-note needs-testing added

Looks really good to me!
Adding needs-dev-note and needs-testing keywords.

#17 @afercia
2 weeks ago

See screenshot above. To clarify the behavior:

  • the "Load more" button is only visible when there are more items to load
  • the spinner is only visible when items are being fetched
  • the "Jump to first new item" button is only visible on focus for keyboard users, after new items have been added

Still to do [Edited]:

  1. avoid focus loss when the "Load more" button disappears because there are no new items to load
  2. scrolling issues in the media modal
  3. update the number of total attachments when adding / removing attachments: an alternative option could be hiding the counts after attachments add/removal
  4. consider whether the default "per page" of 40 attachments should be increased
  5. what to do with other long-standing issues made more apparent by this patch, for example: when filtering attachments, see #50025 (Filters don't show new uploads)
Last edited 9 days ago by afercia (previous) (diff)

This ticket was mentioned in Slack in #core-media by pbiron. View the logs.


2 weeks ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


2 weeks ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


9 days ago

This ticket was mentioned in Slack in #core by afercia. View the logs.


24 hours ago

#22 @afercia
24 hours ago

Here's a list of existing tickets that are blockers for this proposed change:
#50410, #50025, #46127, and #47215.

Note: these are all existing buggy behaviors in the Media views that exist since forever.

Note: See TracTickets for help on using tickets.