Operator labels are not redrawn on filter removal

I have added a patch that fixes this issue. Although the redrawing function was run and filtering out hidden table rows, it was running before the table row was removed. Re-adjusting the order of these functions resolves this issue.

See if this test passes

I found the pre-existing test coverage I made use of. I also realized this bug appears very early - right after clicking on 'Remove' at the last filter (look at the attached screencast) so there was no need to test rearranging filters with drag-and-drop. The #4 patch works for me so I extended it with the test only.

The interdiff for the record.

The patch #6 is working fine.The operator will be gone after removing last item.Including screen recording below after applying the patch:

RTBC

Tested #4 on 9.1. Its working fine
RTBC +1

Nice work on this.

+++ b/core/modules/views_ui/tests/src/FunctionalJavascript/FilterCriteriaTest.php
@@ -125,6 +125,35 @@ class FilterCriteriaTest extends WebDriverTestBase {
+    // The current last filter shouldn't have the operator label.
+    $operator_label = $penultimate_row->find('css', '.views-operator-label');
+    $this->assertNull($operator_label, 'The last filter has no operator label.');

Can we add an assertion that the label is present before we remove the last row, that way we are actually testing for a change. Currently the test would also be green if the label was never there to begin with.

You are right, Lendude. Here is the new test and the patch.

The test is green again.

The patch #12 is working fine on drupal 9.2.dev
before patch
after patch . RTBC

Re-rolled for 9.2.x-dev

@ranjith_kumar_k_u why reroll? It was applying fine to 9.2.x ¯\_(ツ)_/¯

Tests are looking great now. Bit of nitpicking:

1. +++ b/core/modules/views_ui/tests/src/FunctionalJavascript/FilterCriteriaTest.php
   @@ -125,6 +125,56 @@ class FilterCriteriaTest extends WebDriverTestBase {
   +   * Issue: https://www.drupal.org/project/drupal/issues/3161207
   

   This isn't needed. People can just git blame

2. +++ b/core/modules/views_ui/tests/src/FunctionalJavascript/FilterCriteriaTest.php
   @@ -125,6 +125,56 @@ class FilterCriteriaTest extends WebDriverTestBase {
   +    $assert_session = $this->assertSession();
   

   This isn't used

3. +++ b/core/modules/views_ui/tests/src/FunctionalJavascript/FilterCriteriaTest.php
   @@ -125,6 +125,56 @@ class FilterCriteriaTest extends WebDriverTestBase {
   +    $last_row = $row[count($row) - 1];
   +    $penultimate_row = $row[count($row) - 2];
   

   Maybe just using array_pop() twice would make this a little easier to read? Dunno, this works to, but not that easy to read.

Addressed #19.

Let's try this one.

Here is an interdiff I forgot to attach.

I'm changing the status to "Needs review" because a "test-only" patch failed and it changed the status to "Needs work". A complete patch #22 is green and ready to review. The issue meets the requirements of Bug Smash Initiative so I added a relevant tag as well.

I reviewed patch #22 and it looks to me. It addresses what @Lendude pointed on comment #19.
Tests looks good and well documented.

I attached a gif to confirm that I tested the functionality and that it is working.

@kndr next time you can attach the test patch first and then the complete patch so issue status will not change to "needs work" (just a small tip ;) ).