Store media library sidebar state per user and move to folder bulk action support

[KMchaudhary]

Fixes: #733, #1584

This pull request introduces several improvements and new features to the media library, with a focus on supporting folder organization and enhancing the user interface for moving media items into folders. The most significant changes include the addition of bulk "Move to folder" actions in the list view, improved sidebar and modal controls for folder assignment, and code refactoring to support these features in a modular way.

Bulk "Move to folder" and folder assignment UI improvements:

• Bulk Move to Folder in List View:
  Added a new ListViewBulkActions class that injects a "Move to folder…" option into the bulk actions dropdown in the media list view. This feature intercepts the bulk action submit event and opens a folder picker UI instead of submitting the form, allowing users to move multiple items at once. [1] [2] [3]

• Move to Folder Button Styling and Accessibility:
  Improved the styling and placement of the "Move to folder" button for both touch and desktop devices. The button is now always reachable on touch devices and visually consistent with WordPress controls in the sidebar and attachment details modals.

Media sidebar and modal enhancements:

• Attachment Details Sidebar and Modal Integration:
  Refactored the rendering of the folder assignment control so it appears consistently in both the two-column attachment details modal and the sidebar, regardless of how the view is constructed. This ensures users can always assign or move folders from any details view. [1] [2] [3] [4] [5] [6]

• Toolbar Integration for Grid View:
  Added the "Move to folder" button to the grid view's toolbar with a specific priority, ensuring it appears in a logical order relative to other bulk actions and controls. [1] [2]

Code organization and maintainability:

• Refactoring of Attachment Field Table Builders:
  Extracted the logic for building custom fields and tables in the attachment details sidebar into a new module (fields/compat-fields.js). This allows different features to add rows to the same sidebar table without duplicating code or creating redundant tables. [1] [2]

Responsive and user preference improvements:

• Sidebar Collapsed State and Breakpoint Handling:
  The sidebar's initial collapsed state now correctly reflects either the user's saved preference or the current screen width, ensuring a smooth experience on both mobile and desktop devices. [1] [2]

Minor UI Fixes:

• SVG Icon Rendering:
  Ensured background SVG icons in the folder tree are not repeated, fixing a visual bug.

Demo

Screen.Recording.2026-09-02.at.12.11.38.PM.mov

Screen.Recording.2026-09-03.at.1.36.56.PM.mov

Screen.Recording.2026-09-03.at.1.40.00.PM.mov

[Copilot]

🟡 Changes recommended

Concurrent toggle requests can persist an outdated state when responses are processed out of order.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Persists each user’s media-library sidebar visibility across sessions.

Changes:

• Adds a REST endpoint backed by user metadata.
• Initializes the sidebar from localized preference data.
• Saves state whenever users toggle the sidebar.

File summaries

File	Description
pages/media-library/redux/api/folders.js	Adds the preference mutation.
pages/media-library/App.js	Loads and saves sidebar state.
inc/classes/rest-api/class-media-library.php	Implements persistence and REST handling.
inc/classes/class-assets.php	Localizes the saved preference.
assets/src/js/media-library/index.js	Applies the collapsed class before insertion.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[KMchaudhary]

Resolved in 48ab1f9. The save is now debounced (500ms): a burst of rapid toggles coalesces into a single write of the latest value, so there is no longer an out-of-order POST that could persist stale state, and a pending save is flushed on unmount so the last toggle isn't dropped.

[github-actions]

🔍 WordPress Plugin Check Report

⚠️ Status: Passed with warnings

📊 Report

🎯 Total Issues	❌ Errors	⚠️ Warnings
16	0	16

---

⚠️ Warnings (16)

📁 composer.json (1 warning)

📍 Line	🔖 Check	💬 Message
0	missing_composer_json_file	The "/vendor" directory using composer exists, but "composer.json" file is missing.

📁 readme.txt (2 warnings)

📍 Line	🔖 Check	💬 Message
0	mismatched_plugin_name	Plugin name "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" is different from the name declared in plugin header "GoDAM".
0	trademarked_term	The plugin name includes a restricted term. Your chosen plugin name - "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" - contains the restricted term "wordpress" which cannot be used at all in your plugin name.

📁 assets/build/blocks/godam-gallery-v2/render.php (2 warnings)

📍 Line	🔖 Check	💬 Message
15	WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound	Global variables defined by a theme/plugin should start with the theme/plugin prefix. Found: "$inner_block_video_ids".
23	WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound	Global variables defined by a theme/plugin should start with the theme/plugin prefix. Found: "$inner_block_video_ids".

📁 assets/build/css/main.css (1 warning)

📍 Line	🔖 Check	💬 Message
0	EnqueuedStylesScope	This style is being loaded in all contexts.

📁 assets/src/libs/analytics.min.js (5 warnings)

📍 Line	🔖 Check	💬 Message
0	EnqueuedScriptsScope	This script is being loaded in all frontend contexts.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/2026/09/09/demo-post-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/demo-page-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/demo-attachment-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.

📁 assets/build/js/main.min.js (5 warnings)

📍 Line	🔖 Check	💬 Message
0	EnqueuedScriptsScope	This script is being loaded in all frontend contexts.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/2026/09/09/demo-post-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/demo-page-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/demo-attachment-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.

---

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

[subodhr258]

Review

Solid, careful work. The cross-bundle event bridge is a clean way to let the Backbone wp.media views talk to the React sidebar, the drop-handler logic is now shared through useMoveAttachments so drag and every button path behave the same, and the comments explain the reasoning well. I reviewed it across eight areas (server PHP, the bridge, the React move flow, sidebar persistence, the attachment-details refactor, the folder-tree refactor, CSS/accessibility, and cross-cutting quality) and adversarially re-checked every candidate issue against the real code and the WordPress core templates. A handful of things are worth fixing before merge; the rest are optional.

Worth fixing before merge

1. A failed move in list view shows no error, and a partial move looks like a success
pages/media-library/hooks/useMoveAttachments.js:198

In the catch block the fail toast is dispatched, then refreshAfterMove() is called with no argument. In list view (upload.php?mode=list) refreshAfterMove reloads the page, and it only parks a toast in sessionStorage when one is passed in. So the reload wipes the just-dispatched error and nothing is shown after it. The success path at line 185 does the right thing (refreshAfterMove({ notice })), which is why success survives the reload but failure does not. On a partial move (the assign loop is not wrapped in a transaction, so some items can move before one fails) the user is left thinking everything worked.

Fix: build the fail notice once and pass it through, so list view keeps it across the reload:

const failNotice = { message: error?.data?.message || error?.message || __( 'Failed to move items', 'godam' ), type: 'fail' };
dispatch( updateSnackbar( failNotice ) );
refreshAfterMove( { notice: failNotice } );

The grid path ignores the notice argument, so this only changes list-view behaviour.

2. The single-item "Folder" control only works in the upload.php modal

Two separate gaps, both confirmed against WordPress core:

• It does not render in the single-column details sidebar. renderMoveToFolderField requires a .settings element (assets/src/js/media-library/views/fields/move-to-folder-field.js:126), but core's single-column tmpl-attachment-details template has no .settings wrapper (only the two-column template does). So in the post editor "Add Media" / Featured Image sidebar, the Folder row never appears. Fix: fall back when .settings is absent, e.g. const settings = view.el.querySelector( '.settings' ) || view.el; and insert before .attachment-compat only when that child exists.
• Where it does render off the upload screen, it is unstyled. The JS bundle loads on post, page, attachment, widgets, and site-editor screens, but the stylesheet is enqueued only on the upload screen (inc/classes/class-assets.php:358). Fix: enqueue easydam-media-library style on the same screens the script loads (godam_should_load_media_library_assets( $screen )), or move those rules into a SCSS file imported by the media-library JS entry.

Please verify the control both appears and is styled in the post-editor media modal and the Edit Media screen, not just on upload.php.

3. Duplicate DOM id between the details control and the modal radios
pages/media-library/components/modal/MoveToFolderModal.jsx:207 and assets/src/js/media-library/views/fields/move-to-folder-field.js:26

Both build ids from the same godam-move-to-folder- prefix, but one is keyed by attachment id and the other by folder term id. Those are separate id sequences that overlap on real sites. Open attachment 42 in details (its trigger button becomes id="godam-move-to-folder-42"), open the picker, and if a folder with term id 42 exists, its radio and label also resolve to godam-move-to-folder-42. The details button sits earlier in the DOM, so clicking that folder row's label activates the details button (the first match) instead of selecting the radio, which re-opens the request and clears the chosen target. That one folder can only be picked by clicking the small radio circle directly.

Fix: give the modal radios their own prefix, e.g. godam-move-to-folder-target-${folder.id} for both id and htmlFor.

Nice to fix

4. Picking a search-only folder, then clearing the search, leaves an invisible selection
pages/media-library/components/modal/MoveToFolderModal.jsx

targetFolderId resets only when the modal opens, not when the search changes. Select a folder that is only reachable via search (its parent page is not loaded in the tree), then clear the search box: no radio is checked and the "Selected:" label disappears, but Move stays enabled and moves the items to that now-hidden folder. The destination is still the one the user chose, so this is a display mismatch rather than a wrong move. Fix: remember the chosen folder's name at selection time so the "Selected:" indicator and the Move button stay consistent after the search clears (rather than resetting, which would throw away a valid choice).

5. Rapid sidebar toggles can persist the wrong final state (matches the Copilot note)
pages/media-library/App.js:165

Each toggle fires an independent save with no ordering, so two quick toggles can commit out of order and leave the stored value disagreeing with what the user last chose. The live UI is always right; only the next page load's first paint reads the stale value, and the next toggle corrects it. Fix: debounce the save (a trailing 300ms is enough for one boolean) or drop stale responses.

6. Toolbar button leaks event handlers
assets/src/js/media-library/views/filters/media-move-to-folder.js:33

initialize() binds three handlers with this.controller.on(...) and never removes them. The button is rebuilt whenever the picker frame recreates its browser (for example switching Upload Files and back to Media Library), and Backbone's cleanup does not remove direct .on() handlers on another object, so handlers accumulate for the frame's lifetime. The neighbouring code already uses the safe pattern and says why. Fix: use this.listenTo( this.controller, ... ). The same pattern exists in media-retranscode.js and could be fixed alongside.

7. Snackbar can show twice when two sidebar roots are alive
pages/media-library/components/folder-tree/SnackbarComp.jsx

The toast now renders into document.body and reads the shared store without scoping to its own root, so a retained hidden frame (for example a block-editor or Elementor media frame kept in the DOM) can paint a second identical toast at the same position. Cosmetic only. Fix: scope the toast to the visible/owning root using the SidebarRootContext the bridge already threads.

Optional notes

• Tests: the new logic ships without any. The repo has both Jest and PHPUnit set up, and the team convention is to add tests for logic changes. Good candidates: id normalization in the bridge, the canMoveTo / count guard matrix in useMoveAttachments (with the id -1 and id 0 special cases), the list bulk-action parser, and Media_Library::update_sidebar_preference() (store on true, delete on false, and the upload_files gate).
• data-test-id: the new modal, three-dot toggle, and folder control carry none, while the rest of the media-library UI uses about 385 of them. Worth adding for the automation suite.
• Three-dot toggle accessibility: it can be closed from the keyboard (press Enter again on the button), so there is no trap, but it never sets aria-expanded and claims aria-haspopup="menu" for a popup that is not a real menu. Minor polish.
• Duplication: canMoveTo is returned by the hook but nothing outside it uses it, while the modal recomputes the same lock rule inline. Same result today; worth sharing one source so they cannot drift.

Checked and clean

For scope: the new REST endpoint is fine (gated on upload_files, writes only the current user's meta, casts the boolean, no cross-user read or write, no injection or escaping gaps); the boolean round-trips correctly through wp_localize_script; internationalization is complete with correct translator comments; the field-builder extraction into compat-fields.js and the drop-handler extraction into useMoveAttachments both preserve behaviour; the folder count math self-heals against the authoritative refetch; and the list bulk-action priority matches WordPress core's own top-over-bottom rule (one candidate issue there turned out to be a non-issue). The earlier Copilot note about a comment pointing at a sidebar-preference.js file is already fixed in the current diff.

[subodhr258]

Re-review

Since the last review only one commit changed this PR: 48ab1f9, the debounced sidebar save. That fix is correct in what it set out to do, but six of the seven items from the previous round are untouched, including the two functional gaps, and the debounce introduces one new way to lose the preference. Holding off on approval for those.

Fixed

Rapid sidebar toggles persisting the wrong final state (previous item 5, and the Copilot note). persistSidebarPreference now coalesces a burst into one write of the latest value and flushes on unmount. Good.

One small caveat: the comment says "only ever one request is in flight", which is not quite true. Two toggles more than 500ms apart still produce two independent, unordered POSTs. The window is now small enough that this is fine in practice, so this is only about the comment.

New, introduced by the debounce

A pending save is dropped when the page navigates within 500ms
pages/media-library/App.js:159

The flush only runs on React unmount, and a full page navigation never unmounts React. On upload.php?mode=list there is no .media-frame, so triggerFilterChange takes the URL branch and sets window.location.href. Collapse the sidebar and click a folder straight after: the timer dies with the page and nothing is saved. A list-view move takes the same path, since refreshAfterMove reloads.

Before this commit the POST went out immediately, so this is a regression in a fairly ordinary flow. Flush on pagehide, or keep the immediate write and discard stale responses instead (the other option from the last review).

Still open from the last review

1. A failed move in list view shows no error, and a partial move looks like a success
pages/media-library/hooks/useMoveAttachments.js:198

refreshAfterMove() in the catch block still takes no argument, so the reload discards the fail toast that was just dispatched. Build the notice once and pass it through.

2. The single-item "Folder" control still only renders in the two-column modal
assets/src/js/media-library/views/fields/move-to-folder-field.js:128

Confirmed again against core: .settings exists only in tmpl-attachment-details-two-column. The single-column tmpl-attachment-details puts its span.setting rows as direct children, so the early return fires and the row never appears in the post editor's Add Media / Featured Image sidebar. Worth flagging that the PR description says the control now appears "in both the two-column attachment details modal and the sidebar", and the SCSS added for .media-sidebar .setting.godam-move-to-folder-setting targets a row that is never created. Fall back to view.el when .settings is absent, and insert before .attachment-compat only when that child exists.

3. The stylesheet is still enqueued on the upload screen only
inc/classes/class-assets.php:359

The script loads wherever godam_should_load_media_library_assets() is true (post, page, attachment, widgets, site-editor), so the toolbar button is created on those screens without media-library.css. Enqueue the style on the same gate.

4. Duplicate DOM id between the details control and the picker radios
pages/media-library/components/modal/MoveToFolderModal.jsx:207

Both still use the godam-move-to-folder- prefix over two different id sequences, so a folder whose term id matches the open attachment's id cannot be picked by clicking its label. Give the radios their own prefix.

5. Picking a search-only folder, then clearing the search, leaves an invisible selection
pages/media-library/components/modal/MoveToFolderModal.jsx:118

6. The toolbar button still leaks event handlers
assets/src/js/media-library/views/filters/media-move-to-folder.js:33

Three this.controller.on(...) bindings with no teardown. bindEvents() in attachment-browser.js documents this exact trap ten lines away and uses this.listenTo.

7. The snackbar can show twice, and now so can the picker
pages/media-library/components/folder-tree/SnackbarComp.jsx:42

Worth upgrading from cosmetic. MoveToFolderModal reads the same shared store and is rendered by every mounted app, so with two live sidebars (two media frames on a post edit screen, or the 100ms window the modal-close cleanup leaves open) you get two stacked dialogs with two focus traps, not just a doubled toast. SidebarRootContext is already threaded to every app and already used by useMoveToFolderBridge; gate the modal and the toast on it too.

New, smaller

Breakpoint disagrees with the CSS at exactly 900px
pages/media-library/App.js:51, assets/src/js/media-library/index.js:71

innerWidth < 900 against max-width: 900px, which is inclusive. At exactly 900px the overlay layout renders but the code treats it as desktop, so the sidebar is not force-collapsed and expanding it overwrites the saved desktop preference, which is the one thing the desktop-only guard exists to prevent. The literal also sits in three places. window.matchMedia( '(max-width: 900px)' ) removes both problems.

Folder search caps at 20 results with no way to page
pages/media-library/components/modal/MoveToFolderModal.jsx:59

Pinned to page: 1, perPage: 20, and canLoadMore is false while searching. On a site with many folders a match that sorts twenty-first cannot be selected, and nothing tells the user results were truncated.

Duplicated checked-row selector
assets/src/js/media-library/list-view-bulk-actions.js:90

checkedIds() repeats the query getSelectedAttachmentIds() already does in data/media-grid.js:326. This file already imports across the bundle boundary, so the shared helper is reachable.

Unchanged notes

Still no tests for any of the new logic, and no data-test-id on the picker, the three-dot toggle or the Folder control. The three-dot button still declares aria-haspopup="menu" without aria-expanded, over a popup that has no menu roles. canMoveTo is still returned by the hook and unused outside it while the modal recomputes the same lock rule inline.

Checked and clean

The debounce itself, the develop merge, and everything I signed off last time still hold up: the REST endpoint stays gated on upload_files and only touches the current user's meta, the Media_Library import into class-assets.php does not collide with the RTGODAM\Inc\Media_Library namespace already used there, the compat-fields.js extraction is behaviour-preserving (Attachment.render replaces $el HTML on every render, so the new table reuse cannot accumulate rows), the toolbar priority reasoning at -72 is right, and the selection.reset() in refreshAfterMove only ever runs on paths that moved the whole selection, so it costs nothing. CI is green apart from the POT check, which is expected during development.