joomla-cms
joomla-cms copied to clipboard
[5.3] [RFC] Submit an empty value when nothing is selected in list and checkbox(-es) fields
Summary of Changes
The PR adds a hidden input to allow to submit an empty value when nothing is selected in list (multiple) and checkbox(-es) fields. Because browser does not submit anything when <select multiple> is empty, and <input type="checkbox"> is unchecked.
For now Developers had to use extra check in their Table or Model, however suggested fix should simplify such things.
Testing Instructions
Install testing component com_testi-1.0.0.zip Create a new Item with some values in checkboxes and list. Then try uncheck all and remove all selected values, and save again.
Actual result BEFORE applying this Pull Request
After save the previous valueas are shown as checked/selected.
Expected result AFTER applying this Pull Request
After save everything stays unchecked/unselected.
Link to documentations
Please select:
- [ ] Documentation link for docs.joomla.org:
- [ ] No documentation changes for docs.joomla.org needed
- [ ] Pull Request link for manual.joomla.org: TBD
- [ ] No documentation changes for manual.joomla.org needed
I have tested this item :white_check_mark: successfully on dd5e4ac34fd2ccb0efa0ba32de9df9144892a521
Works as described!
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43905.
I have tested this item :white_check_mark: successfully on dd5e4ac34fd2ccb0efa0ba32de9df9144892a521
I tested this PR successfully. I experienced this issue in some of my own custom components, and I'm glad that there's now a solution for that.
Thanks for solving this @Fedik !
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43905.
@pe7er For setting RTC it is not enough to just add the label on GitHub. It needs to change status in the issue tracker. I will do that now.
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43905.
I have tested this item :white_check_mark: successfully on a434358f92c1454578d9dd7e1227e1ffed84c758
I tested this and it worked.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43905.
This pull request has been automatically rebased to 5.3-dev.
I like the idea, but I fear that this will break existing extensions as it comes all of the sudden with an empty string. Stuff like if(empty($data['com_fields]['radio'])) will not be true anymore as it contains an array with an empty element. It needs an additional array_filter call then.
@laoneo Shall I remove RTC?
I would say so till we get confirmation that I'm wrong.
Back to pending. See previous comments.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43905.
@laoneo do you actualy tested it, or it just your theory?
Please test first, it nothing to do with your "radio buttons" example. Thank you.
@laoneo do you provide a proof/example that the PR is breaking something? Otherwise please set it back to RTC.
@Fedik I will have a deeper look ASAP. But it is clear that an extra value is sent to the server. Give me some time to proove my statement.
Is the Postgresql test error related, Screenshot
There some issue with test, I think. But not related to changes in this PR. From screenshot it is clear that the categories are there, but the Test does not see them https://github.com/joomla/joomla-cms/blob/1811f78f8d466cf58366bd97b564cacedda6bc98/tests/System/integration/site/components/com_content/Categories.cy.js#L11-L16
The checker should probably wait when page fully rendered before doing DOM query. But it just my guess.
UPD, on screenshoot the categories in diferent order than expected by test.
I had a look and the returned data when no value is selected (I tested it with list). Then it is an empty string and not an empty array. As an array is expected, it is very likely that it gets casted to an array and then you end up with an array which has one entry, the empty string.
I'm aware that this is an edge case, but a list field is widely used nowadays and I fear this will break stuff.
I see. Thanks for checking. To me it is a bug in data handling by the "list" custom field. The data should be filtered instead of blind cast to array.
I will look, maybe will make it optional, or will change to 6.0.
We already use the same approach for subform (multiple) field for years, and it works fine. It is realy a pain to handle this input behaviors.
And because we have it since ages, it is really hard to change the behavior without breaking stuff into the wild.
I have update the code. Now it is configurable, and disabled by default. Testing component also updated.
Based on what for a criteria should the end user enable or disable the flag?
When developer want to submit and save empty value. Because browser does not submit any value for these cases.
Check the component I made for testing, try edit administrator/forms/item.xml and remove these flag.
I think you never use these fields before, in your projects, did you? :)
By default these fields just unusable without extra work. This PR is fixing it.
Ah sorry, I was on custom fields, but you added the option only to the form elements.
This pull request has been automatically rebased to 6.0-dev.
This pull request has been automatically rebased to 6.1-dev.