Problem/Motivation

field_ui.js contains an @todo above buggy code, this fixes it.
The todo was added in #1792600: Refactor field_ui so common behavior for fields and display overview screens are extracted.

#1786198: Make consistent regions in code for fields UI overview screens intended to fix this as part of a larger refactor.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#2 2796885-field_ui-2.patch709 bytestim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett created an issue. See original summary.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Active » Needs review
FileSize
2796885-field_ui-2.patch709 bytes

This soft-blocks #2796173: Add experimental Field Layout module to allow entity view/form modes to switch between layouts.

droplet’s picture

droplet CreditAttribution: droplet commented

the value passed from `Drupal.fieldUIDisplayOverview.field.prototype.getRegion`. I think it has to update too.

Let's check other tabledrag usages to see if we should change `region` to BOOLEAN
regionChange: function (region)

EDITED

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented

the value passed from `Drupal.fieldUIDisplayOverview.field.prototype.regionChange`. I think it has to update too.

I have no idea what this means. That's the function I'm changing.

Let's check other tabledrag usages to see if we should change `region` to BOOLEAN

In Display Suite, Drupal.fieldUIDisplayOverview.ds.prototype.regionChange needs the actual string.
So does the code I'm working on for core.

droplet’s picture

droplet CreditAttribution: droplet commented

Oh, sorry. This is `getRegion`
@see https://github.com/drupalprojects/drupal/blob/9bb1c3691c257f1a89c7545da9...

@see
https://github.com/drupalprojects/drupal/blob/9bb1c3691c257f1a89c7545da9...

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented

Ah yes! That also needs to change, but would need PHP changes in either #1786198: Make consistent regions in code for fields UI overview screens or #2796581: Fields must store their region in entity displays to work. This is a simple change that just addresses this @todo.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer commented
Status: Needs review » Reviewed & tested by the community

@todo addressed. Good to go.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Now we have JavascriptTestBase we should include test coverage - obviously when the comment was written something gave the author pause before changing it.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Needs work » Closed (duplicate)

At this point, we can just mark this as a duplicate of #2796581: Fields must store their region in entity displays, which makes this same change AND already has a JavascriptTestBase test.