It's broken for non-AJAX callbacks as well, so that will be testable.
Only local images are allowed.Only local images are allowed.Only local images are allowed.

Files: 
CommentFileSizeAuthor
#12 1904932-12.patch2.67 KBYesCT
PASSED: [[SimpleTest]]: [MySQL] 55,570 pass(es).
[ View ]
#12 interdiff-11-12.txt960 bytesYesCT
#11 1904932-11.patch2.68 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,520 pass(es).
[ View ]
#11 interdiff-1904932-11.txt3.23 KBdamiankloip
#8 1904932-8.patch3.77 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 53,864 pass(es).
[ View ]
#8 interdiff.txt1.89 KBdamiankloip
#2 drupal-1904932-2.patch3.68 KBYesCT
FAILED: [[SimpleTest]]: [MySQL] 48,716 pass(es), 2 fail(s), and 3 exception(s).
[ View ]
#2 interdiff-1-2.txt625 bytesYesCT
#1 drupal-1904932-1.patch3.69 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 48,725 pass(es).
[ View ]
rearrange3.png12.69 KBtim.plunkett
rearrange2.png45.05 KBtim.plunkett
rearrange1.png25.16 KBtim.plunkett

Comments

Status:Active» Needs review
StatusFileSize
new3.69 KB
PASSED: [[SimpleTest]]: [MySQL] 48,725 pass(es).
[ View ]

Status:Needs review» Needs work
StatusFileSize
new625 bytes
new3.68 KB
FAILED: [[SimpleTest]]: [MySQL] 48,716 pass(es), 2 fail(s), and 3 exception(s).
[ View ]

manually tested. And the js error is gone, but it jumps me out of the views overlay and wizard and to the front page.

tried to improve grammar of the comment. See if it still says the correct meaning of what you wanted it to say.

Status:Needs work» Needs review

Status:Needs review» Needs work

manually tested. And the js error is gone, but it jumps me out of the views overlay and wizard and to the front page.

So we probably want to set this link to use an empty fragment, so it doesn't do anything for itself, without the javascript?

@dawehner How do we do that?

Also, this is "needs tests"
what kind of test should we add? webtest?

Well, the removing just works on the javascript so I don't see how we should be able to test this?
The error mentioned by tim happens if you click on the link but the javascript fails.

Status:Needs work» Needs review
Issue tags:-Needs tests
StatusFileSize
new1.89 KB
new3.77 KB
PASSED: [[SimpleTest]]: [MySQL] 53,864 pass(es).
[ View ]

Gave this a quick tidy up, now it matches alot of the other js code (I think :)). Also tested, and confirm the fix is good.

Removing needs tests too; Javascript is rubbish, and hard to test ;)

+++ b/core/modules/views/views_ui/js/views-admin.jsundefined
@@ -1098,6 +1098,34 @@ Drupal.viewsUi.resizeModal = function (e, no_shrink) {
+  /**
+   * Handle handler deletion by looking for the hidden checkbox and hiding the
+   * row.
+   */
...
+  /**
+   * Handle display deletion by looking for the hidden checkbox and hiding the
+   * row.
+   */

These aren't method documentations so should we use "//" instead?

Status:Needs review» Needs work

+++ b/core/modules/views/views_ui/js/views-admin.js
@@ -1098,6 +1098,34 @@ Drupal.viewsUi.resizeModal = function (e, no_shrink) {
+Drupal.behaviors.handlerRemoveLink = {};

.handlerRemoveLink -> .viewsUiHandlerRemoveLink

+++ b/core/modules/views/views_ui/js/views-admin.js
@@ -1098,6 +1098,34 @@ Drupal.viewsUi.resizeModal = function (e, no_shrink) {
+    $('#views-row-' + id).hide();
+    $('#views-removed-' + id).attr('checked', true);

These could use the context param too.

+++ b/core/modules/views/views_ui/js/views-admin.js
@@ -1098,6 +1098,34 @@ Drupal.viewsUi.resizeModal = function (e, no_shrink) {
+  $('a.display-remove-link', context).addClass('display-processed').click(function(event) {

Why not .once() instead of .addClass()?

+++ b/core/modules/views/views_ui/js/views-admin.js
@@ -1098,6 +1098,34 @@ Drupal.viewsUi.resizeModal = function (e, no_shrink) {
+      var id = $(this).attr('id').replace('display-remove-link-', '');
+      $('#display-row-' + id, context).hide();
+      $('#display-removed-' + id, context).attr('checked', true);
+      event.preventDefault();
+  });

Indentation.

Status:Needs work» Needs review
StatusFileSize
new3.23 KB
new2.68 KB
PASSED: [[SimpleTest]]: [MySQL] 55,520 pass(es).
[ View ]

Rerolled and added in feedback from #9 and #10, thanks. Its been a while!

StatusFileSize
new960 bytes
new2.67 KB
PASSED: [[SimpleTest]]: [MySQL] 55,570 pass(es).
[ View ]

+++ b/core/modules/views_ui/js/views-admin.jsundefined
@@ -1098,6 +1098,32 @@ Drupal.viewsUi.resizeModal = function (e, no_shrink) {
+Drupal.behaviors.viewsUIhandlerRemoveLink = {};
+Drupal.behaviors.viewsUIhandlerRemoveLink.attach = function(context) {

In this js file, UIwhatever is more using the pattern: UiWhatever.

+++ b/core/modules/views_ui/js/views-admin.jsundefined
@@ -1098,6 +1098,32 @@ Drupal.viewsUi.resizeModal = function (e, no_shrink) {
+  /**
+   * Handle handler deletion by looking for the hidden checkbox and hiding the
+   * row.
+   */
...
+  // Handle display deletion by looking for the hidden checkbox and hiding the
+  // row.

missed one. :)

verified that the bug still exists without the patch.

manually tested with the patch:
Works great, stays in views ui, and I can save. Everything fine.

Next steps:
a code review (I should not rtbc as I did the most recent patch).

======

Actually, with and without this patch, rearranging the sort criteria or the no results behavior... results in all of those being deleted instead of rearranged. Rearranging the filter criteria works fine though. I dont see a issue for rearranging sort criteria bug (yet).

Should not stop this issue as it is a pre-existing bug.

Status:Needs review» Reviewed & tested by the community

I talked to @damiankloip in irc.
rtbc!

RTBC +1

Status:Reviewed & tested by the community» Fixed

Committed f140bce and pushed to 8.x. Thanks!

I guess we need a new issue for the bug mentioned by @YesCT in #13

I think #1929070: Refactor views_ui_rearrange_filter_form to remove theme function for table rendering is related to that, and will possibly/probably/maybe fix what is mentioned by YesCT.

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