Problem

As determined in #3446302: Improve entity display editing UI the form attaches the library `field_ui/drupal.field_ui'`. This library does some complex ajax logic we don't really needed.

Proposed resolution

Simplify things by dropping `field_ui/drupal.field_ui'` and adding a custom JS for what we really need, if anything.

What I see needed:
* AJAX reloads for handling the settings forms. That seems to be handled separately from `field_ui/drupal.field_ui'` via regular form ajax.
* Some logic to add/remove fields. We don't need regions. We only need to store region 'hidden' if disabled, and 'content' if enabled.

As UI for removing/hiding fields I think it would be much cleaner to drop the long-list of hidden fields. Instead, we should simply not render hidden fields but show a small select "Add fields", it lists hidden-fields and has a AJAX "Add field" button which reloads the form.

Then we need to have some alternative for removing a field. So some "X" button in the end, next to the settings button should be good enough there.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

fago created an issue. See original summary.

roderik’s picture

Agreed, and this is also necessary / a prerequisite to more easily support

roderik’s picture

Status: Active » Needs work

This ticket is an unfortunate mess, at the moment. I have two independent branches:

1.

form-rows-as-components branch. We need to move away from "one row in the form == one field", because in the new paradigm enables us to add one field several times (with different formatters) and we want to support things like #3446287: Allow configuration of static props. This branch will also clean up much of the messy comments / temporary code in the form, that was working towards this situation.

I however shelved this for now. I'm at the point where the basic properties in the form (name, formatter) rebuild and save correctly, but the plugin settings form does not show.

I will likely need to re-index the plugin settings form from "field name" to "new row name" too, and I'm not sure what issues I'll encounter there / how long they will take.

So I shelved this, and will continue working on this later (probably in a followup issue that is going to be a prerequisite for #3446287: Allow configuration of static props if not #3446287 itself -- maybe titled "allow a field to be added multiple times").

2.

3446485-ajax branch. Form row names are still named after the fields.

  • I hacked some more temporary helper code in order to build only the fields used by the CE display in the "Content" region (instead of all of them, as the Core form does
  • (This requires EntityDisplayFormBase::form() to be copied wholesale into the form, and then changed)
  • (and will very likely require a modified 'field_ui_table' form element, so I copied the FieldUiTable class to CustomElementsFieldUiTable, to be freer in hacking away the field_ui JS)
  • ...and only the fields which are not in those rows, can be aded using the "Add" AJAX functionality.

However, I have not yet done anything substantial yet, like

  • added an AJAX callback to the "add" select element
  • untied Core AJAX functionality from the "name" row element (which should not have a callback, it just causes a bug, per the bold warning that is now on the screen)
  • check how to implement the deletion. (Deletion is possible currently by explicitly selecting "Disabled" in the region dropdown, and then saving. That is obviously not good UX.)

I have 60-90 minutes more to hack on this before I'm yanked away, and then..... we'll need to plan next steps for the coming 6 days while I'm basically gone. Or: not plan, and I'll pick this up immediately as soon as I can / as I return.

roderik’s picture

Assigned: Unassigned » roderik
roderik’s picture

StatusFileSize
new178.9 KB

Current status of the "3446485-ajax" branch (rebased on current 3.x and tidied): Two out of three points in the previous message are implemented:

  • untied Core AJAX functionality from the "name" + "is_slot" row elements, so those bugs are gone.
    • I needed to clone the field_ui JS library for this, into the module - just to make one little change. But I guess we expected that already.
  • I did keep the "region" dropdowns as a means for deletion, because anything else would have required hacking tabledrag.js as well. IMHO this is "good enough" and if we want an actual delete button, that would be a post-stable-release followup.
    • The "region" selector is now always shown on screen, and renamed to "Delete?". See screenshot.
    • "region" is removed from the config schema now. I think this requires no CR because this is auto-removed on save, and we never did anything with the value anyway.

Region is delete

Left to do: the (AJAX) functionality for adding a field to the display.

fago’s picture

yeah, I agree changing the tabledrag.js for a better deletion XP should be post 1.0. Meanwhile maybe, "Action" would be a good name for the region-column instead of "Delete?" ?

Related, Formatter+Settings really belongs together, so I think it should be moved either once left or right.

roderik’s picture

Status: Needs work » Needs review
StatusFileSize
new234.47 KB

You're right. I moved the "delete dropdown" to the right.

new screen

Things are working now. I think I tested everything.

roderik’s picture

Assigned: roderik » Unassigned
fago’s picture

Status: Needs review » Fixed

Seems all great! Merged!

fago’s picture

Status: Fixed » Closed (fixed)

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