Merge lp:~brandontschaefer/unity/lp.1131646-fix into lp:unity

[Brandon Schaefer]

When in detail mode, you are now allowed to use UP/DOWN keys to move around.

When in detail mode, you are now allowed to use UP/DOWN keys to move around.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3368
http://jenkins.qa.ubuntu.com/job/unity-ci/134/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/22
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/22
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/22

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/134/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

Nice work, overall it looks fine, just few things:

143 - if (detail_selection_index >= (unsigned int) 1)
232 + return (detail_selection_index > 0);

doesn't compile on raring g++... Probably we should wait a little.

478 + std::vector<LayoutWindow::Vector> rows = GetRows(windows, max_bounds);
235 +void SwitcherModel::SetRowSizes(std::vector<int> row_sizes)

Use const& please.

174 + for (unsigned int i = 0; i <= n; i++)

++i is nicer. Also you should make sure that n is < row_sizes_.size().

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3372
http://jenkins.qa.ubuntu.com/job/unity-ci/140/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/28
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/28
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/28

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/140/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

+bool Controller::HandleStartInitiateEvent()
+bool Controller::HandleStopInitiateEvent()

What about using different naming? Such {Start,Stop}DetailMode or better?

Probably it would be better to change the compiz option names also, since they generates functions that doesn't represent anymore what they say. Don't you agree?

Would be now possible to test the controller also?

133 + void NextDetailRow();
134 + void PrevDetailRow();
135 + bool HasNextDetailRow() const;
136 + bool HasPrevDetailRow() const;

Are probably not needed anymore, right (also in the impl)?

[Brandon Schaefer (brandontschaefer)]

Yeeah, but it still starts/stops detail mode. Ill try to think of a different name :). Yes, I don't have to expose those function publicily anymore, and yes it will be possible to get tests on that 2 new function. What I was going to do this morning :)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3373
http://jenkins.qa.ubuntu.com/job/unity-ci/142/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/30
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/30
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/30

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/142/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

Great, I think we're fine for trunk now! :)

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3374
http://jenkins.qa.ubuntu.com/job/unity-ci/143/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/31
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/31
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/31

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/143/rebuild

review: Approve (continuous-integration)

=== modified file 'launcher/SwitcherController.cpp'
--- launcher/SwitcherController.cpp	2013-06-14 16:10:26 +0000
+++ launcher/SwitcherController.cpp	2013-06-18 19:07:26 +0000
@@ -124,6 +124,46 @@
   return visible_;
 }
 
+bool Controller::StartDetailMode()
+{
+  if (visible_)
+  {
+    if (IsDetailViewShown() &&
+        impl_->HasNextDetailRow())
+    {
+      impl_->NextDetailRow();
+    }
+    else
+    {
+      SetDetail(true);
+    }
+
+    return true;
+  }
+
+  return false;
+}
+
+bool Controller::StopDetailMode()
+{
+  if (visible_)
+  {
+    if (IsDetailViewShown() &&
+        impl_->HasPrevDetailRow())
+    {
+      impl_->PrevDetailRow();
+    }
+    else
+    {
+      SetDetail(false);
+    }
+
+    return true;
+  }
+
+  return false;
+}
+
 void Controller::Next()
 {
   impl_->Next();
@@ -586,6 +626,38 @@
   model_->PrevDetail();
 }
 
+void Controller::Impl::NextDetailRow()
+{
+  if (!model_)
+    return;
+
+  model_->NextDetailRow();
+}
+
+void Controller::Impl::PrevDetailRow()
+{
+  if (!model_)
+    return;
+
+  model_->PrevDetailRow();
+}
+
+bool Controller::Impl::HasNextDetailRow() const
+{
+  if (!model_)
+    return false;
+
+  return model_->HasNextDetailRow();
+}
+
+bool Controller::Impl::HasPrevDetailRow() const
+{
+  if (!model_)
+    return false;
+
+  return model_->HasPrevDetailRow();
+}
+
 LayoutWindow::Vector Controller::Impl::ExternalRenderTargets()
 {
   if (!view_)
 
=== modified file 'launcher/SwitcherController.h'
--- launcher/SwitcherController.h	2013-02-26 16:27:23 +0000
+++ launcher/SwitcherController.h	2013-06-18 19:07:26 +0000
@@ -89,6 +89,9 @@
 
   bool Visible();
 
+  bool StartDetailMode();
+  bool StopDetailMode();
+
   void Next();
   void Prev();
 
 
=== modified file 'launcher/SwitcherControllerImpl.h'
--- launcher/SwitcherControllerImpl.h	2013-02-26 16:27:23 +0000
+++ launcher/SwitcherControllerImpl.h	2013-06-18 19:07:26 +0000
@@ -56,6 +56,11 @@
   void NextDetail();
   void PrevDetail();
 
+  void NextDetailRow();
+  void PrevDetailRow();
+  bool HasNextDetailRow() const;
+  bool HasPrevDetailRow() const;
+
   bool IsDetailViewShown();
   void SetDetail(bool detail, unsigned int min_windows = 1);
 
 
=== modified file 'launcher/SwitcherModel.cpp'
--- launcher/SwitcherModel.cpp	2013-02-14 16:26:13 +0000
+++ launcher/SwitcherModel.cpp	2013-06-18 19:07:26 +0000
@@ -38,6 +38,7 @@
   , applications_(icons)
   , index_(0)
   , last_index_(0)
+  , row_index_(0)
 {
   // When using Webapps, there are more than one active icon, so let's just pick
   // up the first one found which is the web browser.
@@ -183,6 +184,7 @@
 
   detail_selection = false;
   detail_selection_index = 0;
+  row_index_ = 0;
   selection_changed.emit(Selection());
 }
 
@@ -197,6 +199,7 @@
 
   detail_selection = false;
   detail_selection_index = 0;
+  row_index_ = 0;
   selection_changed.emit(Selection());
 }
 
@@ -209,6 +212,8 @@
     detail_selection_index = detail_selection_index + 1;
   else
     detail_selection_index = 0;
+
+  UpdateRowIndex();
 }
 
 void SwitcherModel::PrevDetail()
@@ -220,6 +225,98 @@
     detail_selection_index = detail_selection_index - 1;
   else
     detail_selection_index = DetailXids().size() - 1;
+
+  UpdateRowIndex();
+}
+
+void SwitcherModel::UpdateRowIndex()
+{
+  int current_index = detail_selection_index;
+  unsigned int current_row = 0;
+
+  for (auto r : row_sizes_)
+  {
+    current_index -= r;
+
+    if (current_index < 0)
+    {
+      row_index_ = current_row;
+      return;
+    }
+
+    current_row++;
+  }
+}
+
+unsigned int SwitcherModel::SumNRows(unsigned int n) const
+{
+  unsigned int total = 0;
+
+  if (n < row_sizes_.size())
+    for (unsigned int i = 0; i <= n; ++i)
+      total += row_sizes_[i];
+
+  return total;
+}
+
+bool SwitcherModel::DetailIndexInLeftHalfOfRow() const
+{
+  unsigned int half = row_sizes_[row_index_]/2;
+  unsigned int total_above = (row_index_ > 0 ? SumNRows(row_index_ - 1) : 0);
+  unsigned int diff = detail_selection_index - total_above;
+
+  return (diff < half);
+}
+
+void SwitcherModel::NextDetailRow()
+{
+  if (HasNextDetailRow())
+  {
+    unsigned int current_row = row_sizes_[row_index_];
+    unsigned int next_row    = row_sizes_[row_index_ + 1];
+    unsigned int increment   = current_row;
+
+    if (!DetailIndexInLeftHalfOfRow())
+      increment = next_row;
+
+    detail_selection_index = detail_selection_index + increment;
+    row_index_++;
+  }
+}
+
+void SwitcherModel::PrevDetailRow()
+{
+  if (row_index_ > 0)
+  {
+    unsigned int current_row = row_sizes_[row_index_];
+    unsigned int prev_row    = row_sizes_[row_index_ - 1];
+    unsigned int decrement   = current_row;
+
+    if (DetailIndexInLeftHalfOfRow())
+      decrement = prev_row;
+
+    detail_selection_index = detail_selection_index - decrement;
+    row_index_--;
+  }
+  else
+  {
+    detail_selection_index = detail_selection_index - 1;
+  }
+}
+
+bool SwitcherModel::HasNextDetailRow() const
+{
+  return (row_sizes_.size() && row_index_ < row_sizes_.size() - 1);
+}
+
+bool SwitcherModel::HasPrevDetailRow() const
+{
+  return (detail_selection_index > (unsigned int) 0);
+}
+
+void SwitcherModel::SetRowSizes(std::vector<int> const& row_sizes)
+{
+  row_sizes_ = row_sizes;
 }
 
 void SwitcherModel::Select(AbstractLauncherIcon::Ptr const& selection)
 
=== modified file 'launcher/SwitcherModel.h'
--- launcher/SwitcherModel.h	2013-02-05 01:38:11 +0000
+++ launcher/SwitcherModel.h	2013-06-18 19:07:26 +0000
@@ -90,6 +90,13 @@
   void NextDetail();
   void PrevDetail();
 
+  void NextDetailRow();
+  void PrevDetailRow();
+  bool HasNextDetailRow() const;
+  bool HasPrevDetailRow() const;
+
+  void SetRowSizes(std::vector<int> const& row_sizes);
+
   void Select(launcher::AbstractLauncherIcon::Ptr const& selection);
   void Select(unsigned int index);
 
@@ -101,10 +108,16 @@
   void AddProperties(GVariantBuilder* builder);
 
 private:
+  void UpdateRowIndex();
+  unsigned int SumNRows(unsigned int n) const;
+  bool DetailIndexInLeftHalfOfRow() const;
+
   Applications                        applications_;
   unsigned int                        index_;
   unsigned int                        last_index_;
+  unsigned int                        row_index_;
   launcher::AbstractLauncherIcon::Ptr last_active_application_;
+  std::vector<int>                    row_sizes_;
 };
 
 }
 
=== modified file 'launcher/SwitcherView.cpp'
--- launcher/SwitcherView.cpp	2013-04-12 22:48:37 +0000
+++ launcher/SwitcherView.cpp	2013-06-18 19:07:26 +0000
@@ -277,6 +277,7 @@
 
   nux::Geometry layout_geo;
   layout_system_.LayoutWindows(render_targets_, max_bounds, layout_geo);
+  model_->SetRowSizes(layout_system_.GetRowSizes(render_targets_, max_bounds));
 
   return layout_geo;
 }
 
=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2013-05-29 14:52:11 +0000
+++ plugins/unityshell/src/unityshell.cpp	2013-06-18 19:07:26 +0000
@@ -321,8 +321,8 @@
      optionSetAltTabPrevAllInitiate(boost::bind(&UnityScreen::altTabPrevAllInitiate, this, _1, _2, _3));
      optionSetAltTabPrevInitiate(boost::bind(&UnityScreen::altTabPrevInitiate, this, _1, _2, _3));
 
-     optionSetAltTabDetailStartInitiate(boost::bind(&UnityScreen::altTabDetailStartInitiate, this, _1, _2, _3));
-     optionSetAltTabDetailStopInitiate(boost::bind(&UnityScreen::altTabDetailStopInitiate, this, _1, _2, _3));
+     optionSetAltTabDetailStartInitiate(boost::bind(&UnityScreen::altTabDetailStart, this, _1, _2, _3));
+     optionSetAltTabDetailStopInitiate(boost::bind(&UnityScreen::altTabDetailStop, this, _1, _2, _3));
 
      optionSetAltTabNextWindowInitiate(boost::bind(&UnityScreen::altTabNextWindowInitiate, this, _1, _2, _3));
      optionSetAltTabNextWindowTerminate(boost::bind(&UnityScreen::altTabTerminateCommon, this, _1, _2, _3));
@@ -1997,26 +1997,14 @@
   return false;
 }
 
-bool UnityScreen::altTabDetailStartInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options)
+bool UnityScreen::altTabDetailStart(CompAction* action, CompAction::State state, CompOption::Vector& options)
 {
-  if (switcher_controller_->Visible())
-  {
-    switcher_controller_->SetDetail(true);
-    return true;
-  }
-
-  return false;
+  return switcher_controller_->StartDetailMode();
 }
 
-bool UnityScreen::altTabDetailStopInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options)
+bool UnityScreen::altTabDetailStop(CompAction* action, CompAction::State state, CompOption::Vector& options)
 {
-  if (switcher_controller_->Visible())
-  {
-    switcher_controller_->SetDetail(false);
-    return true;
-  }
-
-  return false;
+  return switcher_controller_->StopDetailMode();
 }
 
 bool UnityScreen::altTabNextWindowInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options)
 
=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h	2013-05-17 22:53:57 +0000
+++ plugins/unityshell/src/unityshell.h	2013-06-18 19:07:26 +0000
@@ -152,8 +152,8 @@
   bool altTabPrevInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
   bool altTabForwardAllInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
   bool altTabPrevAllInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
-  bool altTabDetailStartInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
-  bool altTabDetailStopInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
+  bool altTabDetailStart(CompAction* action, CompAction::State state, CompOption::Vector& options);
+  bool altTabDetailStop(CompAction* action, CompAction::State state, CompOption::Vector& options);
   bool altTabNextWindowInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
   bool altTabPrevWindowInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
 
 
=== modified file 'tests/test_layout_system.cpp'
--- tests/test_layout_system.cpp	2013-01-25 21:06:34 +0000
+++ tests/test_layout_system.cpp	2013-06-18 19:07:26 +0000
@@ -21,6 +21,8 @@
 #include "LayoutSystem.h"
 #include "StandaloneWindowManager.h"
 
+#include <vector>
+
 namespace unity
 {
 namespace ui
@@ -153,6 +155,52 @@
   EXPECT_EQ(final_bounds.Intersect(windows_area), windows_area);
 }
 
+TEST_F(TestLayoutSystem, GetRowSizesEven)
+{
+  nux::Geometry max_bounds(0, 0, 200, 100);
+  nux::Geometry final_bounds;
+
+  Window xid = 3;
+  AddFakeWindowToWM(xid, nux::Geometry(4, 5, 200, 200));
+  lwindows.push_back(std::make_shared<LayoutWindow>(xid));
+
+  xid = 4;
+  AddFakeWindowToWM(xid, nux::Geometry(10, 20, 200, 200));
+  lwindows.push_back(std::make_shared<LayoutWindow>(xid));
+
+  ls.LayoutWindows(lwindows, max_bounds, final_bounds);
+
+  std::vector<int> const& row_sizes = ls.GetRowSizes(lwindows, max_bounds);
+  EXPECT_EQ(row_sizes.size(), 2);
+  EXPECT_EQ(row_sizes[0], 2);
+  EXPECT_EQ(row_sizes[1], 2);
+}
+
+TEST_F(TestLayoutSystem, GetRowSizesUnEven)
+{
+  nux::Geometry max_bounds(0, 0, 200, 100);
+  nux::Geometry final_bounds;
+
+  Window xid = 3;
+  AddFakeWindowToWM(xid, nux::Geometry(4, 5, 200, 200));
+  lwindows.push_back(std::make_shared<LayoutWindow>(xid));
+
+  xid = 4;
+  AddFakeWindowToWM(xid, nux::Geometry(10, 20, 200, 200));
+  lwindows.push_back(std::make_shared<LayoutWindow>(xid));
+
+  xid = 5;
+  AddFakeWindowToWM(xid, nux::Geometry(10, 20, 200, 200));
+  lwindows.push_back(std::make_shared<LayoutWindow>(xid));
+
+  ls.LayoutWindows(lwindows, max_bounds, final_bounds);
+
+  std::vector<int> const& row_sizes = ls.GetRowSizes(lwindows, max_bounds);
+  EXPECT_EQ(row_sizes.size(), 2);
+  EXPECT_EQ(row_sizes[0], 2);
+  EXPECT_EQ(row_sizes[1], 3);
+}
+
 }
 }
 }
 
=== modified file 'tests/test_switcher_controller.cpp'
--- tests/test_switcher_controller.cpp	2013-06-14 16:10:26 +0000
+++ tests/test_switcher_controller.cpp	2013-06-18 19:07:26 +0000
@@ -77,6 +77,77 @@
   EXPECT_FALSE(model->detail_selection());
 }
 
+TEST_F(TestSwitcherController, StartDetailMode)
+{
+  controller_->Show(ShowMode::ALL, SortMode::LAUNCHER_ORDER, icons_);
+  controller_->InitiateDetail();
+  controller_->StartDetailMode();
+
+  EXPECT_TRUE(controller_->IsDetailViewShown());
+}
+
+TEST_F(TestSwitcherController, StopDetailMode)
+{
+  controller_->Show(ShowMode::ALL, SortMode::LAUNCHER_ORDER, icons_);
+  controller_->InitiateDetail();
+
+  controller_->StartDetailMode();
+  EXPECT_TRUE(controller_->IsDetailViewShown());
+
+  controller_->StopDetailMode();
+  EXPECT_FALSE(controller_->IsDetailViewShown());
+}
+
+TEST_F(TestSwitcherController, StartDetailModeMovesNextRows)
+{
+  controller_->Show(ShowMode::ALL, SortMode::LAUNCHER_ORDER, icons_);
+  controller_->Select(2);
+  controller_->InitiateDetail();
+
+  controller_->StartDetailMode();
+  EXPECT_TRUE(controller_->IsDetailViewShown());
+
+  auto view = controller_->GetView();
+  auto model = view->GetModel();
+  model->SetRowSizes({2,2});
+
+  controller_->StartDetailMode();
+
+  // Grid: Assert we have gone down a row from index 0 -> 2
+  //  0, 1,
+  //  2, 3
+  EXPECT_FALSE(model->HasNextDetailRow());
+  EXPECT_TRUE(model->HasPrevDetailRow());
+  EXPECT_EQ(static_cast<unsigned int>(model->detail_selection_index), 2);
+}
+
+TEST_F(TestSwitcherController, StopDetailModeMovesPrevRows)
+{
+  controller_->Show(ShowMode::ALL, SortMode::LAUNCHER_ORDER, icons_);
+  controller_->Select(2);
+  controller_->InitiateDetail();
+
+  controller_->StartDetailMode();
+  EXPECT_TRUE(controller_->IsDetailViewShown());
+
+  auto view = controller_->GetView();
+  auto model = view->GetModel();
+  model->SetRowSizes({2,2});
+
+  controller_->StartDetailMode();
+  controller_->StopDetailMode();
+
+  // Assert we have gone up a row from index 2 -> 0
+  //  0, 1,
+  //  2, 3
+  EXPECT_TRUE(model->HasNextDetailRow());
+  EXPECT_FALSE(model->HasPrevDetailRow());
+  EXPECT_EQ(static_cast<unsigned int>(model->detail_selection_index), 0);
+
+  // Now we are in index 0, stoping detail mode must exit detail mode
+  controller_->StopDetailMode();
+  EXPECT_FALSE(controller_->IsDetailViewShown());
+}
 
 TEST_F(TestSwitcherController, ShowSwitcher)
 {
 
=== modified file 'tests/test_switcher_model.cpp'
--- tests/test_switcher_model.cpp	2013-02-15 15:32:11 +0000
+++ tests/test_switcher_model.cpp	2013-06-18 19:07:26 +0000
@@ -80,6 +80,7 @@
   EXPECT_EQ(model->LastSelection(), icons_.front());
   EXPECT_EQ(model->SelectionIndex(), 0);
   EXPECT_EQ(model->LastSelectionIndex(), 0);
+  EXPECT_EQ(static_cast<unsigned int>(model->detail_selection_index), 0);
 }
 
 
@@ -178,4 +179,153 @@
   EXPECT_NE(model->DetailXids().front(), base_model->DetailXids().front());
 }
 
+TEST_F(TestSwitcherModel, TestHasNextDetailRow)
+{
+  SwitcherModel::Ptr model(new SwitcherModel(icons_));
+  model->detail_selection = true;
+  model->SetRowSizes({2,2});
+
+  EXPECT_TRUE(model->HasNextDetailRow());
+
+  model->NextDetailRow();
+  EXPECT_FALSE(model->HasNextDetailRow());
+}
+
+TEST_F(TestSwitcherModel, TestHasPrevDetailRow)
+{
+  SwitcherModel::Ptr model(new SwitcherModel(icons_));
+  model->detail_selection = true;
+  model->SetRowSizes({2,2});
+
+  model->NextDetail();
+  EXPECT_TRUE(model->HasPrevDetailRow());
+
+  model->PrevDetailRow();
+  EXPECT_FALSE(model->HasPrevDetailRow());
+}
+
+TEST_F(TestSwitcherModel, TestHasNextThenPrevDetailRow)
+{
+  SwitcherModel::Ptr model(new SwitcherModel(icons_));
+  model->detail_selection = true;
+  model->SetRowSizes({2,2});
+
+  EXPECT_TRUE(model->HasNextDetailRow());
+
+  model->NextDetailRow();
+  EXPECT_FALSE(model->HasNextDetailRow());
+
+  EXPECT_TRUE(model->HasPrevDetailRow());
+  model->PrevDetailRow();
+  EXPECT_FALSE(model->HasPrevDetailRow());
+}
+
+TEST_F(TestSwitcherModel, TestNextDetailRow)
+{
+  SwitcherModel::Ptr model(new SwitcherModel(icons_));
+  model->detail_selection = true;
+  model->SetRowSizes({2,2});
+
+  model->NextDetailRow();
+
+  // Expect going form index 0 -> 2
+  // 0, 1
+  // 2, 3
+  EXPECT_EQ(static_cast<unsigned int>(model->detail_selection_index), 2);
+}
+
+TEST_F(TestSwitcherModel, TestNextDetailThenNextDetailRow)
+{
+  SwitcherModel::Ptr model(new SwitcherModel(icons_));
+  model->detail_selection = true;
+  model->SetRowSizes({2,2});
+
+  model->NextDetail();
+  model->NextDetailRow();
+
+  // Expect going form index 1 -> 3
+  // 0, 1
+  // 2, 3
+  EXPECT_EQ(static_cast<unsigned int>(model->detail_selection_index), 3);
+}
+
+TEST_F(TestSwitcherModel, TestPrevDetailRow)
+{
+  SwitcherModel::Ptr model(new SwitcherModel(icons_));
+  model->detail_selection = true;
+  model->SetRowSizes({2,2});
+
+  model->NextDetailRow();
+  model->PrevDetailRow();
+
+  // Expect going form index 0 -> 2, then index 2 -> 0
+  // 0, 1
+  // 2, 3
+  EXPECT_EQ(static_cast<unsigned int>(model->detail_selection_index), 0);
+}
+
+TEST_F(TestSwitcherModel, TestNextDetailThenPrevDetailRow)
+{
+  SwitcherModel::Ptr model(new SwitcherModel(icons_));
+  model->detail_selection = true;
+  model->SetRowSizes({2,2});
+
+  model->NextDetail();
+  model->NextDetailRow();
+
+  model->PrevDetailRow();
+
+  // Expect going form index 1 -> 3, then index 3 -> 1
+  // 0, 1
+  // 2, 3
+  EXPECT_EQ(static_cast<unsigned int>(model->detail_selection_index), 1);
+}
+
+TEST_F(TestSwitcherModel, TestUnEvenNextDetailRow)
+{
+  SwitcherModel::Ptr model(new SwitcherModel(icons_));
+  model->detail_selection = true;
+  model->SetRowSizes({3,2});
+
+  model->NextDetailRow();
+
+  // Expect going form index 0 -> 3
+  // 0, 1, 2,
+  //   3, 4
+  EXPECT_EQ(static_cast<unsigned int>(model->detail_selection_index), 3);
+}
+
+TEST_F(TestSwitcherModel, TestUnEvenPrevDetailRow)
+{
+  SwitcherModel::Ptr model(new SwitcherModel(icons_));
+  model->detail_selection = true;
+  model->SetRowSizes({3,2});
+
+  model->NextDetailRow();
+  model->PrevDetailRow();
+
+  // Expect going form index 0 -> 3, then 3 -> 0
+  // 0, 1, 2,
+  //   3, 4
+  EXPECT_EQ(static_cast<unsigned int>(model->detail_selection_index), 0);
+}
+
+TEST_F(TestSwitcherModel, TestNextPrevDetailRowMovesLeftInTopRow)
+{
+  SwitcherModel::Ptr model(new SwitcherModel(icons_));
+  model->detail_selection = true;
+  model->SetRowSizes({3,2});
+
+  model->NextDetail();
+  model->NextDetail();
+  model->PrevDetailRow();
+  model->PrevDetailRow();
+
+  // Expect going form index 0 -> 1, then 1 -> 2, then 2 -> 1, 1 -> 0
+  // since PrevDetailRow must go to the index 0 of at the top of the row
+  // 0, 1, 2,
+  //   3, 4
+  EXPECT_EQ(static_cast<unsigned int>(model->detail_selection_index), 0);
+}
+
 }
 
=== modified file 'unity-shared/LayoutSystem.cpp'
--- unity-shared/LayoutSystem.cpp	2013-02-20 21:40:31 +0000
+++ unity-shared/LayoutSystem.cpp	2013-06-18 19:07:26 +0000
@@ -35,7 +35,7 @@
   LayoutGridWindows(windows, max_bounds, final_bounds);
 }
 
-nux::Size LayoutSystem::GridSizeForWindows(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds)
+nux::Size LayoutSystem::GridSizeForWindows(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds) const
 {
   unsigned count = windows.size();
 
@@ -138,7 +138,18 @@
   return CompressAndPadRow (row, row_bounds);
 }
 
-std::vector<LayoutWindow::Vector> LayoutSystem::GetRows(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds)
+std::vector<int> LayoutSystem::GetRowSizes(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds) const
+{
+  std::vector<LayoutWindow::Vector> const& rows = GetRows(windows, max_bounds);
+  std::vector<int> row_sizes;
+
+  for (auto r : rows)
+    row_sizes.push_back(r.size());
+
+  return row_sizes;
+}
+
+std::vector<LayoutWindow::Vector> LayoutSystem::GetRows(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds) const
 {
   std::vector<LayoutWindow::Vector> rows;
 
 
=== modified file 'unity-shared/LayoutSystem.h'
--- unity-shared/LayoutSystem.h	2012-11-13 19:16:59 +0000
+++ unity-shared/LayoutSystem.h	2013-06-18 19:07:26 +0000
@@ -56,6 +56,7 @@
   LayoutSystem();
 
   void LayoutWindows(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds, nux::Geometry& final_bounds);
+  std::vector<int> GetRowSizes(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds) const;
 
 protected:
   void LayoutGridWindows(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds, nux::Geometry& final_bounds);
@@ -63,9 +64,9 @@
   nux::Geometry LayoutRow(LayoutWindow::Vector const& row, nux::Geometry const& row_bounds);
   nux::Geometry CompressAndPadRow(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds);
 
-  std::vector<LayoutWindow::Vector> GetRows(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds);
+  nux::Size GridSizeForWindows(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds) const;
 
-  nux::Size GridSizeForWindows(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds);
+  std::vector<LayoutWindow::Vector> GetRows(LayoutWindow::Vector const& windows, nux::Geometry const& max_bounds) const;
 
   nux::Geometry ScaleBoxIntoBox(nux::Geometry const& bounds, nux::Geometry const& box);
 };