To reproduce the issue:
1. Install Field Collection module.
2. Create a Field Collection field on an entity with multiple allowed values.
3. Create a DER field in the Field Collection with multiple allowed values.
4. Create an instance of the entity and add an additional Field Collection.

The first field collection's Entity Type field will sometimes interfere with the second field collection's entity types for the autocomplete field.

CommentFileSizeAuthor
#14 make_the_js_class-2862964-12.patch2.77 KBjibran
#14 Screenshot from 2017-06-08 22-24-53.png62.74 KBjibran
#14 interdiff-b9477e.txt1.18 KBjibran
#9 screenshot-d8.dev-2017-05-29-08-55-59.png129.67 KBjibran
#9 2862964-9.patch2.84 KBjibran
#4 jquerydata-2862964-4.patch807 bytesgradywring
#2 jqueryid-2862964-2.patch791 bytesgradywring
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gradywring created an issue. See original summary.

gradywring’s picture

gradywring CreditAttribution: gradywring commented
FileSize
jqueryid-2862964-2.patch791 bytes

Attached a patch that uses the target.id of the Entity Type select list instead of a class as its jQuery selector. This ensures the .parents() call only returns one DOM element and the correct autocomplete field is affected by the Entity Type value change.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for University of Technology, Sydney commented

To reproduce this issue

  1. Have a content model approaching sadism :-P

That sounds like a world of pain.

But onto more serious things...

Target IDs change with each ajax request.

Will this work after you hit 'add another' for example.

Core has worked actively to move away from IDs, towards data- attributes. If class is no good, I'd prefer we did that here instead of using ID.

gradywring’s picture

gradywring CreditAttribution: gradywring commented
FileSize
jquerydata-2862964-4.patch807 bytes

I'm going to be honest with you, when I saw the requirements for this feature I thought, 'Well, how am I supposed to make that?' It's basically a block type that can display a 'flexible grid' of entities. So, a field collection represents each row and a DER field represents each column in that row.

Using ID's it did work after 'add another'. However, using data attributes is much better. Good catch. I've rolled a new patch and tested with my use-case.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for University of Technology, Sydney commented
Status: Active » Needs review

thanks

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for University of Technology, Sydney commented
Assigned: Unassigned » jibran

this looks good to me

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Version: 8.x-2.0-alpha2 » 8.x-1.x-dev
Assigned: jibran » Unassigned

Can someone please check DER+Paragraphs?

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Status: Needs review » Needs work

The JS test failed.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran at PreviousNext commented
Title: Incompatibility with multi-value fields in a multi-value Field Collection » Make the JS class unique for select element of DynamicEntityReferenceWidget
Issue summary: View changes
Status: Needs work » Needs review
FileSize
2862964-9.patch2.84 KB
screenshot-d8.dev-2017-05-29-08-55-59.png129.67 KB

The problem here is that the name is not unique. I made the name unique using $element['#name']. I tested it with unlimited cardinality der field on the node form with unlimited cardinality paragraph field with unlimited cardinality der field and unlimited cardinality field collection field with unlimited cardinality.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran at PreviousNext commented

Every field has the same name as well.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran at PreviousNext commented
Related issues: +#2698065: DER widget doesn't work with Paragraphs

jibran credited dpi.

jibran credited rikki_iki.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran at PreviousNext commented
Issue summary: View changes
FileSize
interdiff-b9477e.txt1.18 KB
Screenshot from 2017-06-08 22-24-53.png62.74 KB
make_the_js_class-2862964-12.patch2.77 KB

It is a JS class so @rikki_iki suggested to add js- prefix to the class. @dpi said $js_class = Html::cleanCssIdentifier("dynamic-entity-reference-{$name}[target_type]"); is not readable so updated that as well.
Here is the resultant classes screenshot.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer and at PreviousNext commented
Status: Needs review » Reviewed & tested by the community

  • jibran committed 9044341 on 8.x-2.x 
    Issue #2862964 by jibran, gradywring, dpi, rikki_iki: Make the JS class...

  • jibran committed 8b6cf56 on 8.x-1.x 
    Issue #2862964 by jibran, gradywring, dpi, rikki_iki: Make the JS class...
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran at PreviousNext commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed and pushed to both branches.

Status: Fixed » Closed (fixed)

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