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

Reference: https://www.drupal.org/core/beta-changes
Issue category Task
Issue priority Not critical
Prioritized changes The main goal of this issue is reducing fragility by removing a 'temporary hack'.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quicksketch’s picture

Status: Active » Needs review
FileSize
876 bytes

Here's a patch that removes the TODO and unneeded comments.

quicksketch’s picture

Issue summary: View changes
jayeshanandani’s picture

Working on adding a change record.

ekl1773’s picture

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

jhedstrom’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Patch 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).

googletorp’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
854 bytes

Rerolled the patch.

jhedstrom’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs change record

I think this is good to go.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Shouldn't we also be documenting the new property somewhere in the code base?

googletorp’s picture

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

googletorp’s picture

Status: Needs work » Reviewed & tested by the community

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

jhodgdon’s picture

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

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 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 :)

  • alexpott committed 0f1a044 on 8.0.x
    Issue #2208585 by googletorp, quicksketch: Make #wrapper_attributes...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

quietone’s picture

publish change record