@Harmageddon with the last 3 commit i have just fixed a error from drone and minimal cs ;)

@zero-24 Awesome, thank you! Didn't know about this macro, very useful!

Added some small code cleanups, inspired by @matrikular. Thank you!

Nice pr. Can you please fix the merge conflicts that we can test it?

@laoneo Thank you! It should work now.

Thanks, added a few comments to use the namespaced classes. If done I will test it.

Done. Thanks in advance!

Now you got me wrong, use the real classes like \Joomla\CMS\Form\Form, etc. Not just adding a backslash before them.

Oh, sorry. Should I copy the rule to libraries/src/Form/Rule, too, then? Or does it work with this "mixed" form (the subform field and the rule in libraries/joomla/form and the form and the JFormFieldRule class in libraries/src/Form)?

Yes please copy the rule also to libraries/src/Form/Rule. The field has to stay because of autoloading. In Joomla 4 we have moved it also to libraries/src/Form/Field.

Okay, a little bit confusing. I hope I haven't messed up anything.

I have tested this item ✅ successfully on d379667

It worked. I'v tested it by changing the file plugins/fields/checkboxes/params/checkboxes.xml to

<?xml version="1.0" encoding="utf-8"?>
<form>
	<fields name="fieldparams">
		<fieldset name="fieldparams">
			<field
				name="options"
				type="subform"
				label="PLG_FIELDS_CHECKBOXES_PARAMS_OPTIONS_LABEL"
				description="PLG_FIELDS_CHECKBOXES_PARAMS_OPTIONS_DESC"
				layout="joomla.form.field.subform.repeatable-table"
				icon="list"
				multiple="true"
				filter="subform"
				validate="subform"
			>
				<form hidden="true" name="list_templates_modal" repeat="true">
					<field
						name="name"
						type="text"
						label="PLG_FIELDS_CHECKBOXES_PARAMS_OPTIONS_NAME_LABEL"
						size="30"
					/>

					<field
						name="value"
						type="text"
						label="PLG_FIELDS_CHECKBOXES_PARAMS_OPTIONS_VALUE_LABEL"
						size="30"
						validate="color"
					/>
				</form>
			</field>
		</fieldset>
	</fields>
</form>

After that I created a checkbox field and tried to create an option where the value is not a hex number. It failed. The I changed validate="color"to filter="int" and tried to save again, then it successfully saved it but turned the value into an integer.

I have tested this item ✅ successfully on 6b79cea

@laoneo can you please retest?

I have tested this item ✅ successfully on 6b79cea

Perhaps you should consider to make your own callback for filter instead of hardcoding the subform filter into the form class. More details can be found here #20569 (comment).

Are there any other filters that could serve as an example? I assume that I'd have to create a new class? Where should this class be located? As far as I see, most core rules (like "access") are directly in the Form class.

I would use the method you suggest only for third-party extensions and keep the core filters in one place - either in the Form class as of now or each one in a dedicated class. If we keep some filters in one and others in another place, we risk inconsistencies (i.e. if the filter mechanism gets changed) and people will have a harder time to find all available filters when digging through the code. Personally, I'd support your approach to create classes for filters, because I remember debugging the filter method until I found the hardcoded filters in the Form class. But I'd prefer to either move all or none.

What is hardcoded into the Form class' filterField method is really a bunch of special cases specific to the form data that don't belong to the InputFilter class. So, "core filters" don't need to be hardcoded into the Form class itself unless there is a strong reason why it must be part of the Form API and available at all times.

Okay... So is there any best practice where else to put such a filter? And if I see this right, we'd then require users/developers to use a filter="SomeHelperClass:subform" instead of just filter="subform"?

IMO, form field filtering and validation should be implicit whenever possible to reduce XML form maintenance needs.

So Form's validateField and filterField logic could be moved to FormField. Subform field could then override parent methods and other fields could declare default filters and validation rules to be used when none is added in XML.

Here's a mockup based on your code and #12414 SharkyKZ/joomla-cms@fa514ae.

So Form's validateField and filterField logic could be moved to FormField. Subform field could then override parent methods and other fields could declare default filters and validation rules to be used when none is added in XML.

Not necessarily, because each part of the API has a separate concern.

FormField is in essence the visual representation of the field and handles the rendering, the class chain does not belong in the filtering/validation process at all.

FormRule defines some validation rules to be applied when validating the form's data, these are called during the validation process only.

Form is the data manager for a form instance and this is where the filtering and validation processes belong.

That's the way it is now but is it really correct?

FormField is in essence the visual representation of the field and handles the rendering, the class chain does not belong in the filtering/validation process at all.

Form fields also provide the data options and data type. So filtering and validation makes sense here. Filtering and validation is applied per-field anyways. That said, shouldn't FormRule be FormFieldRule in the first place?

List-based fields should be validated against options (except when custom option is allowed), numeric fields should be filtered accordingly and Subform should not accept other filters but its own (currently, adding a filter like string to subform field causes fatal error).

I think this functionality is vital and should be implemented one way or another.

Rewrite the entire form package from scratch then? There is already a separation of concern regarding each of the object types in the package and an established pattern of use, if you feel this is inefficient feel free to propose a new package but the existing class chain can't just be changed out of the blue like this. By design, filtering and validation are not the responsibility of the FormField classes and refactoring to move responsibilities like this is no small thing. At the end of the day though, regardless of the class structure, what is responsible for rendering should be a different aspect of the API than what is responsible for data validation (if you look at any proper framework you'll find they have a standalone validation component that the form data is pushed through whereas the closest thing we have to that is the FormRule class and that is tightly coupled to the Form class) and if we're going to start talking refactoring the form API then I don't think we should be making one sort of "super" object that does all the things.

@mbabker Is this something for 3.9?

So with this PR being marked as RTC for almost 4 months now, I'd like to ask what is the status for this one? Is it going to be merged for some future release in the current state or do you prefer me to move the filter to some different location for architectural reasons?
@mbabker @wilsonge (pinging you as the release leads for 3.9/3.10)

I'm hesitant to merge this because I don't think it is architecturally done correctly and inherently once it ships changing the API will never happen.

I see. However, the problem with moving this method outside the Form class is, that we need the form field ($element) in order to load the subform and get the filters for the respective fields inside the subform. Callback filters as well as the InputFilter->clean function only get the value of the field as an argument, but not the whole field.
So a callback filter would get an array containing the values of the fields inside the subform without knowing which value to treat with which filter.

Merged staging to include the changes from #17552.

@mbabker With the current API, I don't see any alternative to this implementation.

(sorry, I accidentally hit the wrong button)

Please see #12414. That might have an impact on this PR.

Thank you for your work there! It does indeed have an impact. I'm going to rewrite this PR for 4.0 then. And as this PR here probably won't be ever merged into 3.x after being RTC for 8 months, I'm now closing it and hope for better luck and API integration for 4.0.

Just for the record I'd definitely be interested in seeing this for Joomla 4!

Apparently, the changes proposed in this PR have been added in b06019e and d34a3ec, so I assume they're going to be merged into 4.0, too, and there is no need for a further PR?
@HLeithner / @wilsonge what do you think?

Correct. All 3.x changes are merged up to 4.0 before the final release.