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 commentedProbably one or the other, yes.
But not too sure which one before looking at some actual code :-)
Comment #3
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 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 commentedComment #7
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
AutocompleteWidgetandAutocompleteTagsWidget) and code was previously abstracted intoAutocompleteWidgetBase. Since this issue separatesWidgetBaseintoWidgetBaseSingleandWidgetBaseMultiplewe cannot inherit both from the previous base class (AutocompleteWidgetBase) and the new one. Therefore we now have some code duplication betweenAutocompleteWidgetandAutocompleteTagsWidget.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 commentedComment #23
undertext commentedRerolled.
Comment #24
undertext commentedFixed form_state vars in function declarations.
Comment #29
undertext commentedOh. This one should be ok.
Comment #44
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #45
smustgrave commentedwanted to bump this one more time if still relevant?
Comment #46
smustgrave commentedbelieve still relevant
Comment #47
e0ipsowoah! Blast from the past.
I am no longer interested in this, but this notification sent me on a memory trip 🥹 ❤️