Make field selection a two-step form

Similar work has been done as part of ✨ Use modals in field creation and field edit flow Needs work . I believe that the proposed resolution here is simpler than that work, and can be used as a first step in that issue. That issue has been under development for several months, and the current MR is very complex. I think that separating out this step will help simplify the work done for that issue, so that it can focus on using modals.

A second step after this issue would be to use modals for the two steps of this form, but still go to the Field Configuration form with a new page load. That would make the "Create a new field" and "Re-use existing field" links on the "Manage fields" page consistent, and then #3386762 could handle the rest of the field creation process.

We might even do that second step as part of this issue.

I ran the Functional tests for the field_ui module with the feature branch, and they passed. I am exercising the testbot, and I expect that some tests will fail.

I cannot run FunctionalJavascript tests locally. (I am not sure what changed. I used to have them working.) So I used the testbot and squashed-and-force-pushed a few times, but now the tests are passing.

I would like to see how hard it is to get the two steps of the form to work in a modal window. I think, in order to control scope, that should be a separate issue. But I might push a second branch to the issue fork for this issue, without creating a merge request, as a proof of concept.

The current MR is +316/-196 lines and changes 11 files. But it is simpler than those numbers suggest. 10 of the 11 files are tests with simple changes because of the two-step process. Most of the changes to FieldStorageAddForm (the only non-test file changed) are in the first four commits, which are just moving code around (refactoring into helper functions). After those four commits, the remaining changes are a manageable +172/-115, of which +73/-65 are in the form class.

This looks good if we can get this part in I can open up another issue to convert the first two steps in a modal.

@omkar.podey Prior to the patch, when selecting a reference, the second-step form would automatically appear. After the patch was applied, upon selecting a reference and clicking the continue button, the second form will now appear.

After patch Screenshot for reference.

Yep that is by design, this is the first step to converting these forms into modals 🌱 [Plan] Improve field creation experience Active . You can take a look at ✨ Use modals in field creation and field edit flow Needs work to get an idea for the new flow.

Added comment to the MR regarding a todo comment. Also the MR needs a rebase now that 📌 Provide alter for the field type UI definitions Fixed landed.

We have now group_field_options_wrapper , new_storage_type, and selected_field_type. I'm not sure what each of these are storing. 😅 Wonder if we need to rename some of them or at least document them somewhere?

I completely agree that those things should be changed. But in the interest of keeping the MR easy to review, I suggest doing them in a follow-up issue. The scope of this issue is to break the form up into two steps. As much as possible, it should keep the existing structure.

As I moved code around, I resisted the temptation to make changes like this, or to fix uses of the global t() function. There is a lot of code cleanup that I would like to do.

I did not realize that 📌 Provide alter for the field type UI definitions Fixed had been created as a separate issue. But that is exactly the sort of small, well-scoped issue that I have been advocating.

I completely agree that those things should be changed. But in the interest of keeping the MR easy to review, I suggest doing them in a follow-up issue. The scope of this issue is to break the form up into two steps. As much as possible, it should keep the existing structure.

+1 for a follow-up. Do you think we could add a brief comment explaining what each of these values are for the meanwhile?

As I moved code around, I resisted the temptation to make changes like this, or to fix uses of the global t() function. There is a lot of code cleanup that I would like to do.

I'm fine with not fixing the calls to t() if that's preferred 😇. I proposed the change for the lines that we were already modifying.

I did not realize that 📌 Provide alter for the field type UI definitions Fixed had been created as a separate issue. But that is exactly the sort of small, well-scoped issue that I have been advocating.

There's also 📌 Create a new OpenModalDialogWithUrl command Active now 👍

@Nitin shrivastava:

Thanks for the screenshots. We should add those, along with some of the "before" screenshots from ✨ Use modals in field creation and field edit flow Needs work , in the issue summary (under "User interface changes").

I rebased the MR. A few comments:

1. Comment #10 mentioned a possible conflict with 📌 Provide alter for the field type UI definitions Fixed . In fact, Git was smart enough to handle it. That is another reason to resist the temptation to make additional fixes: Git can figure out when lines are moved around, but not when they have also been modified.
2. I squashed three commits that I meant to combine before creating the MR.
3. I added a more descriptive commit message to one of @omkar.podey's commits.
4. I amended @omkar.podey's other commit, removing the unreferenced variable that PHPStan detected.

Regarding (1): Git correctly updated FieldStorageAddForm, but I had to update one of the tests. I am a little surprised that it was just one.

@omkar.podey: regarding (4), it is good practice to run static analysis before updating a MR. See https://www.drupal.org/docs/develop/automated-testing/run-core-developme... → .

From #12, referring to group_field_options_wrapper , new_storage_type, and selected_field_type:

Do you think we could add a brief comment explaining what each of these values are for the meanwhile?

No objection, but I am not going to do it today. Maybe over the weekend, or maybe someone else can do it. I think we can eliminate one of the three form keys in the clean-up issue to come. I think the redundancy and unhelpful naming comes from ✨ Make field selection less overwhelming by introducing groups Fixed .

I am setting the status back to NR.

Back to NW for a minor change. The MR looks neat otherwise after the multiple confusing keys have been cleaned up.

Thanks, RTBC'd

lauriii → closed merge request !5789

Committed da8a336 and pushed to 11.x. Thanks!

Created 📌 Move the first two steps of field creation into a modal. Needs work for the next steps (use modals for field creation).

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2023-12-15 Needs work , a few hours after it was fixed. That issue has a link to a recording of the meeting.

There was a strong consensus that this issue is a step in the right direction. The Usability team looks forward to continued improvements in some of the related issues: at least 📌 Field selection breaks conventions and increases cognitive load Needs review , 📌 Grid-style field type keyboard navigation Needs work , and 📌 [PP-1] Move label and machine name fields to the field config edit form Active .

For the record, the attendees at the usability meeting were @AaronMcHale, @Emma Horrell, @benjifisher, @rkoller, @simohell, and @worldlinemine.

Automatically closed - issue fixed for 2 weeks with no activity.