Right now widget plugins can specify in their annotation that they are 'multiple_values' (like: "don't repeat myself, on single copy can handle input of multiple values").
This is used for things like option widgets (checkboxes/radio, select) or the taxo autocomplete widget.
WidgetBase::form() uses this property to call formSingleElement() directly, or formMultipleElements() to "repeat N copies of the widget".
The not-too-nice thing is, the content of WidgetBase::submit() currently only really makes sense for stuff that went through formMultipleElements().
I'm thinking It would probably be cleaner to provide a separate MultipleWidgetBase base class for 'multiple' widgets, and get rid of the 'multiple_values' entry in the widget info. Widgets that are 'multiple' just extend from MultipleWidgetBase instead of WidgetBase.
Comment | File | Size | Author |
---|---|---|---|
#29 | cleanup-widget-api-1846162-29.patch | 54.72 KB | undertext |
#24 | cleanup-widget-api-1846162-24.patch | 46.29 KB | undertext |
#23 | cleanup-widget-api-1846162-23.patch | 46.27 KB | undertext |
#17 | cleanup-widget-api-1846162-17.patch | 54.85 KB | e0ipso |
Comments
Comment #1
sunWould it make sense to extend vertically?
Or perhaps even reversed?
I don't know the internals enough, so don't know which of both bases is actually more specific than the other.
Comment #2
yched CreditAttribution: yched commentedProbably one or the other, yes.
But not too sure which one before looking at some actual code :-)
Comment #3
nils.destoop CreditAttribution: nils.destoop commentedWrote big piece of this friday. but didn't post a patch yet. I went for the first case. Re-rolling it now at latest state.
Comment #4
nils.destoop CreditAttribution: nils.destoop commentedUpdated patch. The 'Store field information in $form_state.' and 'Populate widgets with default values when creating a new entity.' should probably move to a seperate method of WidgetBase. Now that code is duplicated.
Comment #5
nils.destoop CreditAttribution: nils.destoop commentedComment #7
nils.destoop CreditAttribution: nils.destoop commented#4: 1846162-4-MultipleWidgetBase.patch queued for re-testing.
Comment #9
andypostComment #10
e0ipsoNew patch from scratch.
This was pair programmed with @plopesc in Szeged.
Comment #11
e0ipsoNeeds review. Testbot, get to work!
Comment #13
e0ipsoHopefully this will fix the issues from #10.
Common autocomplete functionality (in
AutocompleteWidget
andAutocompleteTagsWidget
) and code was previously abstracted intoAutocompleteWidgetBase
. Since this issue separatesWidgetBase
intoWidgetBaseSingle
andWidgetBaseMultiple
we cannot inherit both from the previous base class (AutocompleteWidgetBase
) and the new one. Therefore we now have some code duplication betweenAutocompleteWidget
andAutocompleteTagsWidget
.A possible workaround would be to use a trait.
Comment #14
plopescRe-rolling after #2188613: Rename EntityStorageController to EntityStorage and #2136197: Move field/instance/widget/formatter settings out of annotation / plugin definition
Comment #15
e0ipso14: cleanup-widget-api-1846162-14.patch queued for re-testing.
Comment #17
e0ipsoRe-roll of #14.
Comment #18
plopescLet's go testbot!
Comment #21
andypostComment #22
undertext CreditAttribution: undertext commentedComment #23
undertext CreditAttribution: undertext commentedRerolled.
Comment #24
undertext CreditAttribution: undertext commentedFixed form_state vars in function declarations.
Comment #29
undertext CreditAttribution: undertext commentedOh. This one should be ok.