I accidentally stumbled over two of hook_field_extra_field()'s undocumented return value properties: "edit" and "delete", which can contain markup that is used for their respective operations fields in the "Manage fields" administration interface of the entity those properties' field belongs to.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xano’s picture

FileSize
1.11 KB

The attached patch documents these properties and adds an example (albeit a crappy one) to the hook definition. The example shows how the properties can be used, but the delete link may be somewhat unrealistic, just like the URL path used there.

jhodgdon’s picture

Where/how did you accidentally stumble upon this? In order to review the patch, I'd need to look at that too... :)

Xano’s picture

Ooh, that's a devilish amount of test passes!

The properties are used in field_ui_field_overview_form(). You can search for $extra_field['edit'] to quickly get to the lines in question.

jhodgdon’s picture

Status: Needs review » Needs work

Confirmed, thanks! But it looks like these 'edit' and 'delete' properties only belong with forms, not with display modes. The patch needs to make this clear, and I think rather than saying "a piece of markup", it should say something like "An HTML link to the...". Also I think it should say this is optional, and should only be used if someone actually *can* edit/delete this extra field (I think most extra fields cannot be edited or deleted?).

Also, for display modes, there is another element too:
http://api.drupal.org/api/drupal/modules!field!field.info.inc/function/_...
This adds in 'visible' for displays that are not defined, and this is used in:
http://api.drupal.org/api/drupal/modules!field_ui!field_ui.admin.inc/fun...

Xano’s picture

Thanks for the feedback!

I think rather than saying "a piece of markup", it should say something like "An HTML link to the...".

I disagree. The docs should explain the allowed value, and not the most conventional value. If someone has a use case for a piece of markup that is not a link, they should know that's technically possible.

I'll reroll the patch later this week :)

jhodgdon’s picture

OK, but please don't use "a piece of markup". How about "A string containing markup... normally a link" or something like that to give a clue to the programmer who might be trying to use this? :)

jhodgdon’s picture

Issue tags: +Novice

Finalizing this would be a good Novice project I think?

Xano’s picture

It would :) Sorry I haven't responded. My laptop recently broke and my backup is slow and has a crappy dev setup, so I'm basically postponing this kind of work until my main laptop is back.

bartmcpherson’s picture

Status: Needs work » Needs review
Issue tags: +SprintWeekend2013
FileSize
777 bytes

Looks like the 'visible' property has already been added to 8.x version of the function.
I rerolled the patch for the current 8.x-dev code base and modified the description of each value a bit.

Status: Needs review » Needs work
Issue tags: -Novice, -SprintWeekend2013

The last submitted patch, 1614108_09.patch, failed testing.

bartmcpherson’s picture

Status: Needs work » Needs review
Issue tags: +Novice, +SprintWeekend2013

#9: 1614108_09.patch queued for re-testing.

jhodgdon’s picture

Status: Needs review » Needs work

OK... Can we standardize on either "component" or "element" in this hook doc? Half of the entries say one and half the other, and the paragraph above (the start of the @return description) uses both too... The whole thing is kind of confusing -- maybe it is appropriate for some to say component and some to say element, but I don't get it. Documentation should be clear. :)

And then one nitpick in the patch: Move the "Only for..." up to the previous line and wrap as close to 80 characters as possible. Thanks!

And the "optional" things would be better if they followed our standard list syntax:
http://drupal.org/coding-standards/docs#lists

Thanks!

bartmcpherson’s picture

Status: Needs work » Needs review
FileSize
1.48 KB

Updated 'component' entries to 'element'. Element seems to be more consistent with other documentation in the code and code variables themselves.
Added (optional) to the start of the entries.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

That looks good, thanks!

jhodgdon’s picture

Assigned: Xano » jhodgdon

There's a big "avoid commit conflicts" issue that also touches field.api.php so we'll have to wait on committing this one until
#1380720: Rename entity "bundle" to "subtype" in the UI text and help
is done.

xjm’s picture

Issue tags: -Novice, -SprintWeekend2013

#13: 1614108_13.patch queued for re-testing.

bartmcpherson’s picture

#13: 1614108_13.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1614108_13.patch, failed testing.

bartmcpherson’s picture

Status: Needs work » Needs review

#13: 1614108_13.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1614108_13.patch, failed testing.

bartmcpherson’s picture

Status: Needs work » Needs review
Issue tags: +Novice, +SprintWeekend2013

#13: 1614108_13.patch queued for re-testing.

bartmcpherson’s picture

Status: Needs review » Reviewed & tested by the community
jhodgdon’s picture

Version: 8.x-dev » 7.x-dev
Assigned: jhodgdon » Unassigned
Status: Reviewed & tested by the community » Patch (to be ported)

Committed to 8.x, thanks again!

I think we need to backport this to 7.x and also document the 'visible' property in 7.x (see #4 and #9 for discussion).

dcam’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.54 KB

Backported #13 and added the 'visible' property to D7.

jhodgdon’s picture

Assigned: Unassigned » jhodgdon
Status: Needs review » Reviewed & tested by the community

Looks good, thanks! I'll get this committed shortly.

jhodgdon’s picture

Assigned: jhodgdon » Unassigned
Status: Reviewed & tested by the community » Fixed

Thanks again -- committed to 7.x.

David_Rothstein’s picture

Status: Fixed » Needs review
+ *   - visible: The default visibility of the element. Only for 'display'
+ *     context.

I don't think this exists in Drupal 7... does it? If it does, tell the people at #1256368: Add 'visible' key to hook_field_extra_fields() since they're still trying to add it :)

jhodgdon’s picture

RE #27 -- see comment #4 above. As of June 2012, it *did* exist. Perhaps the conversion to a class lost it?

Alan D.’s picture

Appears to have been lost in D7 with #1040790: _field_info_collate_fields() memory usage

David_Rothstein’s picture

FileSize
632 bytes

OK, I think the conclusion we reached in #1256368: Add 'visible' key to hook_field_extra_fields() is that the functionality never did exist in Drupal 7. It does exist in Drupal 8 (though was committed accidentally, and only seems to work by accident too as far as I can tell :)

So, this patch just removes the documentation from Drupal 7. The issue linked to above can take of adding it back, if/when the functionality itself is added to Drupal 7.

jhodgdon’s picture

Status: Needs review » Fixed

Thanks for the follow-up, and sorry for the earlier misdirection (it was my fault this got added in the first place to d7)! Committed to 7.x.

Status: Fixed » Closed (fixed)

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

rv0’s picture

Is there any module/code out there with a full implementation of the "edit" and "delete" properties?
I searched but I found none.