Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MODX 3.0.4-pl - Listbox Multiselect #16495

Open
thorjoel opened this issue Nov 10, 2023 · 5 comments · May be fixed by #16561
Open

MODX 3.0.4-pl - Listbox Multiselect #16495

thorjoel opened this issue Nov 10, 2023 · 5 comments · May be fixed by #16561
Assignees
Labels
bug The issue in the code or project, which should be addressed.
Milestone

Comments

@thorjoel
Copy link

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

@thorjoel thorjoel added the bug The issue in the code or project, which should be addressed. label Nov 10, 2023
@thorjoel thorjoel changed the title Listbox Multiselect MODX 3.0.4-pl - Listbox Multiselect Nov 10, 2023
@halftrainedharry
Copy link
Contributor

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

// preserve the order of selected values
$orderedItems = array();
// loop trough the selected values
foreach ($value as $val) {
// find the corresponding option in the items array
foreach ($items as $item => $values) {
// if found, add it in the right order to the $orderItems array
if ($values['value'] == $val) {
$orderedItems[] = $values;
// and remove it from the original $items array
unset($items[$item]);
}
}
}


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
Copy link
Author

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
Copy link
Contributor

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:


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

@halftrainedharry
Copy link
Contributor

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
Copy link
Collaborator

smg6511 commented Nov 13, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue in the code or project, which should be addressed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants