TableView: 3-phase ToggleSortOrder (sorted/reversed/unsorted)

ui/views/controls/table/table_view_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/controls/table/table_view.h"
 
 #include <stddef.h>
 
 #include "base/macros.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 143 matching lines @@
 std::string GetModelToViewAsString(TableView* table) {
   std::string result;
   for (int i = 0; i < table->RowCount(); ++i) {
     if (i != 0)
       result += " ";
     result += base::IntToString(table->ModelToView(i));
   }
   return result;
 }
 
+std::string GetRowsInViewOrderAsString(TableView* table) {

[sky, 2016/07/18 16:18:58]

add a description of the format of the returned string.

[ncarter (slow), 2016/07/25 20:04:02]

Done.

+  std::string result;
+
+  std::string separator = "";

[sky, 2016/07/18 16:18:58]

nit: no = "", just std::string separator;

[ncarter (slow), 2016/07/25 20:04:02]

Done (switched to index checking).

+  for (int row = 0; row < table->RowCount(); ++row) {
+    result += separator;
+    separator = "[";
+    for (auto column : table->visible_columns()) {
+      result += separator;
+      result += base::UTF16ToUTF8(
+          table->model()->GetText(table->ViewToModel(row), column.column.id));
+      separator = ", ";

[sky, 2016/07/18 16:18:58]

nit: reusing separator for the inner loop makes this code mildly confusing.

[ncarter (slow), 2016/07/25 20:04:01]

You're right, this was confusing. I switched to using index-based iteration, and
special-casing iteration zero.

+    }
+    result += "]";
+    separator = ", ";
+  }
+  return result;
+}
+
 class TestTableView : public TableView {
  public:
   TestTableView(ui::TableModel* model,
                 const std::vector<ui::TableColumn>& columns)
       : TableView(model, columns, TEXT_ONLY, false) {
   }
 
   // View overrides:
   bool HasFocus() const override {
     // Overriden so key processing works.
@@ skipping 154 matching lines @@
       ui::GestureEventDetails(ui::ET_GESTURE_SCROLL_UPDATE));
   helper_->header()->OnGestureEvent(&scroll_update);
 
   // This should shrink the first column and pull the second column in.
   EXPECT_EQ(x - 1, table_->visible_columns()[0].width);
   EXPECT_EQ(x - 1, table_->visible_columns()[1].x);
 }
 
 // Assertions for table sorting.
 TEST_F(TableViewTest, Sort) {
+  // Initial ordering.
+  ASSERT_EQ(0u, table_->sort_descriptors().size());

[sky, 2016/07/18 16:18:58]

nit: use empty

[ncarter (slow), 2016/07/25 20:04:02]

Done.

+  EXPECT_EQ("0 1 2 3", GetViewToModelAsString(table_));
+  EXPECT_EQ("0 1 2 3", GetModelToViewAsString(table_));
+  EXPECT_EQ("[0, 1], [1, 1], [2, 2], [3, 0]",
+            GetRowsInViewOrderAsString(table_));
+
   // Toggle the sort order of the first column, shouldn't change anything.
   table_->ToggleSortOrder(0);
   ASSERT_EQ(1u, table_->sort_descriptors().size());
   EXPECT_EQ(0, table_->sort_descriptors()[0].column_id);
   EXPECT_TRUE(table_->sort_descriptors()[0].ascending);
   EXPECT_EQ("0 1 2 3", GetViewToModelAsString(table_));
   EXPECT_EQ("0 1 2 3", GetModelToViewAsString(table_));
+  EXPECT_EQ("[0, 1], [1, 1], [2, 2], [3, 0]",
+            GetRowsInViewOrderAsString(table_));
 
-  // Invert the sort (first column descending).
+  // Toggle the sort (first column descending).
   table_->ToggleSortOrder(0);
   ASSERT_EQ(1u, table_->sort_descriptors().size());
   EXPECT_EQ(0, table_->sort_descriptors()[0].column_id);
   EXPECT_FALSE(table_->sort_descriptors()[0].ascending);
   EXPECT_EQ("3 2 1 0", GetViewToModelAsString(table_));
   EXPECT_EQ("3 2 1 0", GetModelToViewAsString(table_));
+  EXPECT_EQ("[3, 0], [2, 2], [1, 1], [0, 1]",
+            GetRowsInViewOrderAsString(table_));
 
-  // Change cell 0x3 to -1, meaning we have 0, 1, 2, -1 (in the first column).
+  // Change the [3, 0] cell to [-1, 0]. This should move it to the back of
+  // the current sort order.
   model_->ChangeRow(3, -1, 0);
   ASSERT_EQ(1u, table_->sort_descriptors().size());
   EXPECT_EQ(0, table_->sort_descriptors()[0].column_id);
   EXPECT_FALSE(table_->sort_descriptors()[0].ascending);
   EXPECT_EQ("2 1 0 3", GetViewToModelAsString(table_));
   EXPECT_EQ("2 1 0 3", GetModelToViewAsString(table_));
+  EXPECT_EQ("[2, 2], [1, 1], [0, 1], [-1, 0]",
+            GetRowsInViewOrderAsString(table_));
 
-  // Invert sort again (first column ascending).
+  // Toggle the sort again, to clear the sort and restore the model ordering.
+  table_->ToggleSortOrder(0);
+  ASSERT_EQ(0u, table_->sort_descriptors().size());
+  EXPECT_EQ("0 1 2 3", GetViewToModelAsString(table_));
+  EXPECT_EQ("0 1 2 3", GetModelToViewAsString(table_));
+  EXPECT_EQ("[0, 1], [1, 1], [2, 2], [-1, 0]",
+            GetRowsInViewOrderAsString(table_));
+
+  // Toggle the sort again (first column ascending).
   table_->ToggleSortOrder(0);
   ASSERT_EQ(1u, table_->sort_descriptors().size());
   EXPECT_EQ(0, table_->sort_descriptors()[0].column_id);
   EXPECT_TRUE(table_->sort_descriptors()[0].ascending);
   EXPECT_EQ("3 0 1 2", GetViewToModelAsString(table_));
   EXPECT_EQ("1 2 3 0", GetModelToViewAsString(table_));
+  EXPECT_EQ("[-1, 0], [0, 1], [1, 1], [2, 2]",
+            GetRowsInViewOrderAsString(table_));
 
-  // Add a row so that model has 0, 3, 1, 2, -1.
+  // Add a row that's second in the model order, but last in the active sort
+  // order.
   model_->AddRow(1, 3, 4);
   ASSERT_EQ(1u, table_->sort_descriptors().size());
   EXPECT_EQ(0, table_->sort_descriptors()[0].column_id);
   EXPECT_TRUE(table_->sort_descriptors()[0].ascending);
   EXPECT_EQ("4 0 2 3 1", GetViewToModelAsString(table_));
   EXPECT_EQ("1 4 2 3 0", GetModelToViewAsString(table_));
+  EXPECT_EQ("[-1, 0], [0, 1], [1, 1], [2, 2], [3, 4]",
+            GetRowsInViewOrderAsString(table_));
 
-  // Delete the first row, ending up with 3, 1, 2, -1.
-  model_->RemoveRow(0);
+  // Add a row that's last in the model order but second in the the active sort
+  // order.
+  model_->AddRow(5, -1, 20);
   ASSERT_EQ(1u, table_->sort_descriptors().size());
   EXPECT_EQ(0, table_->sort_descriptors()[0].column_id);
   EXPECT_TRUE(table_->sort_descriptors()[0].ascending);
-  EXPECT_EQ("3 1 2 0", GetViewToModelAsString(table_));
-  EXPECT_EQ("3 1 2 0", GetModelToViewAsString(table_));
+  EXPECT_EQ("4 5 0 2 3 1", GetViewToModelAsString(table_));
+  EXPECT_EQ("2 5 3 4 0 1", GetModelToViewAsString(table_));
+  EXPECT_EQ("[-1, 0], [-1, 20], [0, 1], [1, 1], [2, 2], [3, 4]",
+            GetRowsInViewOrderAsString(table_));
+
+  // Click the first column again, then click the second column. This should
+  // yield an ordering of second column ascending, with the first column
+  // descending as a tiebreaker.
+  table_->ToggleSortOrder(0);
+  table_->ToggleSortOrder(1);
+  ASSERT_EQ(2u, table_->sort_descriptors().size());
+  EXPECT_EQ(1, table_->sort_descriptors()[0].column_id);
+  EXPECT_TRUE(table_->sort_descriptors()[0].ascending);
+  EXPECT_EQ(0, table_->sort_descriptors()[1].column_id);
+  EXPECT_FALSE(table_->sort_descriptors()[1].ascending);
+  EXPECT_EQ("4 2 0 3 1 5", GetViewToModelAsString(table_));
+  EXPECT_EQ("2 4 1 3 0 5", GetModelToViewAsString(table_));
+  EXPECT_EQ("[-1, 0], [1, 1], [0, 1], [2, 2], [3, 4], [-1, 20]",
+            GetRowsInViewOrderAsString(table_));
+
+  // Toggle the current column to change from ascending to descending. This
+  // should result in an almost-reversal of the previous order, except for the
+  // two rows with the same value for the second column.
+  table_->ToggleSortOrder(1);
+  ASSERT_EQ(2u, table_->sort_descriptors().size());
+  EXPECT_EQ(1, table_->sort_descriptors()[0].column_id);
+  EXPECT_FALSE(table_->sort_descriptors()[0].ascending);
+  EXPECT_EQ(0, table_->sort_descriptors()[1].column_id);
+  EXPECT_FALSE(table_->sort_descriptors()[1].ascending);
+  EXPECT_EQ("5 1 3 2 0 4", GetViewToModelAsString(table_));
+  EXPECT_EQ("4 1 3 2 5 0", GetModelToViewAsString(table_));
+  EXPECT_EQ("[-1, 20], [3, 4], [2, 2], [1, 1], [0, 1], [-1, 0]",
+            GetRowsInViewOrderAsString(table_));
+
+  // Delete the [0, 1] row from the model. It's at model index zero.
+  model_->RemoveRow(0);
+  ASSERT_EQ(2u, table_->sort_descriptors().size());
+  EXPECT_EQ(1, table_->sort_descriptors()[0].column_id);
+  EXPECT_FALSE(table_->sort_descriptors()[0].ascending);
+  EXPECT_EQ(0, table_->sort_descriptors()[1].column_id);
+  EXPECT_FALSE(table_->sort_descriptors()[1].ascending);
+  EXPECT_EQ("4 0 2 1 3", GetViewToModelAsString(table_));
+  EXPECT_EQ("1 3 2 4 0", GetModelToViewAsString(table_));
+  EXPECT_EQ("[-1, 20], [3, 4], [2, 2], [1, 1], [-1, 0]",
+            GetRowsInViewOrderAsString(table_));
+
+  // Toggle the current sort column again. This should clear both the primary
+  // and secondary sort descriptor.
+  table_->ToggleSortOrder(1);
+  ASSERT_EQ(0u, table_->sort_descriptors().size());
+  EXPECT_EQ("0 1 2 3 4", GetViewToModelAsString(table_));
+  EXPECT_EQ("0 1 2 3 4", GetModelToViewAsString(table_));
+  EXPECT_EQ("[3, 4], [1, 1], [2, 2], [-1, 0], [-1, 20]",
+            GetRowsInViewOrderAsString(table_));
 }
 
 // Verfies clicking on the header sorts.
 TEST_F(TableViewTest, SortOnMouse) {
   EXPECT_TRUE(table_->sort_descriptors().empty());
 
   const int x = table_->visible_columns()[0].width / 2;
   EXPECT_NE(0, x);
   // Press and release the mouse.
   const ui::MouseEvent pressed(ui::ET_MOUSE_PRESSED, gfx::Point(x, 0),
@@ skipping 99 matching lines @@
   //   1
   // B -1
   //   -1
   model_->ChangeRow(2, -1, 0);
   ASSERT_EQ(1u, table_->sort_descriptors().size());
   EXPECT_EQ(0, table_->sort_descriptors()[0].column_id);
   EXPECT_FALSE(table_->sort_descriptors()[0].ascending);
   EXPECT_EQ("0 1 2 3", GetViewToModelAsString(table_));
   EXPECT_EQ("0 1 2 3", GetModelToViewAsString(table_));
 
-  // Toggle to ascending sort.
+  // Toggle to clear the sort.
+  table_->ToggleSortOrder(0);
+  ASSERT_EQ(0u, table_->sort_descriptors().size());
+  EXPECT_EQ("0 1 2 3", GetViewToModelAsString(table_));
+  EXPECT_EQ("0 1 2 3", GetModelToViewAsString(table_));
+
+  // Toggle again to effect an ascending sort.
   table_->ToggleSortOrder(0);
   ASSERT_EQ(1u, table_->sort_descriptors().size());
   EXPECT_EQ(0, table_->sort_descriptors()[0].column_id);
   EXPECT_TRUE(table_->sort_descriptors()[0].ascending);
   EXPECT_EQ("2 3 0 1", GetViewToModelAsString(table_));
   EXPECT_EQ("2 3 0 1", GetModelToViewAsString(table_));
 }
 
 namespace {
 
@@ skipping 393 matching lines @@
   new_selection.AddIndexToSelection(0);
   new_selection.AddIndexToSelection(1);
   new_selection.set_active(0);
   new_selection.set_anchor(0);
   helper_->SetSelectionModel(new_selection);
   model_->RemoveRow(0);
   helper_->OnFocus();
 }
 
 }  // namespace views