Unity 8

Merge lp:~stolowski/unity8/category-reordering into lp:unity8

category-reordering
Merge into trunk

Proposed by Paweł Stołowski on 2013-10-04

Status:	Merged
Approved by:	Albert Astals Cid on 2013-10-09
Approved revision:	253
Merged at revision:	416
Proposed branch:	lp:~stolowski/unity8/category-reordering
Merge into:	lp:unity8
Diff against target:	194 lines (+80/-18) 2 files modified plugins/Unity/categories.cpp (+70/-17) plugins/Unity/categories.h (+10/-1)
To merge this branch:	bzr merge lp:~stolowski/unity8/category-reordering
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Albert Astals Cid (community)		2013-10-04	Approve on 2013-10-09
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2013-10-09
Review via email: mp+189263@code.launchpad.net

Commit message

Handle category_order_changes signal from scopes (used in Home only) and reorder categories accordingly.

Description of the change

Handle category_order_changes signal from scopes (used in Home only) and reorder categories accordingly.

WARNING! It needs a hotfix for Qt proposed here: https://bugs.launchpad.net/ubuntu/+source/qtbase-opensource-src/+bug/1234603 ; please don't top-approve until we know it's going to land at the same time, otherwise this branch will break Dash.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-04:

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity8-ci/1258/rebuild

review: Approve (continuous-integration)

lp:~stolowski/unity8/category-reordering updated on 2013-10-09

248. By Paweł Stołowski on 2013-10-09: Removed unneeded include.
249. By Paweł Stołowski on 2013-10-09: Merged trunk.

Revision history for this message

Albert Astals Cid (aacid) wrote on 2013-10-09:

We don't use
const int mappedIndex = m_categoryOrder.indexOf(i);
for anything so we can save the lookup :-)

review: Needs Fixing

Revision history for this message

Albert Astals Cid (aacid) wrote on 2013-10-09:

Also
std::vector<unsigned int> cat_order
parameter should be const & as discussed on IRC

review: Needs Fixing

lp:~stolowski/unity8/category-reordering updated on 2013-10-09

250. By Paweł Stołowski on 2013-10-09: Act on review commetns.
251. By Paweł Stołowski on 2013-10-09: Fixed RoleRendererHint index.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-09:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1322/rebuild

review: Needs Fixing (continuous-integration)

lp:~stolowski/unity8/category-reordering updated on 2013-10-09

252. By Paweł Stołowski on 2013-10-09: Fixed beginMoveRows.
253. By Paweł Stołowski on 2013-10-09: Clear category order vector on categories model reset.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-09:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1326/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-09:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1328/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2013-10-09:

Looks good now

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Mohammad Rakibul Hasan

Paweł Stołowski

Unity Team

 === modified file 'plugins/Unity/categories.cpp'
 --- plugins/Unity/categories.cpp	2013-10-02 13:02:16 +0000
 +++ plugins/Unity/categories.cpp	2013-10-09 11:01:57 +0000
@@ -90,9 +90,37 @@
        delete model;
+     }
      m_results.clear();
++    m_categoryOrder.clear();
++
      setModel(model);
+ }
++void Categories::onCategoryOrderChanged(const std::vector<unsigned int>& cat_order)
++{
++    for (unsigned int pos = 0; pos<cat_order.size(); pos++)
++    {
++        unsigned int cat = cat_order[pos];
++        const int old_pos = m_categoryOrder.indexOf(cat);
++
++        if (old_pos < 0) {
++            qWarning() << "No such category index:" << cat;
++            continue;
++        }
++
++        if (static_cast<int>(pos) != old_pos) {
++            int target_pos = pos;
++            if (target_pos > old_pos)
++                target_pos++;
++            const bool status = beginMoveRows(QModelIndex(), old_pos, old_pos, QModelIndex(), target_pos);
++            if (status)
++                m_categoryOrder.move(old_pos, pos);
++            else
++                qWarning() << "beginMoveRows failed:" << old_pos << target_pos;
++            endMoveRows();
++        }
++    }
++}
++
  void
  Categories::setUnityScope(const unity::dash::Scope::Ptr& scope)
+ {
@@ -101,9 +129,9 @@
      // no need to call this, we'll get notified
      //setModel(m_unityScope->categories()->model());
--    m_categoriesChangedConnection.disconnect();
--    m_categoriesChangedConnection =
--        m_unityScope->categories()->model.changed.connect(sigc::mem_fun(this, &Categories::onCategoriesModelChanged));
++    m_signals.disconnectAll();
++    m_signals << m_unityScope->categories()->model.changed.connect(sigc::mem_fun(this, &Categories::onCategoriesModelChanged));
++    m_signals << m_unityScope->category_order.changed.connect(sigc::mem_fun(this, &Categories::onCategoryOrderChanged));
+ }
  void
@@ -111,7 +139,9 @@
+ {
      CategoryResults* results = qobject_cast<CategoryResults*>(sender());
      if (results) {
--        m_updatedCategories << results->categoryIndex();
++        if (!m_updatedCategories.contains(results->categoryIndex())) {
++            m_updatedCategories << results->categoryIndex();
++        }
          m_timer.start();
+     }
+ }
@@ -142,7 +172,8 @@
      roles.append(Categories::RoleCount);
      Q_FOREACH(int categoryIndex, m_updatedCategories) {
          if (!m_results.contains(categoryIndex)) continue;
--        QModelIndex changedIndex = index(categoryIndex);
++        const int realIndex = m_categoryOrder.indexOf(categoryIndex);
++        const QModelIndex changedIndex = index(realIndex);
          Q_EMIT dataChanged(changedIndex, changedIndex, roles);
+     }
      m_updatedCategories.clear();
@@ -196,48 +227,70 @@
          return QVariant();
+     }
++    if (m_categoryOrder.size() != rowCount())
++    {
++        // populate category order vector with 0..n
++        m_categoryOrder.clear();
++        const unsigned int lim = rowCount();
++        for (unsigned int i = 0; i<lim; i++) {
++            m_categoryOrder.append(i);
++        }
++    }
++
++    int realRow = index.row();
++    if (index.row() < m_categoryOrder.size()) {
++        realRow = m_categoryOrder[index.row()];
++    } else {
++        qWarning() << "Invalid index" << index.row();
++        return QVariant();
++    }
++
++    const QModelIndex realIndex = createIndex(realRow, index.column());
++
      switch (role) {
          case RoleCategoryId:
--            return DeeListModel::data(index, CategoryColumn::ID);
++            return DeeListModel::data(realIndex, CategoryColumn::ID);
          case RoleName:
--            return DeeListModel::data(index, CategoryColumn::DISPLAY_NAME);
++            return DeeListModel::data(realIndex, CategoryColumn::DISPLAY_NAME);
          case RoleIcon:
--            return DeeListModel::data(index, CategoryColumn::ICON_HINT);
++            return DeeListModel::data(realIndex, CategoryColumn::ICON_HINT);
          case RoleRenderer:
--            return DeeListModel::data(index, CategoryColumn::RENDERER_NAME);
++            return DeeListModel::data(realIndex, CategoryColumn::RENDERER_NAME);
          case RoleContentType:
+         {
--            auto hints = DeeListModel::data(index, CategoryColumn::HINTS).toHash();
++            auto hints = DeeListModel::data(realIndex, CategoryColumn::HINTS).toHash();
              return hints.contains("content-type") ? hints["content-type"] : QVariant(QString("default"));
+         }
          case RoleRendererHint:
+         {
--            auto hints = DeeListModel::data(index, CategoryColumn::HINTS).toHash();
++            auto hints = DeeListModel::data(realIndex, CategoryColumn::HINTS).toHash();
              return hints.contains("renderer-hint") ? hints["renderer-hint"] : QVariant(QString());
+         }
          case RoleProgressSource:
+         {
--            auto hints = DeeListModel::data(index, CategoryColumn::HINTS).toHash();
++            auto hints = DeeListModel::data(realIndex, CategoryColumn::HINTS).toHash();
              return hints.contains("progress-source") ? hints["progress-source"] : QVariant();
+         }
          case RoleHints:
--            return DeeListModel::data(index, CategoryColumn::HINTS);
++            return DeeListModel::data(realIndex, CategoryColumn::HINTS);
          case RoleResults:
              if (m_overriddenCategories.size() > 0)
+             {
--                auto id = DeeListModel::data(index, CategoryColumn::ID).toString();
++                auto id = DeeListModel::data(realIndex, CategoryColumn::ID).toString();
                  if (m_overriddenCategories.find(id) != m_overriddenCategories.end())
                      return QVariant::fromValue(m_overriddenCategories[id]);
+             }
--            return QVariant::fromValue(getResults(index.row()));
++            return QVariant::fromValue(getResults(realRow));
          case RoleCount:
              if (m_overriddenCategories.size() > 0)
+             {
--                auto id = DeeListModel::data(index, CategoryColumn::ID).toString();
++                auto id = DeeListModel::data(realIndex, CategoryColumn::ID).toString();
                  if (m_overriddenCategories.find(id) != m_overriddenCategories.end())
++                {
                      return QVariant::fromValue(m_overriddenCategories[id]->rowCount());
++                }
+             }
--            return QVariant::fromValue(getResults(index.row())->rowCount());
++            return QVariant::fromValue(getResults(realRow)->rowCount());
          default:
              return QVariant();
+     }
 === modified file 'plugins/Unity/categories.h'
 --- plugins/Unity/categories.h	2013-10-02 13:02:16 +0000
 +++ plugins/Unity/categories.h	2013-10-09 11:01:57 +0000
@@ -32,6 +32,9 @@
  #include <QSet>
  #include <QTimer>
++// local
++#include "signalslist.h"
++
  class Categories : public DeeListModel
+ {
      Q_OBJECT
@@ -71,6 +74,7 @@
  private:
      void onCategoriesModelChanged(unity::glib::Object<DeeModel> model);
++    void onCategoryOrderChanged(const std::vector<unsigned int>& cat_order);
      DeeListModel* getResults(int index) const;
@@ -80,7 +84,12 @@
      QHash<int, QByteArray> m_roles;
      QMap<QString, QAbstractItemModel*> m_overriddenCategories;
      mutable QMap<int, DeeListModel*> m_results;
--    sigc::connection m_categoriesChangedConnection;
++    SignalsList m_signals;
++
++    /* Category order array contains indices of actual categories in the underlying DeeListModel.
++       It's used internally to reflect category order reported by scope.
++     */
++    mutable QList<unsigned int> m_categoryOrder;
  };
  #endif // CATEGORIES_H