Provide better default configuration when re-using an existing field

To illustrate a similar problem using just Drupal core,

1. Go to "Administration > Structure > Content types > Page" (/admin/structure/types/manage/page/fields).
2. Click the "Add field" button.
3. From the "Re-use an existing field" select list, choose the "Entity reference: field_tags" option.
4. Click "Save and continue".
5. Fill in the required "Available Vocabularies" field (select "Tags") and click "Save settings".
6. Open the "Manage form display" tab (admin/structure/types/manage/page/form-display).

After these steps, the Tags field on the "Basic page" content type (or is it "Page"?) uses the "Autocomplete" widget, even though the original Tags field on the Article content type uses the "Autocomplete (Tags style)" widget.

The original proposed solution is to use the defaults from PreconfiguredFieldUiOptionsInterface::getPreconfiguredOptions() when reusing an existing field. I do not think that would change the behavior in this situation.

An alternative approach is to copy the configuration (such as the widget) from the field being copied. That would solve the original problems #2668678: Make paragraphs widget the default for paragraphs fields and #2719593: Unable to reuse existing paragraphs field reported for the Paragraphs module and also change the behavior described here.

Here is the code referred to in the proposed resolution: from FieldStorageAddForm.php:

  public function submitForm(array &$form, FormStateInterface $form_state) {
    // ...
    // Create new field.
    if ($values['new_storage_type']) {
      // ...
        $field_type_class = $this->fieldTypePluginManager->getDefinition($field_type)['class'];
        $field_options = $field_type_class::getPreconfiguredOptions()[$option_key];
      // ...
    }

    // Re-use existing field.
    if ($values['existing_storage_name']) {
      // ...
    }

I think that function could be refactored in order to avoid duplicating code and to treat the two cases more consistently. I think that only one of the two blocks will be executed, but the way it is written it looks as though we might both create a new field and reuse an existing one in the same form submission.

After looking at this some more, I realize that neither of the proposed solutions (copying configuration from an existing field and bundle or using the preconfigured options) will work without either changing the UI or saving additional information with the field storage config. I am updating the issue description, suggesting three options.

I am having trouble running the tests locally, so let's see what the testbot thinks of my patch!

This should fix at least one failure.

This patch has a chance of satisfying the testbot.

This patch implements Option 1 from the issue description: give the user the choice of copying settings from any bundle that already has the field attached.

I extended ManageFieldsTest::testPreconfiguredFields() to test the new functionality.

My initial thought was to present the additional choices organized by bundle, with the available fields for each bundle listed together. A vote at the Boston Drupal meetup was pretty overwhelming to do the opposite: organize by field, listing together all the bundles that have the field attached. So that is how this patch works.

There is one behavior of this patch that I am not sure about. Suppose I copy the Tags field from the Article type (source bundle) to the Page type (target bundle). In a default Drupal installation, the RSS view mode is enabled for Article but not Page. With this patch, the RSS view mode will be enabled for the Page type, and the display of the Tags field will be copied over. The same thing will happen for any display mode (view mode or form mode) that is enabled for the source bundle. I think there are a few choices for how this should work when a display mode is enabled for the source bundle:

1. Always enable the display mode on the target bundle (current patch).
2. Enable the display mode unless the source field is disabled for that display mode on the source bundle.
3. Never enable a disabled display mode. Only copy settings for the field if the display mode is already enabled for both source and target bundles.

I think I prefer (2), but I would like to get other opinions before doing any work to change the current behavior. I really do not like (3). It might come as a surprise to a site builder who configures a lot of fields and view modes for the Article type, then copies the fields to the Page type without first enabling the display modes.

Fixed line exceeding 80 characters.

@gaurav.kapoor, thanks for reviewing the patch. It looks to me as though the line you changed was exactly 80 characters long, so it did not need to be fixed.

I meant to mention that this patch rewrites a substantial part of #2446511: Add a "preconfigured field options" concept in Field UI.

Also, I considered adding a "test only" patch, with just the lines I added to ManageFieldsTest::testPreconfiguredFields(). I decided not to, since the test would fail in a very uninteresting way: it looks for an option on /admin/structure/types/manage/page/fields/add-field that is added by the patch, and will not find it.

I got confused in minimum characters in a line . I take it back. Sorry.

Using this on one of my current projects, I notice that the existing fields are not sorted in any predictable way. I also think that I would rather see the list organized by content type, despite the feedback I got at the meetup.

I think the next steps qualify for a novice issue: I want someone to give my patch a try and give feedback.

If we agree on the direction for this issue, then we can update the issue summary.

We are working on this issue at DrupalCon Baltimore in Room 301 with jlint and jrearick.

Screenshots from before and after patch #14.

I think adding "Settings" after the Content type would make it more clear what your options are. It would make it clear that the difference is just in the settings.

Example:
Entity reference (field_tags)
Default settings
Article settings (article)

Looking at how the opt groups that are set up in patch #14, it seems like the data storage is the primary way to drill down instead of field. As a site builder and scanning through the options for a field that is asking to select an existing field, I expect the first thing I see will be a field name. Perhaps the naming order in the opt groups could be {field} ({storage}) instead of {storage}.

Example: Instead of "Entity reference (field_tags)", label it "field_tags (entity reference).

I can foresee, on a site with a lot of fields, there might be a huge number of opt groups starting with the same text, making it difficult to scan down to the field you are looking for.

@AMarshall, thanks for the screen shots! Let's show them in-line. Here is what it looks like without this patch:

and here is what it looks like after the patch:

To clarify: the current version of the patch includes the line "Default settings" and you are suggesting that I add the word "settings" to the Article line. I will give that a try. It might cause trouble when the content type has a long name.

Help text does not transfer to new field.

In our test we created a new image field. We then added help text to the original field and disabled the Alt text required check box. When we used it in another content type, the field had no help text but the Alt text required check box was unchecked.

Doing the same thing @amarshall did, I found that checking the alt text required doesn't change that help text is not transferred over. The help text is not copied as expected.

Steps to reproduce:

• Add help text for the image filed for the article content type
• Create a new content type
• Add the existing field field_image and settings from the article content type to the new content type
• Observe the help text is not populated for the new content type

Oops

@benjifisher Good point. This is what it looks like with a long content type.

Being able to easily clone field configuration including view modes from existing fields would save us a lot of time. During the sprint at DrupalCon Baltimore, we had some good discussion. Below are my thoughts of the discussions we had about this issue. I'm sure the others involved will want to include their clarifications as well.

Multi-select
An idea came up that we could split this into two select boxes to make it easier to select the appropriate field instead of an opt group-based select. It was proposed to have the first select the field and the second to select the bundle/content type or use default configuration. The second select would only include bundles/content types that use the field selected in the first. As a site builder, however, I probably want the order to be be reversed. Since there can be so many fields on a site, it would be easier to drill down by bundle/content type first, and then by field. The problem with that is how do we allow them to select default configuration?

What is 'Default settings'?
As a site builder and new to Drupal, it can be difficult to understand what 'Default settings' means. How do we communicate what this means in the way the UI is designed or what do we put for the help text? If I am re-using an existing field, what does default mean? Perhaps the option should be titled "Default {field type} field settings" would be more clear? Eg. "Default image field settings"

Display mode settings
When getting field settings, as a site builder, I would expect the display configuration to come as well. It does make sense that if the source field is disabled in a particular view mode and the destination view mode is not enabled, we do not enable the view mode on the destination. Another way to put it is copy view mode settings for the field only if they are enabled in a view mode.

Form Display
I would also expect the form display settings to get copied over as well. I wonder if this might be related to @Amarshall's report about the help text not transferring as that might actually be part of the form display. I confirmed other form display settings are not copying over by doing the following:

• Change the preview image style from 100x100 on field_image to something else in the article content type
• Add an existing field to another content type targeting the field_image from the article content type
• Save the field settings on the next screen
• Observe the image style in the form display tab is 100x100 instead of whatever you changed it to on the source

@jrearick: thanks! That is a pretty good summary of the discussion that you, I, and @mikeker had at the end of the sprint day at DrupalCon Baltimore.

At this point, I think there is consensus that this patch is going in the right direction, so I will update the issue summary. (Go with "Option 1".)

Now that I have some feedback, testing, and screenshots, I am removing the Novice tag.

I think the recommendations on display modes in #31 corresponds with option (2) in #14.

There is another reason to choose field first and bundle (or defaults) second, rather than the other way around. Choosing the field first makes the process less of a change from the current behavior.

On the other hand, I think it is important to make these decisions keeping in mind the site builder's point of view. Compare with the discussion on #2446511: Add a "preconfigured field options" concept in Field UI: the UI should reflect the way a site builder sees things, not necessarily the way the data are stored in the back end. Copying the settings for display modes (i.e., form modes and view modes) is a pretty big change, touching several different config objects, but I agree with #31 that it fits better with the way a site builder thinks of the site.

I would still like to hear other opinions on how the UI should work.

oops

I think this needs a reroll now that #2847685: Update doc blocks for configureEntityFormDisplay() and configureEntityViewDisplay() has gone in. I will add the Novice tag for that.

$ git apply --check 2717319-reuse-fields-14.patch

no output for 8.4.x

@kostyashupenko, thanks for checking! I guess I should remove the Novice tag, too.

I have attached a new patch based on Drupal 8.5.x and an interdiff, comparing to the patch in #14.

The new patch addresses most of the points in the comments above, esp. #31. Here is the record from my local git repository:

1. Be a little more ternary.
2. Update the Select list
   Change from "type(field_name)" to "field_name(type)" as suggested in #31.
   Change "Default settings" to "Default type settings" as suggested in #31.
   Sort optgroups alphabetically. See #20.
3. Code cleanup: add "array" as type hint
   Keep CodeSniffer happy (or at least make it happier).
4. Enable display modes for which the copied field is visible
5. Display a message when a display mode is enabled
6. Add description (help text) and default value to copied config

I updated the issue summary (Completed and Remaining tasks). The main remaining task is the one suggested at the Boston Drupal meetup and in #31 (at DrupalCon Baltimore): use two separate select lists instead of a single list with optgroups. I am still thinking about how best to do that. I think I should also update the tests for the additional features in this patch.

Oops, I had to revert the first commit I mentioned in #39. The attached patch passes all field_ui tests locally.

I am attaching interdiffs against the patch in #14 (so you can ignore the failing patch from #39) and against #39 (in case you are curious about what I did wrong). I also did a tiny bit of cleanup for this patch.

I am adding #1953836: Replace "Select a field type" with a hierarchical select as a related issue. There is some discussion there on the merits of a "hierarchical select" approach (as suggested in #31 here). Perhaps it makes more sense in the context of this issue than that one, since the optgroups in the current patch here identify different fields, and are not used just to group the list.

In the UX meeting today, we cannot tell if the 80% use case is that no field settings are copied or that field settings should be copied. The site builder use case with eg. taxonomy terms is that you copy over the vocabulary setting, etc. so you truly have the field reused. What is the use case of not wanting to copy over field settings (the "Default settings" use case from the patch)?

Talked about this on UX meeting.

The problem with the screenshot at #30 is that we're taking what is probably a behaviour people want in 90%+ of cases (that when they clone a field, they also clone the configuration), but they're being forced to take on the cognitive load of deciding between two options for every single field. And worse, the implications of those choices are not at all clear. For example, that "default settings" means the field is not required, size of 60, whatever, and "Article (article)" is required, size of 25, whatever. It seemed like a better approach would be for the system to just do the right thing for the "80% case" automatically, and give the user a chance to review the changes after the fact to ensure they're correct.

Suggestion was instead, go back to the old UI (first screenshot of #26), and behind the scenes, pre-populate the configuration settings based on the first instance of this field. (For example, field_tags on the Article bundle). A standard green message would appear on the subsequent form something to the effect of "Configuration settings copied from Article: field_tags" so the user would be alerted to such.

Tagging with "Needs subsystem maintainer review" to try and get some insight into whether indeed copying config from the existing place is what would qualify as the 80% use case, vs. starting from scratch and trying to remember what all you changed, but it makes sense to me.

(Oops, cross-posted with Gábor. :))

I am moving this back to NW based on @webchick's suggestion in #44. I will update the issue description when I have an updated patch.

The attached patch implements @webchick's suggestion from #44 above.

Most of the interdiff amounts to reverting changes to the "Add field" page. The new code is mostly (maybe all) in the method getExistingFieldDefaults(). This finds an existing field using the selected field storage and copies its configuration. Note the @todo comment there.

Feedback wanted!

I brought this issue up again at the UX meeting yesterday (Oct. 10). We agreed that the next step is to get an idea of what the common use cases are. I would like to see comments along the lines of

1. I like the way Drupal works without this patch.
2. I want to copy configuration from an existing field, and I do not care which one.
3. I want to copy configuration from an existing field, but I need to select which one.

Or a variant of (2) like "I sometimes care, but not enough that I want to make the selection every time."

If, as in the patch from #46, we let Drupal pick a bundle from which to copy configuration, then I have some bad news. I was hoping that I could use either the first bundle for which the field was configured or the most recently configured bundle. Unfortunately, no timestamp is stored in the database. The field configuration is stored in the key_value table and EntityFieldManager::getFieldMap() relies on KeyValueStore\DatabaseStorage::getAll() to fetch the data. Neither of these methods makes any promises about the order of the results it returns.

We can choose either the first or last bundle listed and hope for the best. Or we could sort the list of bundles (alphabetically seems as good as anything) and take the first or last.

My personal choice is 3 - everyone I've talked to agrees.

I've collected some example use cases and opinions and found use cases for all of these:

1. Field settings are the same for all bundles (including form display and default display)
2. Field setting and form display settings are the same but display settings are different per bundle (all bundles using the default display mode)
3. Field setting and form display settings are the same but display settings are different per bundle (bundles have custom display modes)
4. Both field settings and display settings are different

The first one is the most popular.

So there are too many use cases. When I'll have time I can write them so we can use them to test the patch.

As a side note, most of the times when the first case is true, site builders need a "copy by reference", when a field setting or display change is applied to all bundles automatically. I know, that's not the focus of this issue and this feature does not exists in Drupal, but it's worth mentioning.

One idea that came up today in the weekly UX meeting is that we should not just be looking at this page. Maybe, when I am looking at the "Manage fields" page for the Article content type, I should be able to clone the Tags field there and add it to the Basic Page content type. There is already a drop-down list for each field: Edit, Storage settings, Delete. We could add the option Clone to that list.

As a first step to implementing this, I think we should refactor the current patch and move more of the code out of the FieldStorageAddForm class.

A related, and much more ambitious, issue mentioned at the UX meeting is #2539740: Provide users with a visual way to understand and build content types (dream fields) as an experimental module..

@benjifisher: re #49, before jumping on a different implementation, I would let this issue collect some feedback (there is some on the tweet I posted at https://twitter.com/gaborhojtsy/status/920372286254919683, eg. some people saying they literally copy-paste the YMLs to do what you are doing here :). If we are to make decisions based on feedback on twitter, we can/should copy the notes from the twitter thread here once the response rate dies down, so it is kept for posterity.

In order to make it easier to experiment with this issue, I have created a sandbox module: https://www.drupal.org/sandbox/benji/field_clone.

The latest version (git tag 8.x-1.0) is functionally equivalent to the patch from Comment #46 above. Once you enable the module and visit /admin/structure/types/manage/page/fields (for the Basic page content type) there will be a new link, "Clone field", next to the "Add field" link. The new link takes you to /admin/structure/types/manage/page/fields/clone-field. This new form looks like the one at /admin/structure/types/manage/page/fields/add-field, but when you submit the new form, configuration will be copied from one of your existing content types.

adding a comment to present a use case, to help #48 https://www.drupal.org/project/drupal/issues/2717319#comment-12303504

I have a decent sized project where I'm trying to standardize fields (not have a sprawl and glut of fields...)

On a custom block type I made a field named:

field_block_paragraph_ref_01

This is an Entity Reference Revisions field targeting a Paragraph.

This asks : "Type of item to reference" . (I select paragraph, by default)

and "Allowed number of values". (In this case unlimited)

It then asks me for specific field information, such as help text, required, default value, and (most importantly) Reference Type.

When this is used for the field for the FIRST TIME, it works perfect.

When I reuse the field again on a different custom block type, and select a different paragraph type for use on that different block type, I get the wrong interface widget on the block add screen.

On Custom Block Type "CTA Help"
I have "/admin/structure/block/block-content/manage/cta_help/fields/block_content.cta_help.field_block_paragraph_ref_01" as a path to the edit screen, where I have selected a paragraph type.

On another custom block type "FA Card"
I have "/admin/structure/block/block-content/manage/fa_card/fields/block_content.fa_card.field_block_paragraph_ref_01" which is an attempt to reuse the same field on a different custom block type and be able to select a DIFFERENT paragraph type.

You can see an important difference

On the 2nd use of the field, it has options to select a default, where those don't exist in the first use.

When I go to try and add a custom block, it shows the wrong widget on the 2nd use of the field.

If I make a completely NEW field, and select the paragraph type I want. It works.

It was my expectation that I could reuse a field, and have different reference types selected between the reuses and it use the correct widgets.

Hopefully this helps give a good use case scenario that (imho) should work.

Thanks!

@alphex:

Thanks for the detailed comment! Just to be clear, you are describing the behavior without any patches from this issue, right?

When I go to try and add a custom block, it shows the wrong widget on the 2nd use of the field.

I think if you visit "Manage form display" for the second block type and save the form, then you should get the Paragraphs widget instead of the autocomplete field. That is, we understand this behavior even though we do not like it.

I did not previously notice that you have the option to set a default the second time you use the field. I guess that is a side effect of using the autocomplete widget.

I also notice that the three paragraph types (bottom of your second, third, and fourth screenshots) are not always in the same order. That seems odd. Did you do anything to rearrange them?

There are no active patch files -- setting to needs work.

From #49:

[W]hen I am looking at the "Manage fields" page for the Article content type, I should be able to clone the Tags field there and add it to the Basic Page content type. There is already a drop-down list for each field: Edit, Storage settings, Delete. We could add the option Clone to that list.

Yes, let's add that as well. Contextual links are better.

From #56:

There are no active patch files -- setting to needs work.

I believe that's incorrect; folks should be reviewing #46. However, I'll leave this as NW given the other thing I'm saying in this comment (above).

In other news, as nobody's mentioned it yet, there's a Drush command for this, but it's currently broken because the field commands haven't been ported for D8 yet.

Still needs work because we're still missing the contextual links (or maybe that can be a follow-up?), but let's get some feedback from the test bot. Here's a re-roll for 8.8.

Sorry, but interdiff is failing (I'm assuming due to the different baselines). Is there another way to make it work?

colan@leopard[Fri 6 14:28]% interdiff /tmp/2717319-reuse-fields-46.patch /tmp/drupal-clone_fields-2717319-60.patch                             
1 out of 1 hunk FAILED -- saving rejects to file /tmp/interdiff-1.uWUobN.rej
interdiff: Error applying patch1 to reconstructed file

@colan: I am glad to see some interest in this issue! I have been distracted by other things ...

One idea that I think I have not previously mentioned on this issue is to provide the choice one step later. That is, keep the "Add field" page the same. On the next page, provide a drop-down list of entities that already have the field attached (defaults to empty for backwards compatibility). Selecting an item from the list will auto-fill the other fields on this page. Maybe also an option to set options on view modes, which are configured elsewhere.

@benjifisher: I suppose that could work too.

In any case, I'm finally in a position to test the upgraded patch, and it doesn't appear to do anything at all. There's no new "Clone field" button on the Manage Fields page, and it doesn't show up either when grepping though the patch. Am I missing something? I do see it in your sandbox though. Did they diverge at some point? I'm hoping I didn't waste time upgrading the wrong thing.

For future reference, Field Tools may provide some of what we need here.

While I did get the sandbox code working, I've given up on this approach because it doesn't support cloning fields within the same bundle (which is really what I'm after). The only fields that show up when attempting to clone are those in other content types, not the current one.

As such, I've moved on to the issue I posted in my previous comment, where I'm currently working. Field Tools already provides much of the framework to make this possible so there's more to work with.

Having said that, though, I don't see why we couldn't get that module (or something like it) into Core at some point. It already adds a Clone contextual link to each field on the Manage Fields page, with the next screen asking for the target bundles (as a bunch of checkboxes). While from a UX perspective, it feels more natural to bring a field into the current context rather than sending one elsewhere, it may be a good place to start.

The option to clone the current bundle is currently disabled there; that's what I'm adding support for.

It would improve site builder experience to alphabetize the Re-use an existing field by field name rather than field type. This seems related to this issue, but also possible a different issue.

The reason for alphabetizing the field by name is that it's easier to find fields in an alphabetized list sorted by name.

The current sort is by field type then by name. So, to find a field in the list of say, 80 fields, you first have look up the field and find its type, then look for it within the list of fields of those types. Or, you have to scan each field type by name 5 or 6 times until you find the field name you're looking for.

Rerolled the latest patch for 10.1.x.

This should handle some of the test failures.

Addressing phpcs violations

Looks like I accidentally posted an earlier iteration of the patch.. This should address some of the test failures.

Since the thread is pretty long with several different ideas, I decided to test the current patch. I believe what this patch does is preset the manage form and manage display settings for a re-used field. I confirmed that the patch works great for that feature. This is a good improvement for image fields. I constantly forget to set the image style.

I do not have time to test, but I see this comment in the code:

+      // If $existing_bundle is not supplied, then find a suitable candidate.
+      // @todo Consistently choose the bundle. Oldest? Newest?

That is a serious usability question, and that is why this issue stalled back when I was working on it actively. In Comment #62, I suggested a different approach: offer a select list of bundles that already use the same field storage, and fill in configuration with JS when the user selects an option.

Interesting. I didn't test it with multiple types that use the same field with different settings.

#85: I removed the @todo comment and added sorting to make the behavior deterministic. I believe we should move discussing the option to select bundle to a follow-up. This issue in its current scope is already a net win.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Addressing PHPCS violations.

Most of these are maybes or questions, I'm guessing the switch back to NR shouldn't take long 😎
.

1. Could we get a test only patch with testReuseField()?

2. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -566,6 +577,139 @@ protected function getExistingFieldLabels(array $field_names) {
   +   *   'entity_view_display' if these are defined for the existing field. If the
   

   Should this clarify "in an instance of the existing field"?

3. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -566,6 +577,139 @@ protected function getExistingFieldLabels(array $field_names) {
   +      if ($existing_bundle == $this->bundle) {
   

   Is the loose equivalence intentional here?

4. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -566,6 +577,139 @@ protected function getExistingFieldLabels(array $field_names) {
   +      if ($existing_bundle == $this->bundle) {
   +        return [];
   +      }
   +      if (empty($field_map[$this->entityTypeId][$field_name]['bundles'][$existing_bundle])) {
   +        return [];
   +      }
   

   Is there a benefit in combining the conditions since they both return an empty array? Maybe it's to benefit readability?

5. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -566,6 +577,139 @@ protected function getExistingFieldLabels(array $field_names) {
   +      $existing_bundle = reset($bundles);
   

   I was going to suggest a potentially better way to pick the ideal existing bundle, but I think that can wait to a followup as what is happening here is a net improvement already. Refining this approach could delay this welcome improvement from landing.

6. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -566,6 +577,139 @@ protected function getExistingFieldLabels(array $field_names) {
   +      'default_value' => $existing_field->getDefaultValueLiteral(),
   +      'default_value_callback' => $existing_field->getDefaultValueCallback(),
   

   I could see the default values being copied over potentially being of concern, it's probably worth an explanation why this is OK to do in anticipation of such concerns.

7. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -566,6 +577,139 @@ protected function getExistingFieldLabels(array $field_names) {
   +    $field_type_definition = $this->fieldTypePluginManager
   +      ->getDefinition($field_name);
   +    $options = $this->fieldTypePluginManager->getPreconfiguredOptions($field_type_definition['id']);
   

   The line break before ->getDefinition() seems odd since the next line has no breaks and is longer than if 677-678 were all on one line.

#91.1 👍
#91.2 👍
#91.3 👍
#91.4 We certainly could combine those rules but the if condition would become harder to read. IMO what is there currently is easier to read but I don't feel strongly about that.
#91.5 👍
#91.6 What are the concerns with copying over the default value? It seems confusing from the UX perspective if we would leave some of the configuration options out. The user is always taken to the configuration form when the instance is created, so if something is off, they have an opportunity to correct that.
#91.7 👍

#92 failed because of a known random failure.

+++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
@@ -468,25 +464,40 @@ protected function configureEntityFormDisplay($field_name, $widget_id = NULL, ar
+  protected function configureEntityViewDisplay($field_name, array $formatter_settings = []) {

Since we're adding this function, but it's never used with the $formatter_settings arg, nor does that use case have test coverage, perhaps we can do away with that arg & related logic altogether?

I agree that we can remove the argument 👍

The patch looks pretty good! I only read the code and haven't tested it locally yet, so here's a few remarks for now:

1. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -442,25 +425,38 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   +   * @param array[] $widget_settings
   
   @@ -468,25 +464,40 @@ protected function configureEntityFormDisplay($field_name, $widget_id = NULL, ar
   +   * @param string[][] $formatter_settings
   

   The @param for these two doesn't seem to match, but they should...

2. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -442,25 +425,38 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   +   *   (optional) Array of widget settings, keyed by form mode. Defaults to [].
   
   @@ -468,25 +464,40 @@ protected function configureEntityFormDisplay($field_name, $widget_id = NULL, ar
   +   *   formatter. Defaults to [].
   

   I think we usually write (Defaults to) "an empty array" in documentation text, not the PHP syntax for it.

3. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -442,25 +425,38 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   +    // in other form modes  until it is explicitly configured. When an existing
   

   Double space between "modes" and "until" :)

4. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -442,25 +425,38 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   +    $form_modes = $this->entityDisplayRepository->getFormModes('node');
   
   @@ -468,25 +464,40 @@ protected function configureEntityFormDisplay($field_name, $widget_id = NULL, ar
   +    $view_modes = $this->entityDisplayRepository->getViewModes('node');
   

   This kinda shows that the patch was only tested with nodes :) Any chance we can also add tests for another entity type?

5. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -442,25 +425,38 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   +      $form = $this->entityDisplayRepository->getFormDisplay($this->entityTypeId, $this->bundle, $mode);
   

   Can we name this variable $form_display? The current name is very confusing.

6. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -442,25 +425,38 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   +      $this->messenger()->addMessage($this->t('The following form mode(s) were enabled: %enabled_modes', [
   
   @@ -468,25 +464,40 @@ protected function configureEntityFormDisplay($field_name, $widget_id = NULL, ar
   +      $this->messenger()->addMessage($this->t('The following view mode(s) were enabled: %enabled_modes', [
   

   It would be better if we'd use formatPlural() here.

   Also, should we exclude default from $enabled_modes? By only scanning the code in this patch, it seems the user would get (at least) 3 messages when adding a new field:

   • Your settings have been saved.
   • The following form mode(s) were enabled: Default
   • The following view mode(s) were enabled: Default

7. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -468,25 +464,40 @@ protected function configureEntityFormDisplay($field_name, $widget_id = NULL, ar
   +      $view = $this->entityDisplayRepository->getViewDisplay($this->entityTypeId, $this->bundle, $mode);
   

   Same here: $view -> $view_display.

8. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -558,6 +569,138 @@ protected function getExistingFieldLabels(array $field_names) {
   +   * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException
   +   * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException
   

   I can't see anywhere in this method a place where these exceptions can be thrown.

9. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -558,6 +569,138 @@ protected function getExistingFieldLabels(array $field_names) {
   +   *   The machine name of the field. The field class must implement
   +   *   Drupal\Core\Field\PreconfiguredFieldUiOptionsInterface.
   ...
   +    $field_options = $options[$preset_key];
   

   This requirement seems a bit artificial since getPreconfiguredOptions() will return an empty array if the field type class doesn't implement that interface.

   Looks like we can remove the "requirement" by defaulting $options[$preset_key] to an empty array.

10. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
    @@ -558,6 +569,138 @@ protected function getExistingFieldLabels(array $field_names) {
    +   *   An array of settings with keys 'field_config', 'entity_form_display', and
    

    This method (::getNewFieldDefaults()) also returns a field_storage_config key.

Thanks @amateescu! This should address all of your feedback. 😊

After a lot of contemplating, I decided to remove the functionality that enables new view/form modes because even after spending time reading comments on this issue, it's hard to understand when I would expect that to happen. If we still think that's something we'd like to do, we should do that in a follow-up with some additional research to validate that it is what users expect.

This issue would be epic to land together with #3346682: Improve re-use an existing field user experience.

Tested the patch in the UI and it works great. Then went through the code one more time, found just a few variable names that could be more clear (similar to the $form -> $form_display point from #97), but it's not worth holding up this issue for them :)

I did another review of the code, but after the very thorough amateescu review this appear to be in great shape.

I have the 10.0.x patch available if there is interest in backporting. This is an objective improvement, but enough of a change that I'm hesitant to include it in a patch release. There's been enough times recently that I committed something to just 10.1.x then became aware of interest to backport that I figured having the patch at the ready in #101 would be a good idea.

This is now committed to 10.1.x. It's a really nice improvement, and always nice to see completion of an issue that was filed 2 months before the first season of Stranger Things premiered.

I did a little demo of this issue at #3348736: Drupal Usability Meeting 2023-03-24. It starts at about 17:17 in the recording (YouTube).

I showed the effect on the selected image style for an Image field and the form widget for a Paragraph (ERR) field.

This is a nice improvement. I don't think this is eligible for backport as it does make a change to the UI, but tagging so we can highlight it in the 10.1.0 announcement.