Use modals on the Manage Fields page

Rerolled #47 to 10.1.x.

The last submitted patch, 51: 2880003-51.patch, failed testing. View results →

Added an MR with all tests passing. Left a few things that need addressing in the MR.

MR appears to have issues.

Could there be a way for a site to opt out of this? Modals are neat but for a number of our sites I don't imagine we would want this.
Tagging for usability and accessibility review.

Was previously tagged for issue summary which still appears to need and happen.

Thank you for your feedback @smustgrave! At the moment there isn't a way to opt out of this. I'm curious to know why you believe using modals would be a regression for your sites. 😊 Is there something that we should change about the implementation that would make the user experience more seamless?

Let me ask, could there be a scenario where a custom module with a custom field could break with this change? And there be no way to edit your field.

Other concern is accessibility. Know Drupal backend is not the best with that but adding to it would seem bad to me.

This almost reminds of the overlay module of Drupal 7. Think I got that right.

Let me ask, could there be a scenario where a custom module with a custom field could break with this change? And there be no way to edit your field.

You're right that this could be disruptive to a custom field type which configuration form relies on JavaScript (and the JavaScript hasn't been written according to Drupals best practices). Do we have any examples of field types that would ship JavaScript for their admin form?

There certainly would be workarounds (i.e. open the link in a new tab) to this but it wouldn't be ideal to have users rely on that. We would also do something like this only in a future minor giving people time to test and fix their custom code.

Other concern is accessibility. Know Drupal backend is not the best with that but adding to it would seem bad to me.

Do you have specific accessibility concerns in mind? I would love to make sure that we are taking those into account 😊

We certainly want to make sure that the solution is accessible and easy to use for all users. This has already received some feedback regarding accessibility from @bnjmnm who is a provisional accessibility topic maintainer, and I know that he's keeping a close eye on this. We are also using dialogs in some pre-existing UIs such as Layout Builder, meaning that it is a pre-existing pattern which has received review in the past.

This almost reminds of the overlay module of Drupal 7. Think I got that right.

Overlay opened all of the admin pages in a dialog. While this is also utilizing dialogs, it is not exactly the same since we are only using it for specific actions. This is similar to the block UI where a block can be added through a dialog, without having to navigate to a separate page. What is interesting about the block UI is that it is not using dialogs for editing blocks 🤔

Don't quote me but my only knowledge of modals is that focus must remain inside the modal while it's open for tabbed users. Must be able to be closed via tab. Not sure what/if any aria attributes it may need.

We are utilizing the built-in Drupal Dialog (which is using jQuery UI Dialog) which should have all of the necessary accessibility considerations implemented. AFAIK it cannot be closed with a tab and I've personally not heard of that before. If that's a requirement for modal dialogs to be accessible, we should probably open a new issue to investigate that in relation to all of our dialogs.

One issue we are aware of specifically in relation to this issue is that when a dialog is opened using a link that is inside dropbutton, focus is not returned to the triggering element because it is no longer focusable. This is something that will need to be addressed as part of this issue.

tim.plunkett → made their first commit to this issue’s fork.

I removed a lot of unrelated code that was from prototyping other Field UI changes.

I also fixed the next few failing steps in EntityReferenceAdminTest.

However, I highlighted (see the newly add @todo's) the problem we're still facing.
FormBuilder has this code:

    // If this form is an AJAX request, disable all form redirects.
    if ($ajax_form_request = $request->query->has(static::AJAX_FORM_REQUEST)) {
      $form_state->disableRedirect();
    }

which is good, because we don't want it to redirect, but bad, because we have to figure out how to share the destination logic between the submit and the ajaxSubmit.

Meaning, if JS is disabled, things work as expected, but with it on, the AJAX code doesn't know where to take you next. And each form in the chain needs to be fixed.

All of those concerns are for the Add Field flow, which is extremely complicated right now. That portion could be handled in a follow-up, while this can make the links in the dropbutton use a modal.
But until we decide to do that, postponing this on 📌 Combine field storage and field instance forms Fixed

Converting the field creation flow to use modal is blocked by 📌 Save FieldStorageConfig at the same time as FieldConfig Fixed . This would be also simplified by 📌 Combine field storage and field instance forms Fixed , but this isn't a hard requirement for working on this issue.

In Comment #46, @quietone asked for an issue summary update. I am not familiar enough with the current work to update the description myself, but as a first step I have added the standard template. I am keeping the "Needs issue summary update" issue tag.

In Comment #70, this issue was postponed on 📌 Combine field storage and field instance forms Fixed . Comment #72 also mentions 📌 Save FieldStorageConfig at the same time as FieldConfig Fixed . Both of those issues have landed, so I am un-postponing this issue. If it should still be postponed, then please mention the blocking issue in the "Remaining tasks" section of the issue summary. (See the link in Comment #46.)