Add setting for size to email widget

Looks good if green.

We don't want a UI for the setting ?

Re-rolling the patch with test fix.

I'm pretty sure that this change isn't problematic in any way. config schema doesn't care if a property is missing and we automatically merge in widget/formatter default settings, so we can be sure that size is always set. So there is no BC break.

+1 to rtbc

As mentioned in #2, we should expose this setting in the UI just like we do for other widgets.

I do not understand why this issue is "Major" priority.

#2227503: Apply formatters and widgets to Comment base fields is major, so if as @larowlan's IS implies that issue needs this one, then this one should be major.

Fixed #9 :)

RTBC if green

+++ b/core/config/schema/core.entity.schema.yml
@@ -178,6 +178,9 @@ field.widget.settings.email_default:
+    size:
+      type: integer
+      label: 'Size of email field'

This causes a data model change. There is a migration path (good), but the IS says there's no data model change (bad).

#7 explains why this is no BC break. So, leaving at RTBC.

I think we need to worry about existing fields here - we need to resave them so the default configuration is stored - otherwise a user will get unexpected configuration changes if they go to a field and just press save in the UI. Or we can choose to ignore this and just let it happen... but that is not how we've handled this before.

Discussed with @catch and we agreed that a post update hook just to re-save email fields is the right thing to do here.

Adding the size to the summary as well.

Regarding the post update. We need to update the form displays, not the fields. Do we have an post_update place for the entity system/core somewhere ?

Afaics core can't a have a post update so system.post_update.php is the place to put the function.

I would suggest field.post_update.php, that's where we put post update functions for the entity reference field, which is also provided by core.

This should do it :)

Looks sane to me

1. Should this be in a different upgrade docblock group ?
2. Do we need an upgrade path test for it ? (/runs)

nitpick

+++ b/core/modules/field/field.post_update.php
@@ -42,5 +43,27 @@ function field_post_update_entity_reference_handler_setting() {
+  return t('The new size setting for email fields has been added.');

maybe 'email widgets' ?

1. and 3. Fixed, good catch :)

I'm not sure if these post updates are testable or not..

Well, UpdatePathRC1TestBaseTest looks at it at least.

I guess one option is to add an additional file that additionally to standard install then sets up the email field on the article, but ugh ..
We'll also have to compare with the raw version of the config seens getComponent merges in default settings, seems like a lot of hassle for this :/

Hassle or not, here it is :)

Looks everything addressed, RTBC++

Looks great and thanks for the test coverage. Just one nit, but since it's user-facing moving to CNW.

1. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/EmailDefaultWidget.php
   @@ -37,6 +39,13 @@ public static function defaultSettings() {
   +      '#title' => $this->t('Size of the input field'),
   

   Why input field here.

2. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/EmailDefaultWidget.php
   @@ -59,6 +68,7 @@ public function settingsSummary() {
   +    $summary[] = t('Textfield size: @size', array('@size' => $this->getSetting('size')));
   

   But Textfield size here. Should we not use Textfield size in both cases?

3. +++ b/core/modules/field/field.post_update.php
   @@ -44,3 +45,34 @@ function field_post_update_entity_reference_handler_setting() {
   + * @addtogroup updates-8.1.0-beta
   

   This isn't just a beta-to-beta upgrade path, it's an upgrade path from 8.0.0 to 8.1.0, but no idea what a better defgroup name is so let's leave it as is here then open a new issue to discuss.

#29.1 and .2: Fixed :)

#29.3: I see that Views also has an 8.1.0 update function (views_update_8005()) and it's in a "updates-8.1.0" defgroup, so I also changed it in this patch in order to be consistent with that one.

Looks good now, I think.

Maybe we should open issue to look at all upgrade path def groups, because I think we're not consistent everywhere, e.g. user_update_8100() ?

Patch looks good to me too. FWIW user_update_8100 seems wrong it should be user_update_8000. def groups are there to identify this.

user_update_8100() looks fine - it's the first user update for 8.1.x.

Patch looks better now, so committed/pushed to 8.1.x, thanks!