PATCH: fix issue #8691 - DB Boolean Search #9893
Conversation
|
LGTM.. Effectively ports over what 3.x did. |
| $field = new DropdownField($this->name, $title, $source); | ||
| $field->setEmptyString("($anyText)"); | ||
| return $field; | ||
| return (new DropdownField($this->name, $title, $source)) |
There was a problem hiding this comment.
Would using the injector be better instead of instantiating the class directly?
There was a problem hiding this comment.
It would be. Unfortunately, most DB fields do still circumvent Injector in their scaffoldSearchFields() implementations. I think it's just code from 3.x that survived the upgrade in tact.
There was a problem hiding this comment.
Yes, it would... Do you mean:
DropdownField::create()Happy to change it.
There was a problem hiding this comment.
I'd say let's do that in a separate PR that applies injector to all these core implementations, rather than just do a one-off. The problem is, if we have some fields that are responding to Injector overrides and some that aren't, it just creates a confusing dev experience. In my quick assessment, it looks like most fields are still using new, so I'd advocate for just going with the pack, here.
Consistent use of Injector in a separate body of work would be sweet, though.
See: 8691 for details
@unclecheese