The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
This is the replacement of D7's field_ui_field_type_options().
Proposed resolution
Create a suitable function, similar to code existing in FieldStorageAddForm::buildForm(), and similar to FormatterPluginManager::getOptions and WidgetPluginManager::getOptions
Remaining tasks
Review patch
User interface changes
None
API changes
New FieldTypePluginManager::getOptions() method
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #37 | create-2017711-37.patch | 3.74 KB | jcnventura |
| #37 | interdiff.txt | 3.61 KB | jcnventura |
Comments
Comment #1
fagoComment #2
netsensei commentedMaybe i'm underestimating the amount of work here, but willing to take a look at this anyway, if that's ok.
Comment #3
netsensei commentedComment #4
yched commentedThanks @netsensei ! Should be fairly simple ;-)
Comment #5
netsensei commentedComment #6
yched commentedNitpick: I'd just shorten that to foreach ($this->getDefinitions() as $field_type => $definition) {
Other than that, looks ready to me :-)
Comment #7
netsensei commentedGreat. :-) Adapted the patch.
Comment #8
yched commentedWorks for me. Thanks !
Comment #9
alexpottI think we need to override
clearCachedDefinitions()to clear fieldTypeOptions property and call the parent.Comment #10
netsensei commentedAdded the override of
clearCachedDefinitions(). Clears fieldTypeOptions property.Comment #11
amateescu commentedLooks good to me.
Comment #13
swentel commented@Alex, why would we want to do that ? We don't do it either in the widget or formatter plugin manager, and it's not that the list of field types is going to change during the request or so. It can (and honestly only /may/) change when enabling or disabling a module.
Moving back to RTBC for #7 and to get feedback since I couldn't get you on IRC :)
Comment #14
webchickI am not sure. :) Assigning to Alex. He's currently on vacation but I believe gets back next week.
Comment #15
xano#10: 2017711-FieldTypePluginManager-getOptions-10.patch queued for re-testing.
Comment #17
plopescRe-rolling patch in #7 and adding
getOptions()toFieldTypePluginManagerInterfacein order to improve type hinting.Comment #19
plopescUps, I didn't realize that
getDefinitions()had to be renamed togetConfigurableDefinitions().Using this method, and renaming
getOptions()togetConfigurableOptions()to keep consistency.Sorry for the noise.
Comment #20
berdirSee #2191709: Remove the "configurable" flag on field types.
Comment #21
swentel commented19: fieldtypepluginmanager_options-2017711-19.patch queued for re-testing.
Comment #23
jsbalseraRerolled
Comment #24
jsbalseraComment #26
jsbalseraSo the reroll wasn't as simple as expected, working on the failing tests
Comment #27
jsbalseraChanging getConfigurable* for getUi* when it's needed
Comment #28
jsbalseraForgot the interdiff, sorry
Comment #29
berdirNot sure if this method name makes sense, there are only ui options, so maybe just getOptions() would be enough? Not sure.. maybe discuss with someone else, swentel?
THe check is no longer necessary, because getUiDefinitions() already filters no_ui out.
Comment #30
swentel commentedYeah, getOptions() is better and consistent with the formatter and widget manager.
Comment #31
jsbalseraChanging name and fixing 2.
Comment #32
jsbalseraComment #33
berdirI think this is a merge conflict and shouldn't be here
There's no $field_name param, I think that's incorrectly copied from widget/formatter methods?
So remove @param and simplify @return.
Maybe explicitly specify somewhere in the description that this will only return fields that can be added in the UI and have a default widget and formatter? Possibly as a second line after the initial sentence?
Comment #34
jsbalseraFixing 1. and change docblock as described in 2.
Comment #35
alansaviolobo commentedreroll
Comment #36
xanoI'm not sure we should be adding UI helpers to an API-level class like a plugin manager. Can we move this to a separate class so we separate responsibilities?
I know this has been done all over core, but we should stop adding such switch parameters to methods, and instead create separate methods so return values' structures are actually predictable and easy to document.
Comment #37
jcnventuraXano, valid points, but this should be done with the same pattern as the the Widget and Formatter plugins.
As it's too late to remove getOptions from the others, I think that it's more coherent this way.
Comment #38
jcnventuraTestbots are go!
Comment #39
xanoI still disagree. It provides inconsistent DX. It's a new method on an interface (which is also a BC break), so it's a chance to do it right.
Comment #51
smustgrave commentedWonder after 7 years if this is still a valid task?
If so it will need to be updated for D10 and I believe it will need tests.
Comment #54
smustgrave commentedIf still a valid task please reopen but closing for now.
Comment #55
amateescu commentedJust wanted to point out that this issue really is outdated, all the code around adding fields from the UI was shuffled a lot in #3356894: Make field selection less overwhelming by introducing groups and the followup issues.