Problem/Motivation

Views filters dragging in the grouping view seems to be a little broken. Here's a gif demonstration about the problem:

Proposed resolution

Figure out is this a template level problem or is there something wrong with the javascript needed for that functionality.

Remaining tasks

- Write test
- Write fix or wait to see if #2215857: Behaviors get attached to removed forms fixes this issue.

User interface changes

-

API changes

-

CommentFileSizeAuthor
#16 result without attachBehaviors for $form.png69.71 KBmichielnugter
#16 result with attachBehaviors for $form.png75.28 KBmichielnugter
#16 2493945-Views_filters_dragging-16.patch3.33 KBmichielnugter
#16 interdiff.txt1.15 KBmichielnugter
#12 2493945-Views_filters_dragging-12.patch986 bytescferthorney
#7 2493945-7.patch944 bytesolli
#4 2493945-do-not-test.patch749 bytesolli
views-filter-groups.gif431.86 KBlauriii
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer commented
Issue tags: +JavaScript

Preemptively adding to my queue.

droplet’s picture

droplet CreditAttribution: droplet commented

It seems #2489826: tabledrag is broken will fix above errors

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

Also this claims to fix it too... #2496501: Grouped Filters Javascript Improvements

olli’s picture

olli CreditAttribution: olli commented
FileSize
2493945-do-not-test.patch749 bytes

Not a proper fix but this seems to be related to ajax/once behavior. #2 didn't help.

droplet’s picture

droplet CreditAttribution: droplet commented

@olli is correct.

When you click add new group, old table is replaced with new table via Ajax. $(table).data() will be removed.

Downside of 2.0: #2348321: Upgrade to jQuery Once 2.x

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Issue tags: +VDC

@droplet or @olli Is there an issue to fix once behaviour?

Maybe delegation is a more proper fix?

olli’s picture

olli CreditAttribution: olli commented
Status: Active » Needs review
FileSize
2493945-7.patch944 bytes

#6: I don't know. Patch?

Here's an alternative that checks if this.$form was replaced by ajax.

olli’s picture

olli CreditAttribution: olli commented

Almost identical patch in #2215857: Behaviors get attached to removed forms.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra as a volunteer commented

Ran into this problem and found that in Firefox fails but Chrome works OK when adding filter groups and sort the stuff around.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Issue tags: +Needs tests

I'm wondering whether now that we have javascript tests we could actually leverage it.

cferthorney’s picture

cferthorney
he/them
Primary language English
Location Huntingdon, UK
CreditAttribution: cferthorney commented
FileSize
2493945-Views_filters_dragging-12.patch986 bytes

I've rerolled #7 this against the latest 8.2.x-dev . Please let me know if this actually needs to go against 8.3.x now. No tests added, as I'm uncertain how to test this.
*edited to correct grammar*

droplet’s picture

droplet CreditAttribution: droplet commented
Status: Needs review » Active

this isn't a fix @see #4. :)

cferthorney’s picture

cferthorney
he/them
Primary language English
Location Huntingdon, UK
CreditAttribution: cferthorney commented

My bad. Rerolled against #2215857 instead.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Issue summary: View changes
FileSize
interdiff.txt1.15 KB
2493945-Views_filters_dragging-16.patch3.33 KB
result with attachBehaviors for $form.png75.28 KB
result without attachBehaviors for $form.png69.71 KB

I extended the test in #2717629: Add filter group for a View fails to add test coverage for this.

I ran the test without any alterations which causes the attachBehaviors to run for the form. This results in an incorrect situation, see the screenshot form the test.

When simply commenting ajax.js line 892:

Drupal.attachBehaviors(this.$form.get(0), settings);

Everything works correctly, which is testable in the UI and will make this test pass (see the different result in the screenshot).

The problem is that the behaviors are attached twice, once for the insert (replacing the form) and once for the form reference that is kept for the beforeSerialize() logic. The mentioned issue #2215857: Behaviors get attached to removed forms really seems relevant here and I think if that issue is fixed, this issue will also be fixed.

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Status: Active » Needs review
michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Status: Needs review » Needs work

Trigger test to run to make sure it fails op drupal.org as well.

Note: the test needs refinement before committing it.

The last submitted patch, 4: 2493945-do-not-test.patch, failed testing.

The last submitted patch, 16: 2493945-Views_filters_dragging-16.patch, failed testing.

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Status: Needs work » Closed (duplicate)
Related issues: +#2215857: Behaviors get attached to removed forms

Marking this a duplicate of #2215857: Behaviors get attached to removed forms as that fix fixes this issue as well.

hkirsman’s picture

hkirsman CreditAttribution: hkirsman commented

#2215857 Seems to fix this indeed but you need to rebuild cache.