Clearing the sorted indicator if sorting was disabled (#16563)

Change-Id: I03677e25d639264d5056c5b86b7be8277fe332a4

client/src/com/vaadin/client/ui/VScrollTable.java

@@ -3617,9 +3617,9 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                     c.setText(caption);
                 }
 
+                c.setSorted(false);
                 if (col.hasAttribute("sortable")) {
                     c.setSortable(true);
-                    c.setSorted(false);
                 } else {
                     c.setSortable(false);
                 }

uitest/src/com/vaadin/tests/components/table/DisabledSortingTable.java

@@ -0,0 +1,60 @@
+package com.vaadin.tests.components.table;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Table;
+
+public class DisabledSortingTable extends AbstractTestUI {
+
+    @SuppressWarnings("unchecked")
+    @Override
+    protected void setup(VaadinRequest request) {
+        final Table table = new Table();
+
+        table.addContainerProperty("header1", String.class, "column1");
+        table.addContainerProperty("header2", String.class, "column2");
+        table.addContainerProperty("header3", String.class, "column3");
+
+        for (int row = 0; row < 5; row++) {
+            Object key = table.addItem();
+            table.getItem(key).getItemProperty("header1")
+                    .setValue(String.valueOf(row));
+            table.getItem(key).getItemProperty("header2")
+                    .setValue(String.valueOf(5 - row));
+        }
+
+        addComponent(table);
+
+        addButton("Enable sorting", new Button.ClickListener() {
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                table.setSortEnabled(true);
+            }
+        });
+
+        addButton("Disable sorting", new Button.ClickListener() {
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                table.setSortEnabled(false);
+            }
+        });
+
+        addButton("Sort by empty array", new Button.ClickListener() {
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                table.sort(new Object[] {}, new boolean[] {});
+            }
+        });
+    }
+
+    @Override
+    public String getTestDescription() {
+        return "Sorting with empty arrays should hide sorting indicator but not reset sorting in Table with default container.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 16563;
+    }
+}

uitest/src/com/vaadin/tests/components/table/DisabledSortingTableSqlContainer.java

@@ -0,0 +1,44 @@
+package com.vaadin.tests.components.table;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.ui.Button;
+
+public class DisabledSortingTableSqlContainer extends TableSqlContainer {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        super.setup(request);
+
+        addButton("Enable sorting", new Button.ClickListener() {
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                table.setSortEnabled(true);
+            }
+        });
+
+        addButton("Disable sorting", new Button.ClickListener() {
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                table.setSortEnabled(false);
+            }
+        });
+
+        addButton("Sort by empty array", new Button.ClickListener() {
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                table.sort(new Object[] {}, new boolean[] {});
+            }
+        });
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Sorting with empty arrays should reset sorting and hide sorting indicator in Table connected to a SQLContainer";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 16563;
+    }
+
+}

uitest/src/com/vaadin/tests/components/table/DisabledSortingTableTest.java

@@ -0,0 +1,99 @@
+package com.vaadin.tests.components.table;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import org.junit.Test;
+import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.support.ui.ExpectedCondition;
+
+import com.vaadin.testbench.TestBenchElement;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.TableElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class DisabledSortingTableTest extends MultiBrowserTest {
+
+    @Override
+    protected void closeApplication() {
+        // need to close manually to use the correct ui class.
+    }
+
+    @Test
+    public void sortingByEmptyArrayShouldClearSortingIndicator() {
+        openTestURL(DisabledSortingTable.class);
+
+        assertThatFirstCellHasText("0");
+
+        sortFirstColumnAscending();
+        assertThatFirstCellHasText("4");
+
+        disableSorting();
+
+        sortByEmptyArray();
+        assertThatFirstCellHasText("4");
+
+        openTestURL(DisabledSortingTable.class, "closeApplication");
+    }
+
+    @Test
+    public void emptySortingClearsIndicatorAndResetsSortingWithSQLContainer() {
+        openTestURL(DisabledSortingTableSqlContainer.class);
+
+        assertThatFirstCellHasText("1");
+
+        sortFirstColumnAscending();
+        assertThatFirstCellHasText("2");
+
+        disableSorting();
+        sortByEmptyArray();
+
+        assertThatFirstCellHasText("1");
+
+        openTestURL(DisabledSortingTableSqlContainer.class, "closeApplication");
+    }
+
+    private void sortFirstColumnAscending() {
+        getFirstColumnHeader().click();
+        waitUntilHeaderHasExpectedClass("v-table-header-cell-asc");
+    }
+
+    private TestBenchElement getFirstColumnHeader() {
+        return getTable().getHeaderCell(1);
+    }
+
+    private TableElement getTable() {
+        return $(TableElement.class).first();
+    }
+
+    private void assertThatFirstCellHasText(String text) {
+        assertThat(getTable().getCell(0, 0).getText(), is(text));
+    }
+
+    private void sortByEmptyArray() {
+        $(ButtonElement.class).caption("Sort by empty array").first().click();
+
+        waitUntilHeaderHasExpectedClass("v-table-header-cell");
+    }
+
+    private void disableSorting() {
+        $(ButtonElement.class).caption("Disable sorting").first().click();
+    }
+
+    protected void waitUntilHeaderHasExpectedClass(final String className) {
+        final TestBenchElement header = getFirstColumnHeader();
+        waitUntil(new ExpectedCondition<Boolean>() {
+            @Override
+            public Boolean apply(WebDriver input) {
+                return className.equals(header.getAttribute("class"));
+            }
+
+            @Override
+            public String toString() {
+                // Timed out after 10 seconds waiting for ...
+                return String.format("header to get class name '%s'",
+                        className);
+            }
+        });
+    }
+}

uitest/src/com/vaadin/tests/components/table/TableSqlContainer.java

@@ -21,13 +21,15 @@ import com.vaadin.ui.VerticalLayout;
 
 public class TableSqlContainer extends AbstractTestUI {
 
+    protected Table table;
+
     @Override
     protected void setup(VaadinRequest request) {
         setLocale(Locale.ENGLISH);
         VerticalLayout layout = new VerticalLayout();
         addComponent(layout);
 
-        final Table table = new Table("Table with SQLContainer");
+        table = new Table("Table with SQLContainer");
         layout.addComponent(table);
 
         final Label selectedLabel = new Label("Selected: null");