Refactor data attributes for form fields#29252
Conversation
Co-authored-by: Quy <quy@fluxbb.org>
layouts/joomla/form/field/user.php
Outdated
| <?php echo $dataAttribute; ?>> | ||
| <div class="input-group"> | ||
| <input <?php echo ArrayHelper::toString($inputAttributes); ?> readonly> |
There was a problem hiding this comment.
This was on <input> element before.
There was a problem hiding this comment.
I know but I think it should be on the joomla wc or I'm wrong?
There was a problem hiding this comment.
Fancy select is on the input within the WC too. Hard to say which is better - there's an argument for adding support to both to be honest. Sometimes you want fill out the actual field value (i.e. the input), sometimes you want add properties to the WC for custom JS logic
There was a problem hiding this comment.
Sometimes you want fill out the actual field value (i.e. the input), sometimes you want add properties to the WC for custom JS logic
This is very true. The reason you probably want to also fill a nested input in a custom element is to cover forms where the data is populated and the JS is disabled. If there is an input type=submit then the form will be submitted but if the input wasn't there then the backend will not get that particular input data which will lead to undesired behaviour. Edge case? Well, I don't know, I thought the core output should be consistent for all possible cases. Even if the core doesn't even use any such case. The Form Fields should be as close to metal as possible they're fundamental building blocks so their support should be solid. At least that was my approach, you can ditch it and dictate things, I mean Joomla is already doing this for the Form fields as they're still hardcoded to Bootstrap and Font Awesome which is definitely wrong if the aim is broad support...
In this case
<input <?php echo ArrayHelper::toString($inputAttributes); ?> <?php echo $dataAttribute; ?> readonly>
seems the change that could be merged without any testing
There was a problem hiding this comment.
Ok I moved it to the input, but if js disabled you can't use it in the input anyway.
|
Thanks! |
Reduced duplicated code introduce by #27212 and added missing Fields.
Summary of Changes
Added a data render function to formfield
Refactor layouts to use preprocessed variable
Added missing fields
Testing Instructions
Code Review and check if Joomla forms till work especially the new fields.
Expected result
Works
Actual result
Works