-
-
Notifications
You must be signed in to change notification settings - Fork 529
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
View values in TV listbox-multiple then it manually entered #16241
base: 2.x
Are you sure you want to change the base?
Conversation
What does it do? - Displays values manually entered by the manager in the admin panel. For MODX 2.x Why is it needed? - It is necessary to fix the display problem. Related issure(s): modxcms#15136
Thanks for helping to make MODX better. This fix should check, wether the TV option forceSelection is deactivated and not add the unsupported values then. Otherwise the patch looks ok. What happens if an unsupported value is in the TV and the TV is saved? |
I just put up a PR to fix this in 3.x. Take a look (see #16242), as it may inform how you approach this. I believe the un-patched file is virtually the same for 2.x and 3.x, so you may be able to use what I've done if you like. The one thing I see (that would need fixing) is that you probably end up duplicating selected values in the list when they are among the pre-defined input options values. |
It is stored normally. It is not duplicated in the Database. Also, there is no duplication of the value, JS does not allow this to be done in the admin panel. And even if it is possible to write the value, then after saving and when updating the page (F5), the data is displayed correctly.
Duplication of the value does not occur. Js does not allow this to be done in the admin panel. I also thought that it would be duplicated, but as practice has shown, this does not happen, because unique values come to be recorded in the field DB. |
$orderedItems[] = array( | ||
'text' => $val, | ||
'value' => $val, | ||
'selected' => 1, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small change to treat the custom input values the same as the pre-defined ones (htmlspecialchars). Also, I encourage the modernization of any passage of code you add or edit (here, use the short array declaration using only brackets).
$orderedItems[] = array( | |
'text' => $val, | |
'value' => $val, | |
'selected' => 1, | |
); | |
$val = htmlspecialchars($val, ENT_COMPAT, 'UTF-8'); | |
$orderedItems[] = [ | |
'text' => $val, | |
'value' => $val, | |
'selected' => 1 | |
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this solution and it does fix the issue. The solution I created for 3.x could also just be dropped in, as the class definitions are exactly the same (comparing the original 3.x one before my merged changes and the 2.x one being changed here). Honestly, it's been too long for me to fully remember the motive for my 3.x approach although I do remember finding the original code somewhat hard to follow and wanted to make the process more immediately readable.
My first choice would be to drop in the 3.x solution, but I'd also approve this one with the more minor change I specified above.
What does it do?
Why is it needed?
How to test
Describe how to test the changes you made.
Related issue(s)/PR(s)
#15136