#2858 - Bulk Edit - The Ability To Remove Tags, Connections Or Options

[kodinkat]

• Bulk Edit. The ability to remove a tag, connection, or option #2858

---

[github-actions]

PR Review: Bulk Edit - Remove Tags, Connections, or Options

Good feature addition that improves bulk edit UX by consolidating the clear/restore buttons into a single toggle and supporting selective value removal. Here are some issues worth addressing before merge:

---

Bug: Event Listener Accumulation

File: dt-assets/js/modular-list-bulk.js

In renderBulkEditField(), the toggle listener is registered via addEventListener directly on the DOM element:

modeToggle[0].addEventListener('change', function () {
  handleModeToggleChange(this);
});

The previous clear/restore buttons used jQuery delegated events ($(document).on('click', ...)) which are safe to re-register. This new approach adds a new listener every time renderBulkEditField is called for the same field key. If a user adds, removes, and re-adds the same field, handleModeToggleChange will fire multiple times per toggle change.

Consider switching to a delegated event via $(document).on('change', '.bulk-edit-mode-toggle', ...), or remove any existing listener before adding the new one.

---

Performance: Extra API Call Per Record for key_select/user_select

File: dt-assets/js/modular-list-bulk.js, do_each() (~line 196)

For every record being bulk-edited where a key_select or user_select field removal is involved, the code now issues a get_post call to fetch the current record before deciding whether to update it:

const updatePromise = window.API.get_post(list_settings.post_type, item)
  .then((currentRecord) => { ... });

This doubles the API request count for these field types. On a bulk edit of 100 records, that's 200 requests instead of 100. Consider whether the server-side update endpoint can handle a "remove only if value matches X" instruction, or accept an empty string without requiring a pre-fetch (since clearing a field that's already empty is a no-op from the user's perspective).

---

UX: Silent Behavior Change When No Values Are Selected

File: dt-assets/js/modular-list-bulk.js, handleModeToggleChange()

When the user toggles "Remove Values" ON but hasn't entered any values in the field, the code silently falls back to fieldData.cleared = true (full field clear):

} else {
  fieldData.cleared = true;
  fieldData.operation = undefined;
  // shows: "Field will be cleared/unset"
}

A user seeing a toggle labelled "Remove Values" who activates it with no selection will get a full clear instead of a no-op or an explanation. This should at minimum show a message explaining why the behavior changed (e.g., "No values selected — field will be fully cleared").

---

Code Smell: Metadata Mixed into the Update Payload

File: dt-assets/js/modular-list-bulk.js

Using _conditionalRemovals as a hidden metadata key inside the updatePayload object is fragile. The update object is passed around and expected to be a clean API payload. Stripping it in do_each() relies on callers not passing update to the API before the deletion occurs.

A cleaner approach would be to pass conditionalRemovals as a separate parameter to do_each() / process(), keeping it structurally separate from the API data.

---

Orphaned Comment

File: dt-assets/js/modular-list-bulk.js (~line 1435 in the original, now just before /** Field Rendering */)

// Update clear button labels when field values change
/**
 * Field Rendering
 */

This is a stray comment left over from refactoring. It should be removed.

---

Potentially Unused Translation

File: dt-assets/functions/enqueue-scripts.php

'remove_values' => __( 'Remove Values', 'disciple_tools' ),

The toggle label is rendered server-side in archive-template.php using esc_attr_e(), which is the correct approach. The JS-side translation list_settings.translations.remove_values does not appear to be referenced anywhere in the JS. If it is not used, it can be dropped. If it will be used in future JS, a comment explaining why would help.

---

Minor: Typeahead Always Destroyed and Re-created

File: dt-assets/js/modular-list-bulk.js, initializeBulkEditFieldHandlers()

The original code guarded typeahead initialization with:

if (userInput.length && !window.Typeahead[`.js-typeahead-${fieldId}`]) {

The new code always destroys and re-creates the instance. This is needed for the restore-from-remove-mode path, but it means every normal initialization also goes through the destroy step (which is a no-op if nothing exists, but adds unnecessary branching). A comment explaining why the guard was removed would help future maintainers.

---

Overall the approach is sound and the field type coverage (multi_select, tags, connection, key_select, user_select) is comprehensive. Addressing the event listener accumulation bug and the performance concern are the most important items before merge.