Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Spin off from #2226063: Merge ListBooleanItem from options module into BooleanItem
The widgets provided by options.module are field-type agnostic and work with any field type that implements AllowedValuesInterface.
So they would be better off in Core, now that Core can provide plugins (wasn't the case back when we converted options.module to plugins). No need to depend on options.module if you just want the widgets.
That's kind of what we had in D7 - options.module was just providing agnostic widgets, and list.module was providing the field types. Then the two were merged because for some reason "list" was problematic for a module name in D8 (reserved keyword or something). But now we have Core for that :-)
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | 2302021_optionWidgets_Core-20.patch | 9.51 KB | undertext |
| #17 | interdiff.txt | 8.17 KB | yched |
| #17 | 2302021_optionWidgets_Core-16.patch | 9.5 KB | yched |
| #14 | 2302021-14.patch | 2.77 KB | swentel |
| #6 | 2302021-6.patch | 2.51 KB | swentel |
Comments
Comment #1
yched CreditAttribution: yched commentedComment #2
swentel CreditAttribution: swentel commentedComment #4
swentel CreditAttribution: swentel commentedoh man
Comment #6
swentel CreditAttribution: swentel commented#2199637: Replace "required" flag of Field module with proper dependencies just landed
Comment #7
yched CreditAttribution: yched commentedIf we do that, we should be able to also remove dependencies on options.module in a couple core modules (like at least taxonomy ?), and also probably in a couple tests ?
Also, if we move the widget classes in a folder / namespace that alredy has 10+ widget files, we might want to make the class names a bit more specific (and connected to their base class) ?
Like OptionsWidgetBase / OptionsButtonsWidget / OptionsSelectWidget... ?
Comment #8
yched CreditAttribution: yched commentedHm, "options" as a prefix is a bit pointless once those are out of options.module... Those are generic widgets for field types that implement AllowedValuesInterface, so maybe something like AllowedValuesWidgetBase etc... ?
Comment #9
swentel CreditAttribution: swentel commentedShould the plugin id's then also be renamed, e.g. options_select -> select ?
Comment #10
yched CreditAttribution: yched commentedHeh, then again, #2329937: Allow definition objects to provide options renames AllowedValuesInterface to OptionsProviderInterface, so "options" would make sense again, It's then options.module, as the provider of List* field types, that is misnamed :-p
I'd vote for keeping Options / options_ as a prefix here (but adding it to the class names)
Comment #11
yched CreditAttribution: yched commentedIt's #2332885: Rename AllowedValuesInterface to OptionsProviderInterface, actually.
Comment #12
fagoAgreed that staying with options and having it as prefix is fine. It should move its tests also though.
Comment #13
fagoRelated: #2342581: Merge list field types into main field types
Comment #14
swentel CreditAttribution: swentel commentedNew patch, renamed the classes and removed the dependency of taxonomy on options.
Comment #15
yched CreditAttribution: yched commentedThe namespace of the various classes is still Drupal\options\... :-p
Comment #17
yched CreditAttribution: yched commentedFixed the namespaces, and removed a couple dependencies in tests (manually re-tested those to checks that the dependencies were actually not needed anymore)
And the RTBC in the previous comment was totally unintended :-p
Comment #18
swentel CreditAttribution: swentel commentedAhaha, silly me :)
Comment #20
undertext CreditAttribution: undertext commentedRerolled
Comment #21
undertext CreditAttribution: undertext commentedComment #22
BerdirCheck the diff of the two patches, only difference is ContentEntityInterface -> FieldableEntityInterface in one case.
Comment #23
alexpottCommitted 10cfcd6 and pushed to 8.0.x. Thanks!