As part of #1838114: Change node form’s vertical tabs into details elements, the new FAPI property #wrapper_attributes was added to theme_form_element(). However, it was added with the comment // Temporary hack for #type 'item'
.
Since #wrapper_attributes was added, it has been used as though it were official throughout core. There are currently 54 instances of the string "#wrapper_attributes" in core, many of which are legitimate uses of the property. The new addition is indeed useful to add things like classes to the wrapper around form elements (especially when working with radio buttons or checkboxes). We should remove the statement of it being a "temporary hack" and make it official. In addition, this new property needs to be documented through a change record.
Beta phase evaluation
Issue category | Task |
---|---|
Issue priority | Not critical |
Prioritized changes | The main goal of this issue is reducing fragility by removing a 'temporary hack'. |
Comment | File | Size | Author |
---|---|---|---|
#6 | make-2208585-6.patch | 854 bytes | googletorp |
#1 | wrapper_attributes-2208585.patch | 876 bytes | quicksketch |
Comments
Comment #1
quicksketchHere's a patch that removes the TODO and unneeded comments.
Comment #2
quicksketchComment #3
jayeshanandani CreditAttribution: jayeshanandani commentedWorking on adding a change record.
Comment #4
ekl1773Thanks for getting the change notice started, @jayeshanandani.
Could you add specific examples of how to use #wrapper_attributes?
The description could explain what the change accomplishes, and the examples can show exactly how it changed.
Here are the instructions for creating a change record, for future reference.
Comment #5
jhedstromPatch no longer applies, but this still an issue. Change record exists, but will need to be published once this lands. (Found by way of #2296873: Really old draft change notices exist).
Comment #6
googletorp CreditAttribution: googletorp as a volunteer commentedRerolled the patch.
Comment #7
jhedstromI think this is good to go.
Comment #8
alexpottShouldn't we also be documenting the new property somewhere in the code base?
Comment #9
googletorp CreditAttribution: googletorp as a volunteer commentedI talked with alexpott about this.
Basically what we need is to update the documentation for https://api.drupal.org/api/drupal/developer!topics!forms_api_reference.h...
The patch itself is still good. I have no idea how this is done, but jhodgdon should know, so I'm trying to point her in this direction from IRC.
Comment #10
googletorp CreditAttribution: googletorp as a volunteer commentedI talked with alexpott about this issue.
So since the documentation is not part of the patch, the patch is RTBC. We still need to update the documentation on the special form api reference page, after this is committed.
Comment #11
jhodgdonThe Form API reference file is in the Documentation project git repository.
So you should file a separate issue and make a patch on that. Put the issue in project "Documentation", component "API Documentation files", and please add a tag "FAPI reference" to it as well.
Unfortunately this file is really painful to edit. We are planning to get rid of it sometime, but this plan has been stalled for approximately 7 years now. Sorry.
Comment #12
alexpottCommitted 0f1a044 and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation - I agree with the fact that we should be removing the "temporary hack" comment since it no longer is :)
Comment #15
quietone CreditAttribution: quietone at PreviousNext commentedpublish change record