joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

Implement 'requireon' attribute to form fields for conditional requiredness

Open stephan-ansems opened this issue 3 years ago • 8 comments

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 parseShowOnConditions to parseFieldConditions
  • new javascript asset requiron to perform the client-side magic
  • Addition of the requireon attribute and implementation in all places where the showon is 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.

stephan-ansems avatar May 15 '22 10:05 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.

stephan-ansems avatar May 15 '22 10:05 stephan-ansems

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

richard67 avatar May 15 '22 11:05 richard67

@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 avatar May 15 '22 11:05 dgrammatiko

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

rdeutz avatar May 15 '22 11:05 rdeutz

@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 avatar May 15 '22 19:05 stephan-ansems

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

richard67 avatar May 16 '22 09:05 richard67

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

richard67 avatar May 18 '22 10:05 richard67

This pull requests has been automatically converted to the PSR-12 coding standard.

joomla-bot avatar Jun 27 '22 20:06 joomla-bot

This pull request has been automatically rebased to 5.0-dev.

HLeithner avatar May 02 '23 16:05 HLeithner

This pull request has been automatically rebased to 5.1-dev.

HLeithner avatar Sep 30 '23 22:09 HLeithner

This pull request has been automatically rebased to 5.2-dev.

HLeithner avatar Apr 24 '24 09:04 HLeithner