MODX 3.0.4-pl - Listbox Multiselect

[thorjoel]

Bug report

Summary

The order that the elements is chosen is saved, but when you refresh backend and look at the resource with the Template Variable the order is not shown.

Step to reproduce

Create a template variable with atleast two items. Use that template variable on a resource. Choose another order than the elements is showing in the list. Save. Refresh resource. Check template variable.

Observed behavior

The initial order is shown in the list, not the selected order.

Expected behavior

The selected order should be the order it is showing after a refresh. If the page is saved again, the initial order is saved and the elements are saved in "wrong" order.

Environment

MODX version 3.0.4-pl

[halftrainedharry]

In the MODx 2.x branch, there is this code in the listbox-multiple render to "preserve the order of selected values".

https://github.com/modxcms/revolution/blob/3f887dc6494fe32d3b2a52da20ec76a63bc8fb7d/core/model/modx/processors/element/tv/renders/mgr/input/listbox-multiple.class.php#L34-L47

---

In MODX 3, this code has been deleted in the PR #16242.

Now when the resource is loaded, the selected values are always ordered in the same order as the Dropdown List Options in the TV. (For example, if the Dropdown List Options are Bird||Cat||Dog, "Bird" will always be displayed first after a reload, if it was selected.)

[thorjoel]

Would that feature of the multiselect be added back in? We tend to use the order the customer is choosing to show the resources that way. The first time it is saved it is in the customers choice order. Then the customer edits the page and not reorder the select and then it becomes the default order.

[halftrainedharry]

Would that feature of the multiselect be added back in?

I suppose so. It seems to be an oversight, not a conscious decision to remove it.

---

To (temporarily) fix the issue, you could add this new code

// preserve the order of selected values
$orderedSelections = [];
// loop trough the selected values
foreach ($savedValues as $val) {
	// find the corresponding option in the selections array
	foreach ($selections as $item => $values) {
		// if found, add it in the right order to the $orderedSelections array
		if ($values['value'] == $val) {
			$orderedSelections[] = $values;
			// and remove it from the original $selections array
			unset($selections[$item]);
		}
	}
}
// merge the correctly ordered items with the remaining ones
$selections = array_merge($orderedSelections, $selections);

before this line in the existing code: https://github.com/modxcms/revolution/blob/73bfd2712427f46d9f32730b86f335fbe3296703/core/src/Revolution/Processors/Element/TemplateVar/Renders/mgr/input/listbox-multiple.class.php#L78

---

@smg6511 Can you maybe take a look at this, as you were the one who deleted the code?

[halftrainedharry]

A more elegant solution is probably to use a sort function:

$savedValuesIndexLookup = array_flip($savedValues);
usort($selections, function($a, $b) use ($savedValuesIndexLookup) {
	return $savedValuesIndexLookup[$a['value']] <=> $savedValuesIndexLookup[$b['value']];
});

[smg6511]

@halftrainedharry @thorjoel - Yes, I'll take a look soon. I won't be able to focus on this until the beginning of December. I did remove that code on purpose, but it's been quite a while since that change was made so I want to go over it carefully before re-introducing the sorting. I'd also look to make it a config option in the TV creation panel, rather than a default behavior.