I just ran into a Field API limitation over at #1586334: field_extrawidgets makes the default widget for fields "Hidden". That module is providing some advanced widgets for all fields: 'hidden' and 'read-only'. Widgets can have weights relative to a specific field instance, which controls the order of the widget on the entity form. However, since there's no support for weights for the widgets themselves, there's no good way for this contrib module to ensure its widgets come last in the choice of widget in the Field UI. So, once you turn on field_extrawidgets, the default widget for all fields via the Field UI is "Hidden". UI #fail indeed. My hack work-around is to use hook_field_widget_info_alter() so the module advertises its widgets that way and can stuff them at the bottom of the array, but then other modules can't alter its widget info. Ugh.

I was lamenting this fact in IRC and webchick encouraged me to open an issue about it, saying this sort of API addition is potentially backportable to D7. I searched and found nothing, so I hope this isn't duplicate.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww’s picture

Assigned: Unassigned » dww
Status: Active » Needs review
FileSize
2.47 KB
2.49 KB

Patches for D8 and D7. Tested on D7 and works nicely.

Status: Needs review » Needs work

The last submitted patch, 1586356-1.field_widget_info_weight.d7.patch, failed testing.

dww’s picture

Status: Needs work » Needs review
FileSize
2.49 KB

Sorry, I don't know all the fancy ways of the bot these days. Here's just the D8 patch again.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs backport to D7

I was also bothered by the default UX behavior provided by field_extrawidgets, and this patch looks really clean and straightforward to me.

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

This looks simple enough but let's add a test for it.

dww’s picture

What exactly is a test supposed to test here?

Do you want a unit test that uasort($info['widget types'], 'drupal_sort_weight'); does what we expect? That's the *only* change to the code, everything else is comments and docs. I don't follow core's test coverage that closely, but surely we'd already know if that was broken, right?

Do you want a functional test that adds dummy widgets and tries to ensure the right order in the field UI? That seems like an awful lot of trouble, especially if we already know drupal_sort_weight() works.

Cheers,
-Derek

yched’s picture

Status: Needs work » Reviewed & tested by the community

I don't think I see where a test would be useful either.

This being said, and while I approve this, this topic about ordering Plugin Implementations (we're working on transforming widgets to Plugins right now - #1742734: [META] Widgets as Plugins) already surfaced in #1512602-9: Write a hook based plugin discovery implementation..

Short version : currently image_effect_definitions() (equivalent of _field_info_collate_types() for image effects) does a ksort on available image effects. It was agreed that a "list of available plugins implementation" has no natural order by itself - i.e the discovery process can do some massaging on each plugin info (like filling in defaults for missing properties), but has no business massaging the list as a whole - and currently the Plugin API doesn't allow this.

If an order makes sense, it's only presentational, and the ordering should be made by the calling code (the UI) - and different places in the UI might each have their own relevant sorting logic, for that matter.

So, I have no problem getting this in as is (image effects currently do something similar), but #1742734: [META] Widgets as Plugins will take the ordering out of the discovery and in the UI code.

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: -Needs tests

OK fair enough. Committed/pushed to 8.x, moving to 7.x for backport.

amateescu’s picture

Status: Patch (to be ported) » Reviewed & tested by the community
FileSize
2.47 KB

Re-uploading the D7 patch from #1.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

This seems harmless enough for D7, and helps out d.o as well.

Committed and pushed to 7.x. Thanks!

Status: Fixed » Closed (fixed)

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

David_Rothstein’s picture

Issue tags: +7.17 release notes

This is a neat little feature addition; seems worth mentioning in CHANGELOG.txt and the release notes.