NPM Resource Changed ? ? Pending

User tests: Successful: Unsuccessful:

avatar stephan-ansems
stephan-ansems
15 May 2022

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.

avatar stephan-ansems stephan-ansems - open - 15 May 2022
avatar stephan-ansems stephan-ansems - change - 15 May 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 15 May 2022
Category Administration com_config Repository NPM Change JavaScript Layout Libraries
avatar stephan-ansems
stephan-ansems - comment - 15 May 2022

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.

avatar richard67 richard67 - change - 15 May 2022
Title
mplement 'requireon' attribute to form fields for conditional requiredness
Implement 'requireon' attribute to form fields for conditional requiredness
avatar richard67 richard67 - edited - 15 May 2022
avatar richard67
richard67 - comment - 15 May 2022

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

avatar dgrammatiko
dgrammatiko - comment - 15 May 2022

@stephan-ansems out of curiosity: why didn't you extended the existing showOn functionality and duplicated the whole stack just for the extra attribute?

avatar rdeutz
rdeutz - comment - 15 May 2022

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

avatar stephan-ansems stephan-ansems - change - 15 May 2022
Labels Added: NPM Resource Changed ?
avatar stephan-ansems
stephan-ansems - comment - 15 May 2022

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

avatar richard67
richard67 - comment - 16 May 2022

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

avatar richard67
richard67 - comment - 18 May 2022

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

avatar joomla-bot
joomla-bot - comment - 27 Jun 2022

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

avatar HLeithner HLeithner - change - 27 Jun 2022
Labels Added: ?
avatar HLeithner
HLeithner - comment - 2 May 2023

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

avatar HLeithner
HLeithner - comment - 30 Sep 2023

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

avatar HLeithner
HLeithner - comment - 24 Apr 2024

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

avatar HLeithner HLeithner - change - 24 Apr 2024
Title
Implement 'requireon' attribute to form fields for conditional requiredness
[5.2] Implement 'requireon' attribute to form fields for conditional requiredness
avatar HLeithner HLeithner - edited - 24 Apr 2024

Add a Comment

Login with GitHub to post a comment