More documentation improvements

- Added docs for CellStandardItem.
- Re-shuffled code in CellStandardItem.

Re #22263

dev-docs/source/BatchWidget/Internals/CellBasedViewProperties.rst

+.. _CellBasedViewProperties:
+
+==========================
+Cell Based View Properties
+==========================
+
+As described in :doc:`../API/JobTreeView` per-cell view properties can be retrieved as an
+instance of the :code:`Cell` class. The mechanics behind this are implemented in
+:code:`CellStandardItem.h`.
+
+Implementation
+##############
+
+This header contains an enumeration of the per-cell attributes which are not members of
+:code:`QStandardItem`, this enumeration starts at :code:`Qt::UserRole + 1`. Qt has a built in
+mechanism for extending the per-cell view attributes which is what we use here.
+
+Two significant functions defined in this file are :code:`applyCellPropertiesToItem` and
+:code:`extractCellPropertiesFromItem`. These functions are used to save the property values from the
+:code:`Cell` instance onto the :code:`QStandardItem` and later retrieve them.
+
+A note on adding new properties
+###############################
+
+When adding new properties to the :code:`Cell` class and by extension the :code:`CellUserRoles`
+enumeration it is important to consider if the new property is indeed a **view** property or whether it
+should instead be part of your model. The :code:`JobTreeView` is designed with this use case in mind
+and has mechanisms to allow you to synchronise the state of the view with the state of your model.

dev-docs/source/BatchWidget/Internals/QtStandardItemTreeModelAdapter.rst

@@ -5,8 +5,8 @@ QtStandardItemTreeModelAdapter
 ==============================
 
 The :code:`QtStandardItemTreeModelAdapter` is a wrapper around :code:`QStandardItemModel`
-it helps to enforce the strong typing on :code:`QModelIndex`\ s eliminates some of the boilerplate
-required when working with the model in the :doc:`../API/JobTreeView`, aiming to prevent
+which helps to enforce the strong typing on :code:`QModelIndex`\ s and eliminates some of the
+boilerplate required when working with the model in the :doc:`../API/JobTreeView`, aiming to prevent
 :code:`JobTreeView` and higher level classes from working directly with :code:`QStandardItem`\ s.
 
 It's header also contains definitions for :code:`modelItemFromIndex` who's usage in it's raw

dev-docs/source/BatchWidget/Internals/StrictModelIndexing.rst

@@ -72,3 +72,11 @@ Asserting that an index is from one space or the other as described above is not
 from one space into the other. This is performed using the functions :code:`mapToMainModel` and
 :code:`mapToFilteredModel` defined in :code:`JobTreeView` which internally call methods of the
 filtered model.
+
+Type Erasure
+------------
+
+Some functions are ambivalent to which model an index belongs to, a good example might be those in
+:code:`QtBasicNavigation.h`, in order to still be able to use these functions without requiring them
+all to be templates, the strict versions have a member function :code:`.untyped()` which returns the
+internal plain old :code:`QModelIndex`.

dev-docs/source/BatchWidget/index.rst

@@ -27,3 +27,4 @@ Internals Documentation
    Internals/FindRootNodes
    Internals/StrictModelIndexing
    Internals/QtStandardItemTreeModelAdapter
+   Internals/CellBasedViewProperties

qt/widgets/common/inc/MantidQtWidgets/Common/Batch/CellStandardItem.h

@@ -17,9 +17,18 @@ enum CellUserRoles {
 void setBorderThickness(QStandardItem &item, int borderThickness);
 int getBorderThickness(QStandardItem const &item);
 
-void setBorderColor(QStandardItem &item, QColor const &borderColor);
+void setBorderColor(QStandardItem &item, std::string const &borderColor,
+                    int alpha);
 QColor getBorderColor(QStandardItem const &item);
 
+std::string getIconFilePath(QStandardItem const &item);
+void setIcon(QStandardItem &item, std::string const &iconFilePath);
+void setIconFilePath(QStandardItem &item, QString const &iconFilePath);
+
+std::string getBackgroundColor(QStandardItem const &item);
+void setBackgroundColor(QStandardItem &item,
+                        std::string const &backgroundColor);
+
 void applyCellPropertiesToItem(Cell const &cell, QStandardItem &item);
 Cell extractCellPropertiesFromItem(QStandardItem const &item);
 }

qt/widgets/common/src/Batch/CellStandardItem.cpp

@@ -3,6 +3,29 @@ namespace MantidQt {
 namespace MantidWidgets {
 namespace Batch {
 
+void applyCellPropertiesToItem(Cell const &cell, QStandardItem &item) {
+  item.setText(QString::fromStdString(cell.contentText()));
+  item.setEditable(cell.isEditable());
+  setBorderThickness(item, cell.borderThickness());
+  setBackgroundColor(item, cell.backgroundColor());
+  setBorderColor(item, cell.borderColor(), cell.borderOpacity());
+  setIcon(item, cell.iconFilePath());
+}
+
+Cell extractCellPropertiesFromItem(QStandardItem const &item) {
+  auto cell = Cell(item.text().toStdString());
+  cell.setBorderThickness(getBorderThickness(item));
+  cell.setIconFilePath(getIconFilePath(item));
+  cell.setBackgroundColor(getBackgroundColor(item));
+
+  auto borderColor = getBorderColor(item);
+  cell.setBorderColor(borderColor.name().toStdString());
+  cell.setBorderOpacity(borderColor.alpha());
+
+  cell.setEditable(item.isEditable());
+  return cell;
+}
+
 void setBorderThickness(QStandardItem &item, int borderThickness) {
   item.setData(borderThickness, CellUserRoles::BorderThickness);
 }
@@ -53,28 +76,6 @@ QColor getBorderColor(QStandardItem const &item) {
   return item.data(CellUserRoles::BorderColor).value<QColor>();
 }
 
-void applyCellPropertiesToItem(Cell const &cell, QStandardItem &item) {
-  item.setText(QString::fromStdString(cell.contentText()));
-  item.setEditable(cell.isEditable());
-  setBorderThickness(item, cell.borderThickness());
-  setBackgroundColor(item, cell.backgroundColor());
-  setBorderColor(item, cell.borderColor(), cell.borderOpacity());
-  setIcon(item, cell.iconFilePath());
-}
-
-Cell extractCellPropertiesFromItem(QStandardItem const &item) {
-  auto cell = Cell(item.text().toStdString());
-  cell.setBorderThickness(getBorderThickness(item));
-  cell.setIconFilePath(getIconFilePath(item));
-  cell.setBackgroundColor(getBackgroundColor(item));
-
-  auto borderColor = getBorderColor(item);
-  cell.setBorderColor(borderColor.name().toStdString());
-  cell.setBorderOpacity(borderColor.alpha());
-
-  cell.setEditable(item.isEditable());
-  return cell;
-}
 }
 }
 }

qt/widgets/common/src/Batch/FindSubtreeRoots.cpp

@@ -3,24 +3,6 @@ namespace MantidQt {
 namespace MantidWidgets {
 namespace Batch {
 
-void FindSubtreeRoots::nodeWasSubtreeRoot(RowLocation const &rowLocation) {
-  previousWasRoot = true;
-  previousNode = rowLocation;
-}
-
-void FindSubtreeRoots::nodeWasNotSubtreeRoot(RowLocation const &rowLocation) {
-  previousWasRoot = false;
-  previousNode = rowLocation;
-}
-
-bool FindSubtreeRoots::isChildOfPrevious(RowLocation const &location) const {
-  return location.isChildOf(previousNode);
-}
-
-bool FindSubtreeRoots::isSiblingOfPrevious(RowLocation const &location) const {
-  return location.isSiblingOf(previousNode);
-}
-
 auto FindSubtreeRoots::operator()(std::vector<RowLocation> region)
     -> boost::optional<std::vector<RowLocation>> {
   std::sort(region.begin(), region.end());
@@ -36,10 +18,7 @@ auto FindSubtreeRoots::operator()(std::vector<RowLocation> region)
           (!previousWasRoot && isSiblingOfPrevious(currentNode))) {
         nodeWasNotSubtreeRoot(currentNode);
       } else if (currentNode.isDescendantOf(previousRoot)) {
-        if (previousNode.depth() < currentNode.depth())
-          return boost::none;
-        else
-          nodeWasNotSubtreeRoot(currentNode); // Dead case?
+        return boost::none;
       } else {
         nodeWasSubtreeRoot(currentNode);
         roots.emplace_back(currentNode);
@@ -52,6 +31,25 @@ auto FindSubtreeRoots::operator()(std::vector<RowLocation> region)
   }
 }
 
+void FindSubtreeRoots::nodeWasSubtreeRoot(RowLocation const &rowLocation) {
+  previousWasRoot = true;
+  previousNode = rowLocation;
+}
+
+void FindSubtreeRoots::nodeWasNotSubtreeRoot(RowLocation const &rowLocation) {
+  previousWasRoot = false;
+  previousNode = rowLocation;
+}
+
+bool FindSubtreeRoots::isChildOfPrevious(RowLocation const &location) const {
+  return location.isChildOf(previousNode);
+}
+
+bool FindSubtreeRoots::isSiblingOfPrevious(RowLocation const &location) const {
+  return location.isSiblingOf(previousNode);
+}
+
+
 } // namespace Batch
 } // namespace MantidWidgets
 } // namespace MantidQt