Problem/Motivation

Drupal.views.ajaxView is not initializing pagers in nested views. The part here:

// Add the ajax to pagers.
this.$view
  // Don't attach to nested views. Doing so would attach multiple behaviors
  // to a given element.
  .filter($.proxy(this.filterNestedViews, this))
  .once('ajax-pager').each($.proxy(this.attachPagerAjax, this));

checks if view is inside another view. As a result pager links in embedded views are initialized in the context of parent. This makes it impossible to perform ajax operations in context of the child view (see this Views Infinite Scroll issue).

Proposed resolution

Initialize ajax views in order determined by their nesting level. The innermost view should go first and mark its elements with .once so they aren't processed again when parent view is initialized.

Remaining tasks

  1. Write patch
  2. Review
  3. Write tests

User interface changes

None

API changes

None

Data model changes

None

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

blazey created an issue. See original summary.

blazey’s picture

Attached patch changes the order in which ajax views are initialized.

blazey’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 2858890-2.patch, failed testing.

blazey’s picture

Assigned: blazey » Unassigned
Issue summary: View changes
Status: Needs work » Needs review
FileSize
3.58 KB
1.18 KB
Pavan B S’s picture

+++ b/core/modules/views/js/ajax_view.js
@@ -38,6 +35,35 @@
+    // Sort views by nesting level descending. The goal is to start with the innermost.

Line exceeding more than 80 characters
Applying the patch, please review.

stewest’s picture

I can confirm that https://www.drupal.org/files/issues/2858890-2.patch does work for us! Nice work Blazey

stewest’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

We can now write tests that leverage javascript. Yay! So we should add a test here to ensure we don't regress and have properly fixed this. See https://www.drupal.org/docs/8/phpunit/phpunit-javascript-testing-tutorial

blazey’s picture

Status: Needs work » Needs review
FileSize
10.43 KB
14.01 KB

Thanks for review @Pavan B S @stewest @alexpott. Attaching a test patch that:

  • Adds a new test view test_view_area_ajax. It's just a simple content view with the test_content_ajax view embedded in the header.
  • Adds a new module views_text_ajax_subscriber that can do assertions on responses to ajax requests.
  • Adds a new EmbeddedViewPaginationAJAXTest javascript test that visits test_view_area_ajax, simulates a click on a pager link in the embedded view and tells views_text_ajax_subscriber module to assert that the inner view, not the enclosing one, is associated with the request.

The last submitted patch, 10: 2858890-10-test-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 10: 2858890-10.patch, failed testing.

blazey’s picture

Added @group annotation.

The last submitted patch, 13: 2858890-13-test-only.patch, failed testing.

blazey’s picture

FileSize
10.5 KB
blazey’s picture

Status: Needs review » Needs work

The last submitted patch, 16: 2858890-15-test-only.patch, failed testing.

blazey’s picture

Status: Needs work » Needs review
blazey’s picture

Issue tags: -Needs tests +JavaScript

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

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