Media: Add a checkbox to toggle infinite scrolling in the media modal - #12795
Media: Add a checkbox to toggle infinite scrolling in the media modal#12795itzmekhokan wants to merge 3 commits into
Conversation
Infinite scrolling of the attachments list can now be turned on and off at the point of use, from a checkbox rendered before the list of media items in both the media modal and the Media Library grid view. The checkbox reflects the value resolved by `wp_enqueue_media()` and overrides it for the current view only, so the `media_library_infinite_scrolling` filter and the "Infinite Scrolling" user profile option still determine the initial state. Turning the checkbox off reveals the "Load more" button and stops the scroll handler from requesting more attachments; turning it back on hides the button and resumes loading on scroll. To make room for the checkbox before the list, the attachments wrapper is now the positioned, scrolling region in both modes, rather than only when the "Load more" button is present. Fixes #65775.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
… and media items.
joedolson
left a comment
joedolson
left a comment
There was a problem hiding this comment.
This works well within a limited scope, but I think it needs to save the user's preference, rather than just persisting for the duration of the modal. Along with that, notices that inform the user of the preference toggle would be needed.
I pushed a minor visual improvement to fix alignments.
| this.infiniteScrolling = infiniteScrolling; | ||
| this.attachments.options.infiniteScrolling = infiniteScrolling; | ||
| this.$el.toggleClass( 'has-load-more', ! infiniteScrolling ); | ||
|
|
There was a problem hiding this comment.
I think it may be helpful to include some kind of visual affordance that confirms that something has happened, since all the control changes happen out of the viewport.
I also think that the control needs to save this preference for the user, so one possibility would be a notice that appears/is spoken saying "Infinite scrolling preference saved"
…owser. The checkbox that turns infinite scrolling on and off now writes the same "Infinite Scrolling" personal option as the profile screen, through a new `save-media-infinite-scrolling` AJAX action, so the choice persists beyond the current view. Changing the checkbox displays a confirmation next to it and sends the same message to `wp.a11y.speak()`, since the "Load more" button it reveals is at the end of the list of attachments and is usually out of view. Because the control saves that personal option, it is only rendered when saving the option takes effect. `wp_enqueue_media()` now reports whether a `media_library_infinite_scrolling` filter callback overrides the option, or there is no user to save it for. The label is shortened to "Infinite scrolling", as the checked state conveys whether it is enabled. See #65775.
|
Thanks for the review — all three points are addressed in c763743.
One decision worth your input. Since the control saves the personal option, |
|
Tested this out and the experience feels a bit rough. It doesn't automatically update when you change your preference. The alignment of the text for the option is also off, both for the initial "Infinite scrolling" and what I think is the description text when you select an option (it seems like this still isn't fixed?) 
The double infinite scrolling text feels repetitive too. Let's get some @WordPress/gutenberg-design help here in terms of phrasing and placement! We want it to fit in well with other approaches WordPress takes. |
|
@annezazu In my testing, I thought it wasn't switching modes when I checked it, but it actually did; it was just slightly difficult to tell in my test environment, for two reasons: there wasn't a sharp difference between the number loaded in infinite scroll and the total, and because if you've already scrolled to the end, not much actually changes - there's nothing more to load. Can you double check that? (My testing was on the previous iteration of the PR, so it is possible that it's regressed since I checked.) |
|
I tested with 87 photos on a site and if I turned it off, the load more never appeared. I had to navigate away and back for it to show. I almost wonder if a better option is to have a dismissible notice that tells folks there’s an option available to control this in your profile options rather than permanently placing this here. Again, I want to hear from more designers! |
|
To be perfectly honest, I feel this toggle is too conspicuous and might unnecessarily complicate things. Furthermore, I question whether the average user is familiar with the term "infinite scrolling." I'm also hesitant to make such a significant code change right before RC1. 
My proposal is to simply revert the default behavior of infinite scrolling to "disabled" without adding a toggle, while still maintaining the user option. This allows users to maintain the previous default behavior while offering an option for users who prefer infinite scrolling. 
I also wonder if this setting needs to be on the fly. Dynamically switching infinite scroll is a feature that hasn't been around before, and as mentioned, it's hard to tell if dynamically switching infinite scroll is actually working unless there are a lot of images, and it seems like it would only confuse users. |
|
That’s against what @m has explicitly asked for (on by default with the option to disable). This is a project leadership call that’s already been made. |
| checkbox = $( '<input />', { | ||
| type: 'checkbox', | ||
| id: id, | ||
| checked: this.infiniteScrolling |
There was a problem hiding this comment.
In addition to feedback that has been given previously, we should make sure to consider newly rendered media browsers, since they seem to discard the saved choice if it differs from the one during page-load.
To test this:
- Open Featured Image > Media Library with scrolling disabled.
- Enable it and wait for "Preference saved".
- Switch to Upload Files, then back to Media Library.
- The recreated checkbox is disabled again, while the live-region message still says it was enabled and saved.
| checkbox.on( 'change', function() { | ||
| view.toggleInfiniteScrolling( this.checked ); | ||
| view.saveInfiniteScrolling( this.checked ); | ||
| } ); |
There was a problem hiding this comment.
I've noticed that quick changes can persist the opposite of the final checkbox state. Every change immediately launches an independent AJAX request. If an enable request and a subsequent disable request execute out of order, the old value can become the final value.
| view.updateInfiniteScrollingStatus( | ||
| infiniteScrolling ? | ||
| __( 'Infinite scrolling enabled. Preference saved.' ) : | ||
| __( 'Infinite scrolling disabled. Load more button displayed. Preference saved.' ) |
There was a problem hiding this comment.
The success text always announces "Load more button displayed", but the button is kept hidden when the library is empty or hasMore() is false. Can be reproduced with an empty library: no button appeared, but both the visible status and wp.a11y.speak() reported that it did. We may want to use a generic disabled message or condition it on the actual button state.
|
|
||
| $infinite_scrolling = wp_validate_boolean( wp_unslash( $_POST['infiniteScrolling'] ) ); | ||
|
|
||
| update_user_meta( get_current_user_id(), 'infinite_scrolling', $infinite_scrolling ? 'true' : 'false' ); |
There was a problem hiding this comment.
This needs multisite coverage. get_user_option() prefers a blog-prefixed option before the global user meta, but the AJAX handler updates only global infinite_scrolling meta. If a prefixed value exists, which is particularly plausible on multisite, the endpoint reports success while reload continues using the unchanged prefixed value. I believe we should either use global meta consistently, matching the profile screen, or explicitly handle the higher-priority site option.
| * option, so it is only offered when saving that option takes effect: there | ||
| * has to be a user to save it for, and no filter callback overriding it. | ||
| */ | ||
| $can_toggle_infinite_scrolling = is_user_logged_in() && ! has_filter( 'media_library_infinite_scrolling' ); |
There was a problem hiding this comment.
Looks like we only check the filter, and as a consequence the toggle appears in attachment browsers where pagination is meaningless. Rendering is controlled only by one global setting, so every AttachmentsBrowser receives the checkbox, including Edit Gallery, playlist/collection editing, and Edit Selection browsers backed by fixed Selection collections. See GalleryEdit and editSelectionContent(). There, toggling cannot reveal meaningful pagination but still changes a global preference and announces that the current view changed. We might want to make this an explicit per-browser option enabled only for pageable library queries.
|
|
||
| update_user_meta( get_current_user_id(), 'infinite_scrolling', $infinite_scrolling ? 'true' : 'false' ); | ||
|
|
||
| wp_send_json_success(); |
There was a problem hiding this comment.
It appears like the return value of update_user_meta() is ignored and wp_send_json_success() runs unconditionally. A failed write therefore produces "Preference saved".
|
Agree with @t-hamano about the settings location and the familiarity with the term. To me, this belongs to a system preference, similar to changing from light to dark mode and increasing UI contrast. Therefore, I would expect this in the wp-admin settings. Aki's example shows it in General, but it makes more sense to me in Media just before the "Image sizes" heading. Also, if the setting only impacts the modal view and the grid layout can not be modified, then the copy could be more explicit: The above is a suggestion in addition to showing it on by default, as Anne said. I might be missing some context here, but the latest changes use DataViewsPicker and the modern UI, but I see the previous in this PR. If the setting is agreed to be displayed on this screen, and the use of DataViewsPicker remains, I would place it in the View options menu. 
|
|
Thanks for the attention to detail. In balancing leadership direction as far as defaults, with preferences that enable options to accommodate everyone, immediately coming to mind is our desire for a welcome-modal that through an out-of-box experience can let everyone quickly set their personal preferences. Jay Koster explores this in-depth, noting that not one size fits all. That's a long term consideration. Important to know about to contextualise what we do, but not a near-term fix. A user-level opt-out already exists. My understanding is that this opt-out is suggested as being not-discoverable-enough. The way to improve this discoverability is through slow and sustained iteration of the IA to ensure that the same things live in the same places. Users > Profile is perhaps a confusing place for finding things like your colour schemes. But Fran with his mention of Dark Mode alludes to a solution: with the admin-bar everywhere, I would expect a much more prominent and improved CTA for configuring user preferences in that dropdown, clearer than simply "Edit Profile": 
It's not in a great place at the moment, with "admin" and "Edit Profile" being two lines of text inside the same link. As a drop-down menu, I'd expect "Admin" being text, "Edit Profile" and "Log Out" to be menu items. I'd even suggest that "Edit profile" item could be a button with a clearer label, and perhaps help text: "Customise your profile, preferred colour scheme, and interaction settings." That's an iterative design task, worth doing regardless of where we land. For the approach outlined in this PR, let's be clear however what it is we're building: it's a per-view user preference. Importantly, that has precedence:
In both cases, per-view options make sense. The challenge here is that the present default media library has no view options: there's no "Screen Options", and there's no Appearance control like in DataViews has: 
This omission feels like a compromise in the name of its modal variant, moreso than an intentional and considered design choice. Because of that, and absent a simple way to introduce a screen options tab for Media, the solution would be to add a view-config button similar to what DataViews has: reuse the same pattern, and put the infinite scroll toggle inside that. Infinite scroll, items per page, view density: all are valid per-view controls, and none of them deserve the prominence of permanent visibility above the list of media. |
|
Is it possible to move that toggle to the toolbar? 
The dataview-based media modal is still an experimental feature. |
From a purely personal opinion, but ultimately deferring to leads chiming in, this feels like a bandaid solution. The most direct pattern to follow is a cog button that opens a menu, with the toggle inside. |
|
Thanks for the thoughtful response @jasmussen I second you approach, and +1 to this
To @t-hamano
You're right, my mistake. I confused it with the media edit in modal 🙃. |
|
There are two key things in the recent threads that I think are key:
|
|
I'm unsure how to proceed with this PR.
Should we perhaps try this approach once? |
|
I believe that some implementation is necessary for the upcoming release of 7.1 RC3 next week. To move this forward, let's first discuss and agree on a general approach that seems reasonable from both design and accessibility perspectives. One idea I had was to add a toggle button to display the settings for the media library as a dialog. I would appreciate your feedback. |
|
Thanks for your efforts. The approach you took in 12932 is maybe not visually in the best place, but conceptually it uses mostly the same language as DataViews does, which is important from a "same things in the same places" perspective. So if it can build a consensus, I can support it on the path to more iteration in the future. |
7 participants
Adds a point-of-use control for infinite scrolling in the attachments browser, as a follow-up to #65564.
What the problem was:
What the fix does:
save-media-infinite-scrollingAJAX action writes the sameinfinite_scrollingpersonal option as the profile screen, and the two stay in sync in both directions.wp.a11y.speak(), because the controls that change are at the end of the list and usually out of view.Approach and why:
wp_enqueue_media()passes a newcanToggleInfiniteScrollingsetting, which is false when amedia_library_infinite_scrollingfilter callback overrides the option, or when there is no user to save it for. Announcing "Preference saved" for a value a filter overrides on the next page load would be misleading, so no control is offered in that case. This is deliberately blunt: any callback on the filter hides the control, even one that only applies conditionally.media_library_infinite_scrollingfilter and the profile option still determine the initial state, and their documented precedence is unchanged.infiniteScrollingmoves from a module-level constant to per-instance state onAttachmentsBrowser, and is passed down to theAttachmentsview rather than read from the global a second time..attachments-wrapperis now the positioned, scrolling region in both modes rather than only when the "Load more" button is present. This reuses the offsets that already applied to the wrapper at every breakpoint, so no per-breakpoint pixel values change.Trac ticket: https://core.trac.wordpress.org/ticket/65775
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Ticket analysis, tests and generate PR. All changes were reviewed and validated by me.
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.