[#37798] - [5.2] Implement 'requireon' attribute to form fields for conditional requiredness
- Closed
- 2 Aug 2024
- Medium
- Build: 4.2-dev
- # 37798
- Diff
- stephan-ansems:4.2-dev
User tests: Successful: Unsuccessful:
Pull Request for a new Feature
Summary of Changes
Implementation for an new attribute requireon to the form field spec to that accepts conditional statements identical to that of the showon attribute. However the effect is to toggle the required attribute on form fields and show/hode the little star next to the label that indicates that the field is required.
Code changes:
- small refactoring of FormHelper method
parseShowOnConditionstoparseFieldConditions - new javascript asset
requironto perform the client-side magic - Addition of the
requireonattribute and implementation in all places where theshowonis also present
Why is this useful?
Some argue that having to toggle the requiredness of a field is a design flaw in the form, however I believe that it is not. For example a form to specify OAuth2 settings could have a dropdown to indicate the grant type. Depending on the grant type chosen, the form can change quite drastically. Some fields are not relevant for certain grant types and can be hidden, others are no longer required. The problem with only showon is that you can hide these fields, but a hidden field still remains required and form validation will not pass succesfully.
Testing Instructions
Create a form with a listfield with two options and a field that has the new attribute, for example this content:
<?xml version="1.0" encoding="utf-8"?>
<field
name="foo"
type="list"
>
<option value="1">MANDATORY</option>
<option value="0">OPTIONAL</option>
</field>
<field
name="bar"
type="text"
requireon="foo:1"
/>
</form>Open the form and change the selection of the foo field and see the bar field become mandory or optional.
Actual result BEFORE applying this Pull Request
Any form field that uses the requireon attribute remain required or optional depending on value of the required attribute. Joomla ignores the extra attribute, but adding the attribute to fields before this feature is implemented makes no sense.
Expected result AFTER applying this Pull Request
Any form fields that have the requireon attribute will be conditionally required when the condition evaluates to true. The logic works the same as for the showon attribute.
Documentation Changes Required
(https://docs.joomla.org/Form_field#Modal_form_field_types) can probably be documented in one fell swoop with the ShowOn property explanations.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_config Repository NPM Change JavaScript Layout Libraries |
| Title |
|
||||||
@stephan-ansems I think renaming that public method is a b/c break and so maybe not a good idea.
It also seems to break the installation form, at least the automated test break when making a new installation.
Have you tested that, make a new Joomla installation with your changed code? Or in an existing installation, change something in Global Configuration in backend and try to save the changes?
@stephan-ansems out of curiosity: why didn't you extended the existing showOn functionality and duplicated the whole stack just for the extra attribute?
@stephan-ansems I think renaming that public method is a b/c break and so maybe not a good idea.
@richard67 is right, renaming makes sense but you need to add the old function calling the new function. The old function should be depreciated 5.0. That should fix the testing problem for now. But we need to change the tests and replace all the calls to the old function.
Good addition btw.
| Labels |
Added:
NPM Resource Changed
?
|
||
@stephan-ansems out of curiosity: why didn't you extended the existing showOn functionality and duplicated the whole stack just for the extra attribute?
@dgrammatiko I don't see how i could do that. This seemed like the most obvious solution. Also I think that requireon and showon are not always linked, so hiding a field may influence the requiredness of it, but the reverse is not true.
@stephan-ansems If you think the PR is ready for being tested, it needs you to click the "Ready for review" button at the bottom of the PR on GitHub to remove the "Draft" status.
@stephan-ansems It does not need to update your branch every time GitHub shows that the branch is not up to date. It needs to do that only when GitHub shows in addition that there are conflicting files, which is not the case here.
This pull requests has been automatically converted to the PSR-12 coding standard.
| Labels |
Added:
?
|
||
This pull request has been automatically rebased to 5.0-dev.
This pull request has been automatically rebased to 5.1-dev.
This pull request has been automatically rebased to 5.2-dev.
| Title |
|
||||||
Hey @stephan-ansems, could you update this PR and solve the file conflicts? I'd like to merge this into 5.2 and finally solve this. Sorry that this is taking so long.
| Labels |
Added:
Feature
Updates Requested
PBF
PR-5.2-dev
Removed: ? ? |
||
I've solved the file conflicts.
I've solved the file conflicts.
can you also fix the phpcs issues please
I am sorry, this PR should be closed.
It is hack around of the limitation of showon feature.
We better should look how to improve showon feature, than continue hacking around.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2024-08-02 08:22:27 |
| Closed_By | ⇒ | stephan-ansems |
To any of the experienced Joomla developers, i'm learning how to do this and I appologize for things that are not according to guidelines or procedures. Please help me learn them.