Let field types provide their support for "default values" and associated input UI

tagging

Rephrase

Awesome - I was hoping someone would grab this one :-)

Be aware of #2050801: Unify handling of default values between base and configurable fields, it's going to be a part of the equation - the getDefaultValue() method added over there is how field types can override their own logic to generate actual default values from the stored configuration (like, UUID --> local numeric id, 'now' --> actual date...).

re #5:
- $this->instance in FieldInstanceEditForm should be $this->getFieldDefinition() in FieldItem classes. Code should then only use FieldDefinitionInterface methods, no more direct access to $instance->someProperty (let me know if this causes trouble)
- "default value" broken when field is using the "hidden" widget: nope I don't think there's an issue for this. In this case, I think we should fall back to the "default_widget" for the field type.

Also - patch #4 seems to contain a lot of unrelated changes from other patches in the queue :-)

That looks like a very good start, let's see if we can push it a bit.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/FieldType/ConfigEntityReferenceItemBase.phpundefined
@@ -148,6 +148,27 @@ public function instanceSettingsForm(array $form, array &$form_state) {
+   * {@inheritdoc}
+   */
+  public function defaultValuesForm(array $element, array &$form, array &$form_state) {
+    return array();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function defaultValuesFormValidate(array &$form, array &$form_state) {
+    return array();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function defaultValuesFormSubmit(array &$form, array &$form_state) {
+    return array();

Those would need to duplicate the method bodies from ConfigFieldItemBase ? :-/
Although at this point, it might be easier to have ConfigEntityReferenceItemBase extend ConfigFielditemBase and duplicate the methods in EntityReferenceItem...
Let's leave it like that for now, I'll try to have @amateescu chime in.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/FieldType/ConfigFieldItemBase.phpundefined
@@ -45,4 +46,79 @@ public function instanceSettingsForm(array $form, array &$form_state) {
+    $entity_form_display = $form['#entity_form_display'];

Then it seems $form['#entity_form_display'] is used only within the defaultValuesForm*() implementations here, and FieldInstanceEditForm has no business defining it and placing it in the form.
It's only an implementation choice of ConfigFieldItemBase : "use the widget configured for the entity in the 'default' form mode", for which it needs the $entity_form_display.

Suggestion:
Move the logic about "figure out the actual widget to use (grab the entity_form_display, grab the widget used for 'default mode', unless it's 'hidden' in which case fallback to the 'default_widget' of the field type)" to a protected defaultValueWidget() method, witch returns an instantiated widget plugin object.
Each step (form, formValidate, formSubmit) can call it without storing stuff in $form.

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldInstanceEditForm.phpundefined
@@ -274,34 +224,7 @@ protected function getDefaultValueWidget($field, array &$form, &$form_state) {

I tend to think we should get rid of FieldInstanceEditForm::getDefaultValueWidget(). Like, always call the FieldItem::defaultValueForm*() methods. It's up to the FieldItem classes to do something or not.

FieldInstanceEditForm::buildForm() calls defaultValueForm() and wraps it in the 'details' element ("The default value for this field, used when creating new content" help text...) only if the return value is not empty. This allows to get rid of the $element param in defaultValueForm()

At any rate, we need to get rid of the field_behaviors_widget() check in FieldInstanceEditForm::buildForm(), that logic is now moot.

Side note: there was this issue about the bug with "hidden" widget: #2028759: Clean up instance['widget'] in Field UI
I linked to the current issue from over there.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/FieldType/ConfigFieldItemBase.phpundefined
@@ -45,4 +46,100 @@ public function instanceSettingsForm(array $form, array &$form_state) {
+    $default_widget = $this->defaultValueWidget();

Nitpick: just $widget would be more accurate / less confusing.

(same in the the other methods)

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/FieldType/ConfigFieldItemBase.phpundefined
@@ -45,4 +46,100 @@ public function defaultValuesFormValidate(array $form, array &$form_state) {
+    $element = $form['instance']['default_value_widget'];

OK, this is not ideal because it means the class has knowledge of the structure of the form generated by FieldInstanceEditForm - i.e "I know you called my defaultValuesForm() method and placed the result in $form['instance']['default_value_widget']". This kind of coupling is not too great.

Pondering this a bit...

Great ! The test that fails precisely tests the behavior we're changing, so it can just be removed.
Instead, we need a test that "if widget is set to 'hidden', then the default widget is used instead"

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/FieldType/ConfigFieldItemBase.phpundefined
@@ -45,4 +46,96 @@ public function instanceSettingsForm(array $form, array &$form_state) {
+    static $widget = NULL;

Let's not use static variables in object classes.
I propose the following:
Let defaultValueWidget() receive $form_state by ref, and do:

if (!isset($form_state['default_value_widget']) {
  $form_state['default_value_widget'] = ... $widget...;
}
return $form_state['default_value_widget']

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/FieldType/ConfigFieldItemBase.phpundefined
@@ -45,4 +46,96 @@ public function instanceSettingsForm(array $form, array &$form_state) {
+      $entity_form_display = entity_get_form_display($this->getFieldDefinition()->entity_type, $this->getFieldDefinition()->bundle, 'default');

$this->getFieldDefinition() ->entity_type or ->bundle are not strictly kosher, since they are not part of FieldDefinitionInterface. They work here because those "defaultValueForm" methods will be called for configurable fields for now, so getFieldDefinition() returns a FieldInstance object that has the properties. But it's not great if we want to change that in the future.

Instead, you can use:
$entity = $this->getParent()->getParent();
$entity->entityType() / $entity->bundle().

Similarly, the other methods in ConfigFieldItemBase should do $entity = $this->getParent()->getParent(); rather read it from $form['#entity']. Let's avoid a tight couple on stuff the caller has placed stuff in $form.

(side note, I've long been willing to open an issue to add a getEntity() method as shorthand for getParent()->getParent()
on FieldItemInterafce but failed to do so so far - If you feel like it ;-)

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/FieldType/ConfigFieldItemBase.phpundefined
@@ -45,4 +46,96 @@ public function instanceSettingsForm(array $form, array &$form_state) {
+      if (!$widget = $entity_form_display->getRenderer($field_name)) {

I don't think this will work, since the 'hidden' widget is an actual plugin.
Should be something like:
$widget = entity_get_form_display(...)->getRenderer($field_name);
if (empty($widget) || $widget->getPluginId() == 'hidden')) {
// fallback to the default widget
}

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/FieldType/ConfigFieldItemBase.phpundefined
@@ -45,4 +46,96 @@ public function instanceSettingsForm(array $form, array &$form_state) {
+        $field_type = \Drupal::service('plugin.manager.entity.field.field_type')->getDefinition($this->getFieldDefinition()->getFieldType());
+        $widget = \Drupal::service('plugin.manager.field.widget')->getDefinition($field_type['default_widget']);
+        $configuration = array(
+          'field_definition' => $this->getFieldDefinition(),
+          'settings' => $widget['settings']
+        );
+        $widget = \Drupal::service('plugin.manager.field.widget')->createInstance($field_type['default_widget'], $configuration);
+      }

Could be reduced to just:
$widget = \Drupal::service('plugin.manager.field.widget')->getinstance(array('field_definition' => $this->getFieldDefinition());
getInstance() takes care of fetching the default widget and assigning its settings.

Also:

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldInstanceEditForm.phpundefined
@@ -187,30 +160,8 @@ public function buildForm(array $form, array &$form_state, FieldInstanceInterfac
     if (isset($form['instance']['default_value_widget'])) {

This will always be set now, so the check can go away (same in submitForm())

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetPluginManager.phpundefined
@@ -87,7 +87,7 @@ public function __construct(\Traversable $namespaces, CacheBackendInterface $cac
   public function getInstance(array $options) {
-    $configuration = $options['configuration'];
+    $configuration = isset($options['configuration']) ? $options['configuration'] : array();

Ah, indeed :-)
If we're doing this, then let's rather add this at the top of the function:

// Fill in defaults for missing properties.
$options += array(
  'configuration' => array(),
  'prepare' => TRUE,
);

Then the line after "// Fill in default configuration if needed." a couple lines below can be simplified to just if ($options['prepare']) {. Yay for cleaner code.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/FieldType/ConfigFieldItemBase.phpundefined
@@ -117,25 +117,27 @@ public function defaultValuesFormSubmit(array $element, array &$form, array &$fo
+      if (!$form_state['default_value_widget'] = $entity_form_display->getRenderer($field_name)) {

Nitpick: keeping an intermediate $widget variable used to figure out the right $widget, then assigning it to $form_state[...] at the end would make clearer code.
Also, matter of taste, but we generally tend to favor readability over terseness, and unfold syntax like "test negation assignment" (if (!$var = ...)):

$widget = (plan a)
if (!$widget) {
  $widget = (plan b)
}
$form_state['...'] = $widget;

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldInstanceEditForm.phpundefined
@@ -160,9 +160,7 @@ public function buildForm(array $form, array &$form_state, FieldInstanceInterfac
-    if (isset($form['instance']['default_value_widget'])) {
       $this->getFieldItem($form['#entity'], $this->instance['field_name'])->defaultValuesFormValidate($form['instance']['default_value_widget'], $form, $form_state);

Indentation needs fixing then ;-)
(same in submit)

Thanks for being so quick ! Stacking remarks till a next reroll is also fine if it's easier for you.
I add my remarks as I see stuff so that I don't forget them ;-)

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/FieldType/ConfigFieldItemBase.phpundefined
@@ -45,4 +46,101 @@ public function instanceSettingsForm(array $form, array &$form_state) {
+      // When field is hidden, use the default widget and store it in the form.

Grammar is incorrect, and the comment is slightly misleading (we put the widget in the form state in all cases).
Maybe just a single comment on top of that code block : "Use the widget currently configured for the 'default' form mode, or fallback to the default widget for the field type" ?

#23: default_values-2056405-23.patch queued for re-testing.

Ok, sorry about that, we need to keep ConfigEntityReferenceItemBase extends EntityReferenceItem, because that's what triggers EntityReferenceItemNormalizer (runs on item classes that extend EntityReferenceItem).

So it means copying the defaultValuesForm*() methods. Sad panda...
There might be ways to reformulate that whole EntityReferenceItem inheritance class, but that's far outside the scope of this issue, so let's do this for now.

I refactored and shortened a couple things. Sorry @plopesc for "stealing" that last stretch, but some parts I needed to experiment myself, so it was easier that way.

don't steal our "approved" tag, d.o...

Side note: found #2060765: warning on "Field instance edit" form for a taxo field while testing this. Not related, though, happens in HEAD too.

the follow-up about $entity = $this->getParent()->getParent() ?
Would be awesome :) A helper method for the language would be cool too.

Field UI tests needed to be updated for the new structure of the form values.

In addition:
- put back "don't show anything if isset($instance->default_value_function)", as in HEAD
- reintroduced conditional execution of defaultValueFormValidate() / defaultValueFormSubmit(). Was needed by the previous point, and makes it easier for FieldItem classes to opt out (simply override defaultValueForm())
- make FileItem and ImageItem opt out, since those field types have their own handling of default values
- remove the "does not support default value" property on widgets; as per the OP, it's a behavior of the field type now

This should be ready now.
We need #2050801: Unify handling of default values between base and configurable fields before we can fix date.module

Hm, re-uploading

...

#35: default_values-2056405-33.patch queued for re-testing.

damn annotations

Boy I was tired, that's debug code for another issue :-)

Actually, while working on #2050801: Unify handling of default values between base and configurable fields again it occurred to me that the handling of "assign default values in newly created entities" happens in the Field classes, not the FieldItem classes.

So field types willing to modify how their default values are handled (eg. replace numeric IDs with UUIDs) will need to override methods in their 'list class' (extends Field) rather than in their 'item class' (extends FieldItem).

So it would make sense that the defaultValueForm*() methods live in ConfigFieldInterface / ConfigField rather than in ConfigFieldItemInterface / ConfigFieldItemBase.
- defaultValueFormSubmit() is the mirror method of the Field::getDefaultValue() method added in the other issue - massage before storing in config / massage after reading from config. If you implement one, you'll need to implement the other, so it's better DX if both live in the same interface & class.
- It would also make the code in the base implementations less weird: they currently live in the FieldItem object, but do most of their work on $this->getParent(), i.e the Field object. That just reflects the fact that "default values" are a concept that applies at the Field level, not FieldItem level.
- it would avoid duplicating the implementations in ConfigEntityReferenceItemBase, since they would live in ConfigField, which is the 'list' class entity_reference fields use.

Let's wait on #2050801: Unify handling of default values between base and configurable fields before refactoring this, it shouldn't be too far away.

API changes

Now that #2050801: Unify handling of default values between base and configurable fields is in, and as per #45, moved the methods from the FieldItem classes to the Field classes.
I'll update the OP accordingly.

Reroll

Good call(s) :-)

re FormInterface():
I don't think FormInterface should be our concern here. A ConfigField class is not a form, it's a class that, as a *secondary* job, happens to provide a subform (currently one single subform, but ConfigFieldItem classes provide two different subforms: field settings form, instance settings form).
Those methods are about providing and processing a very specific subform which is "the default value form for this field type". Calling the methods buildForm(), validateForm(), submitForm() would be very misleading here.

Change notice added at https://drupal.org/node/2076489

I still disagree on #54.
- FormInterface is designed for forms, not subforms (we can't implement FormInterface::getFormID()
- seeing Field::buildForm(), Field::validateForm() + simply {@inheritdoc}... would be *very* misleading - "build a form for the field ?". We're only talking about "the UI for default values of the field", those are in now way inherent to "what is a Field, what would be its 'form'". Totally beats "developers will have more experience with FormInterface" IMO.
- what if we need to have those classes generate another, different subform ?

[edit: oh, signatures don't match anyway ;-) defaultValuesFormValidate() & defaultValuesFormSubmit() have an $element param - which is strongly tied to "being a subform"]