Move label and machine name fields to the field config edit form

This was unblocked by 📌 Make field selection a two-step form Fixed .

Just to clarify: if the first step of the "Add field" form selects a field type (like Boolean) and not a field group (like Reference), then we should skip the second step and go straight to the field configuration form.

Maybe we should move some code from the submit method of the "Add field" form to the buildForm method of the field configuration form. I am not sure how that would affect the "Re-use an existing field" form.

The second step in case of ungrouped field types mentioned in #3 isn't implemented yet. It will be affected by the issue that removes the radio buttons from the field cards in the first step: 📌 Field selection breaks conventions and increases cognitive load Needs review . I think we should wait on those changes to be committed to avoid re-work here.

omkar.podey → made their first commit to this issue’s fork.

The ajax callback was causing the form to be rebuilt which prevented it from being saved the first time, now fixing the issue with the correct order of execution as we should run the validation checks earlier and not let the temp field to be saved early.

Now the tests are failing because of two main reasons.

1. The content translation module somehow doesn't appear to be installed, while works fine in UI.
2. The default value field is created with the random temp field name so have to figure out a way to assert that.

Hi,
I have tested MR !5992 on the Drupal version 11.x

Testing steps:

1. Install Drupal 11.x
2. Go to any content type and add a field type like boolean (not a field group like Reference)
3. Check for the 2nd step having the Label and machine name
4. Apply branch and again check for add field

Test Result:
The patch is applied successfully, and the second step form is skipped and directly goes to the field configuration form for the Boolean type field which does not have a field group like Reference.

Attaching screenshots for reference.

Moving to RTBC

hm i've tried to apply the patch to a completely fresh install of drupal 11.x-dev installed with the standard profile. but somehow i get Could not apply patch! Skipping. The error was: Cannot apply patch https://git.drupalcode.org/project/drupal/-/merge_requests/5992.diff when i try to apply the diff with composer patches. but "somehow" some" changes or all(?) changes got applied. but not sure if that is an actual problem with the patchor a problem due to the way composer-patches applies the patch, just wanted to note.

but changes are in place, at least in the UI, so i was able to give it a quick try. definitely the right choice moving the label/machine_name field over to the field config edit form. looks great!

one detail to note. 📌 [PP-1] Replace the "Change field type" button with one that goes to the previous step Postponed which is currently postponed on this issue is a sort of essential addition to this issue from my perspective. because currently the only way to move one step back as soon as you are on the field config edit form is the back button of your browser. there should definitely be a dedicated option to navigate back and forth in the interface through all the steps until the field is finally saved in this last step. otherwise one might feel stuck arriving on this field config edit form in case the person had clicked the wrong field type or option in one of the previous steps.

Another detail I've just noticed. For the field type Media you directly get to the field settings form now. "problem" is like Field upload pre patch it had a description underneath explaining when to use media reference fields and when to use file and image reference fields. For Field upload that description is still shown on step two when you are able to choose the option, that second step got removed for Media and so is the description. I am not sure if it would make sense to squeeze it onto the field settings page instead for media. but i wonder if it would make sense to create a dedicate help topics page in a follow up where the user gets an overview of all available fields in core and when best to use which field. that you have all available options and background information for each field type on a single page.

@Kanchan Bhogade: Thanks for testing, but please do not move an issue to RTBC unless it has also passed code review. The R is for review and the T is for test.

@rkoller: I am moving the issue back to NW for Comment #13. Losing the notice about media and image fields is a regression. I am adding a note to the issue summary so that we do not lose track of this regression.

I do not think we have to worry about applying the diff as a patch (Comment #12). You should be able to clone the issue fork and check out the feature branch using the instructions near the top of the issue,

I think it is OK to keep 📌 [PP-1] Replace the "Change field type" button with one that goes to the previous step Postponed as a separate issue. If we can get this issue fixed, then I think it is a step in the right direction even without that improvement. I hope we can get both done in time for Drupal 10.3.

I am moving the issue back to NW for Comment #13. Losing the notice about media and image fields is a regression. I am adding a note to the issue summary so that we do not lose track of this regression.

I'm not 100% sure the message is needed for the media type at least when not editing fields for media entity. Maybe on this issue we move the message to the field config edit form and discuss in a follow-up if we should not display it when editing fields for other entity types than media.

I am not sure if the field settings edit form is the best place even as a temporary place for that description. That form and page is the last step in the field creation flow, step 2 for media and step 3 for file upload with this patch applied. that description ideally would be more useful at the point where the person trying to create a field and either doesn't know anything about the different field types or is uncertain which field type is the right pick, in case of the media and file upload example, and needs a reminder.
i wonder if it would make sense to remove both the descriptions for media and file upload in step 2 and 3. instead incorporate the brief gist into their short descriptions in step one (i am currently working on a draft for 📌 Refine field descriptions Active ). in addition to that provide a description pointing to a to be created help topic page providing an overview for a "when to pick which field type" situation. But I would place the description pointing to the help topics page before the list of available field types in step 1 instead of after.

Then maybe something like this

could work, just having the general message appear on the initial field selection whenever media is enabled.

After applied MR !5992, Message appear bottom of the choose field type.

Before MR

After MR

RTBC ++.

I'm not sure about adding info to the bottom of this form specific to choosing media. The options themselves already include description text specific to that option - and option-specific descriptions should live there. Ideally the current media description text could be shortened (its longer than it needs to be IMO) to make room for this info. Having it as a footnote technically addresses the regression, but I don't think it actually improves the experience.

If rewriting that description is not a feasible option, at the very least associate the bottom-of-the-form text with the media option with an asterisk. In its current state I think it is unlikely a user will even read it - they will see the option they want then click it to proceed.

Does this seem better and attracts enough attention to the note?

Does this seem better and attracts enough attention to the note?

It's better than not having the association, but it's not great. I think updating the descriptions has a better result as it keeps the UI clean, removes the edge case, and the relevant information is in the correct context. Also less code needed 🙂

Something like this looks better and I think has a higher chance of benefitting a user vs. the current tacked on message.

Updated descriptions, more suggestions ?

This stuff is really tricky to understand, so nice work @omkar.podey! That said, I did a code review and found some stuff that should probably be fixed, plus some questions that need answering.

See a lot of conversation in the threads and feedback (60 threads!) This good for review now?

I've asked @omkar.podey to mark resolved threads with ✅ so @srishtiiee can close them

Appreciate it!

So testing was pretty straight forward. Applied the MR and verified label is appearing on config edit form.
Tested with a few field types (had 0 doubt this would matter but double checked) and no issue creating fields.
Remaining tasks appear to be complete.
Issue summary contains a screenshot of the new change.

+1 from me but may be worth getting additional eyes.

From what I can tell all feedback has been addressed.

There are still unresolved threads.

Appears to be a few open threads or is this ready for testing?

yep it is ready to be tested

Rebased the MR as thought it had been a while, nothing broke

Retested following same steps

Applied the MR and verified label is appearing on config edit form.
Tested with a few field types (had 0 doubt this would matter but double checked) and no issue creating fields.

Believe all threads have been fixed or answered

For me this looks good.

There are unresolved threads on the MR for things that are very confusing to me.

I simplified the whole code block by removing the creation of new field storage config and hence moving away from the whole translatable key issue.

Seems to be 1 open question left a comment. Once answered don't mind marking

Keep this one from stalling going to mark as it's still functional.

Replied on the MR - but this shouldn't have been RTBC again, the thread is still outstanding.

MR appears to have some failures.

Appears feedback has been addressed.

With this patch applied, I'm getting a weird behavior where clicking "Save" on the field configuration form the first time causes the page to jump back to the top, and I have to click "Save" a second time in order to save the configuration. It does not seem to happen with HEAD. Steps to reproduce:

1. Install Standard on 11.x HEAD with the patch/MR applied.
2. Go to admin/structure/types/manage/article/fields
3. Click "Create a new field".
4. Select "Boolean" (it occurs for other field types, but this one's simplest)
5. Enter a label.
6. Click "Save". Note that the cursor jumps to the top of the page.
7. Click "Save" a second time and note that the field is only then saved.

I've been able to reproduce this consistently with the patch, and able to consistently reproduce it not happening with HEAD.

Actually seeing the same as @xjm appears the code that runs the machine name could be causing it.

If I enter a label
Click out of the label input (any where on the page)
Click save I don't get that behavior.

I also disagree that losing this message is an acceptable regression or scopable to a followup:

This message was an important compromise for the stabilization of Media, to dissuade people from creating a bunch of difficult-to-migrate image data when they should have been using Media. That single paragraph was the compromise for giving up built-in redirects, a migration tool, removal of image fields from the UI except on Media types, etc. etc. etc. It does not have the greatest UX -- ideally it would be styled as a little info box rather than a blah paragraph of text, or something even more assertive -- but that doesn't mean it's okay to lose it entirely.

I confirmed what @smustgrave said above: If you click out of the label field, the bug does not happen. (Clicking in or out of the label field in HEAD had no effect; it worked every time.)

I also reproduced #47 in Firefox, both it not happening in HEAD, and it happening with the patch applied.

Another regression is in the page title for the field configuration page. In HEAD we have:

With the patch it's simply:

This sounds like it's all field settings for articles, rather than the settings of one particular field.

I understand that we don't have the field name when the page is loaded, but could we at least put the field type in the title? E.g.:

Boolean field settings for Article

I finally finished my manual testing and my code review -- phew! Overall this is in pretty good shape, and it's a really good usability improvement on the whole. IMO there are three regressions that need addressing (see #47, #49, and #51), plus a few small cleanups, but on the whole it's close to ready. Great work everyone!

Saving issue credits.

Tagging per my comment above about the temporary label and machine name being generated to make the form work.

Re #53 I don't have a better idea, but I found that incredibly confusing reading the MR (like deleting config entities that would never actually exist etc.). Definitely could use a follow-up to revisit. I haven't reviewed this since my last round of feedback was addressed.

The screenshot in the issue summary needs an update. The page title now includes the field type. For example, when adding a boolean field, the page title is "Boolean settings for Basic page Add to Default shortcuts", not "Field settings for Basic page Add to Default shortcuts".

After I save the new field, I can still edit it. When I do that, the page title still includes the field type (like "Boolean") instead of the field name (like "Test boolean field"). The same thing happens with existing fields. For example, with the Standard profile I can visit /admin/structure/types/manage/page/fields/node.page.body, and I see the page title "Text_with_summary settings for Basic page". I think we should use the field name when it is available.

I also left some comments on the MR. We need the usual BC steps when adding a parameter to the constructor, and that means we need a change record. I am adding the issue tag for that.

Looking at the tags and can't answer the release or framework manager. But can the issue summary be updated from #56 (should help the manager review later).

Left a small comment on MR. Also appears to have a number of test failures. Some of them could need a rebase but didn't check each.