Handle custom column classes; inherit from HTMLElement.

* Fixes bug 960588, allowing custom (and mixed) column classes
    to be handled appropriately in the table metaclass.
  * Reworks Column and Cell to inherit from HTMLElement, making
    them both more consistent and easier to customize.
    Fixes bug 960590.

Change-Id: I88ec6d8d66703f11c508b4c10af439e3b732b3ce

horizon/tables/base.py

@@ -45,7 +45,7 @@ PALETTE = termcolors.PALETTES[termcolors.DEFAULT_PALETTE]
 STRING_SEPARATOR = "__"
 
 
-class Column(object):
+class Column(html.HTMLElement):
     """ A class which represents a single column in a :class:`.DataTable`.
 
     .. attribute:: transform
@@ -120,6 +120,16 @@ class Column(object):
         A list of functions (often template filters) to be applied to the
         value of the data for this column prior to output. This is effectively
         a shortcut for writing a custom ``transform`` function in simple cases.
+
+    .. attribute:: classes
+
+        An iterable of CSS classes which should be added to this column.
+        Example: ``classes=('foo', 'bar')``.
+
+    .. attribute:: attrs
+
+        A dict of HTML attribute strings which should be added to this column.
+        Example: ``attrs={"data-foo": "bar"}``.
     """
     # Used to retain order when instantiating columns on a table
     creation_counter = 0
@@ -147,7 +157,12 @@ class Column(object):
 
     def __init__(self, transform, verbose_name=None, sortable=False,
                  link=None, hidden=False, attrs=None, status=False,
-                 status_choices=None, empty_value=None, filters=None):
+                 status_choices=None, empty_value=None, filters=None,
+                 classes=None):
+        self.classes = classes or getattr(self, "classes", [])
+        super(Column, self).__init__()
+        self.attrs.update(attrs or {})
+
         if callable(transform):
             self.transform = transform
             self.name = transform.__name__
@@ -172,14 +187,10 @@ class Column(object):
         self.creation_counter = Column.creation_counter
         Column.creation_counter += 1
 
-        self.attrs = {"classes": []}
-        self.attrs.update(attrs or {})
-        # Make sure we have a mutable list.
-        self.attrs['classes'] = list(self.attrs['classes'])
         if self.sortable:
-            self.attrs['classes'].append("sortable")
+            self.classes.append("sortable")
         if self.hidden:
-            self.attrs['classes'].append("hide")
+            self.classes.append("hide")
 
     def __unicode__(self):
         return self.verbose_name
@@ -221,10 +232,6 @@ class Column(object):
         self.table._data_cache[self][datum_id] = data
         return self.table._data_cache[self][datum_id]
 
-    def get_classes(self):
-        """ Returns a flattened string of the column's CSS classes. """
-        return " ".join(self.attrs['classes'])
-
     def get_link_url(self, datum):
         """ Returns the final value for the column's ``link`` property.
 
@@ -382,15 +389,17 @@ class Row(html.HTMLElement):
                                   % cls.__name__)
 
 
-class Cell(object):
+class Cell(html.HTMLElement):
     """ Represents a single cell in the table. """
-    def __init__(self, datum, data, column, row, attrs=None):
+    def __init__(self, datum, data, column, row, attrs=None, classes=None):
+        self.classes = classes or getattr(self, "classes", [])
+        super(Cell, self).__init__()
+        self.attrs.update(attrs or {})
+
         self.datum = datum
         self.data = data
         self.column = column
         self.row = row
-        self.attrs = {'classes': []}
-        self.attrs.update(attrs or {})
 
     def __repr__(self):
         return '<%s: %s, %s>' % (self.__class__.__name__,
@@ -446,12 +455,13 @@ class Cell(object):
         else:
             return "status_unknown"
 
-    def get_classes(self):
+    def get_default_classes(self):
         """ Returns a flattened string of the cell's CSS classes. """
-        union = set(self.attrs['classes']) | set(self.column.attrs['classes'])
+        column_class_string = self.column.get_final_attrs().get('class', "")
+        classes = set(column_class_string.split(" "))
         if self.column.status:
-            union.add(self.get_status_class(self.status))
+            classes.add(self.get_status_class(self.status))
-        return " ".join(union)
+        return list(classes)
 
 
 class DataTableOptions(object):
@@ -583,9 +593,9 @@ class DataTableMetaclass(type):
 
         # Gather columns; this prevents the column from being an attribute
         # on the DataTable class and avoids naming conflicts.
-        columns = [(column_name, attrs.pop(column_name)) for \
+        columns = [(column_name, attrs.pop(column_name)) for
-                            column_name, obj in attrs.items() \
+                   column_name, obj in attrs.items()
-                            if isinstance(obj, opts.column_class)]
+                   if issubclass(type(obj), (opts.column_class, Column))]
         # add a name attribute to each column
         for column_name, column in columns:
             column.name = column_name
@@ -608,13 +618,13 @@ class DataTableMetaclass(type):
         if opts.multi_select:
             multi_select = opts.column_class("multi_select",
                                              verbose_name="")
-            multi_select.attrs = {'classes': ('multi_select_column',)}
+            multi_select.classes.append('multi_select_column')
             multi_select.auto = "multi_select"
             columns.insert(0, ("multi_select", multi_select))
         if opts.actions_column:
             actions_column = opts.column_class("actions",
                                                verbose_name=_("Actions"))
-            actions_column.attrs = {'classes': ('actions_column',)}
+            actions_column.classes.append('actions_column')
             actions_column.auto = "actions"
             columns.append(("actions", actions_column))
         attrs['columns'] = SortedDict(columns)

horizon/templates/horizon/common/_data_table.html

@@ -9,7 +9,7 @@
     <thead>
       <tr>
         {% for column in columns %}
-          <th class="{{ column.get_classes }}">{{ column }}</th>
+          <th {{ column.attr_string|safe }}>{{ column }}</th>
         {% endfor %}
       </tr>
     </thead>

horizon/templates/horizon/common/_data_table_row.html

@@ -1,3 +1,3 @@
 <tr {{ row.attr_string|safe }}>
-  {% for cell in row %}<td class="{{ cell.get_classes }}">{{ cell.value }}</td>{% endfor %}
+  {% for cell in row %}<td {{ cell.attr_string|safe }}>{{ cell.value }}</td>{% endfor %}
 </tr>

horizon/tests/table_tests.py

@@ -76,6 +76,10 @@ class MyAction(tables.Action):
         return shortcuts.redirect('http://example.com/%s' % len(object_ids))
 
 
+class MyColumn(tables.Column):
+    pass
+
+
 class MyRow(tables.Row):
     ajax = True
 
@@ -142,7 +146,7 @@ class MyTable(tables.DataTable):
     value = tables.Column('value',
                           sortable=True,
                           link='http://example.com/',
-                          attrs={'classes': ('green', 'blue')})
+                          attrs={'class': 'green blue'})
     status = tables.Column('status', link=get_link)
     optional = tables.Column('optional', empty_value='N/A')
     excluded = tables.Column('excluded')
@@ -153,6 +157,7 @@ class MyTable(tables.DataTable):
         status_columns = ["status"]
         columns = ('id', 'name', 'value', 'optional', 'status')
         row_class = MyRow
+        column_class = MyColumn
         table_actions = (MyFilterAction, MyAction, MyBatchAction)
         row_actions = (MyAction, MyLinkAction, MyBatchAction, MyToggleAction)
 
@@ -172,14 +177,16 @@ class DataTableTests(test.TestCase):
         # Column ordering and exclusion.
         # This should include auto-columns for multi_select and actions,
         # but should not contain the excluded column.
+        # Additionally, auto-generated columns should use the custom
+        # column class specified on the table.
         self.assertQuerysetEqual(self.table.columns.values(),
-                                 ['<Column: multi_select>',
+                                 ['<MyColumn: multi_select>',
                                   '<Column: id>',
                                   '<Column: name>',
                                   '<Column: value>',
                                   '<Column: optional>',
                                   '<Column: status>',
-                                  '<Column: actions>'])
+                                  '<MyColumn: actions>'])
         # Actions (these also test ordering)
         self.assertQuerysetEqual(self.table.base_actions.values(),
                                  ['<MyBatchAction: batch>',
@@ -199,10 +206,12 @@ class DataTableTests(test.TestCase):
         # Auto-generated columns
         multi_select = self.table.columns['multi_select']
         self.assertEqual(multi_select.auto, "multi_select")
-        self.assertEqual(multi_select.get_classes(), "multi_select_column")
+        self.assertEqual(multi_select.get_final_attrs().get('class', ""),
+                         "multi_select_column")
         actions = self.table.columns['actions']
         self.assertEqual(actions.auto, "actions")
-        self.assertEqual(actions.get_classes(), "actions_column")
+        self.assertEqual(actions.get_final_attrs().get('class', ""),
+                         "actions_column")
 
     def test_table_force_no_multiselect(self):
         class TempTable(MyTable):
@@ -273,13 +282,13 @@ class DataTableTests(test.TestCase):
         self.table = MyTable(self.request, TEST_DATA)
         # Verify we retrieve the right columns for headers
         columns = self.table.get_columns()
-        self.assertQuerysetEqual(columns, ['<Column: multi_select>',
+        self.assertQuerysetEqual(columns, ['<MyColumn: multi_select>',
                                            '<Column: id>',
                                            '<Column: name>',
                                            '<Column: value>',
                                            '<Column: optional>',
                                            '<Column: status>',
-                                           '<Column: actions>'])
+                                           '<MyColumn: actions>'])
         # Verify we retrieve the right rows from our data
         rows = self.table.get_rows()
         self.assertQuerysetEqual(rows, ['<MyRow: my_table__row__1>',
@@ -310,21 +319,22 @@ class DataTableTests(test.TestCase):
         self.assertEqual(unicode(name_col), "Verbose Name")
         # sortable
         self.assertEqual(id_col.sortable, False)
-        self.assertNotIn("sortable", id_col.get_classes())
+        self.assertNotIn("sortable", id_col.get_final_attrs().get('class', ""))
         self.assertEqual(name_col.sortable, True)
-        self.assertIn("sortable", name_col.get_classes())
+        self.assertIn("sortable", name_col.get_final_attrs().get('class', ""))
         # hidden
         self.assertEqual(id_col.hidden, True)
-        self.assertIn("hide", id_col.get_classes())
+        self.assertIn("hide", id_col.get_final_attrs().get('class', ""))
         self.assertEqual(name_col.hidden, False)
-        self.assertNotIn("hide", name_col.get_classes())
+        self.assertNotIn("hide", name_col.get_final_attrs().get('class', ""))
         # link and get_link_url
         self.assertIn('href="http://example.com/"', row.cells['value'].value)
         self.assertIn('href="/auth/login/"', row.cells['status'].value)
         # empty_value
         self.assertEqual(row3.cells['optional'].value, "N/A")
-        # get_classes
+        # classes
-        self.assertEqual(value_col.get_classes(), "green blue sortable")
+        self.assertEqual(value_col.get_final_attrs().get('class', ""),
+                         "green blue sortable")
         # status
         cell_status = row.cells['status'].status
         self.assertEqual(cell_status, True)

horizon/utils/html.py

@@ -11,19 +11,22 @@ class HTMLElement(object):
 
     def get_default_classes(self):
         """
-        Returns a list of default classes which should be combined with any
+        Returns an iterable of default classes which should be combined with
-        other declared classes.
+        any other declared classes.
         """
         return []
 
     def get_default_attrs(self):
+        """
+        Returns a dict of default attributes which should be combined with
+        other declared attributes.
+        """
         return {}
 
-    @property
+    def get_final_attrs(self):
-    def attr_string(self):
         """
-        Returns a flattened string of HTML attributes based on the
+        Returns a dict containing the final attributes of this element
-        ``attrs`` dict provided to the class.
+        which will be rendered.
         """
         final_attrs = copy.copy(self.get_default_attrs())
         final_attrs.update(self.attrs)
@@ -31,6 +34,15 @@ class HTMLElement(object):
         default = " ".join(self.get_default_classes())
         defined = self.attrs.get('class', '')
         additional = " ".join(getattr(self, "classes", []))
-        final_classes = " ".join((defined, default, additional)).strip()
+        non_empty = [test for test in (defined, default, additional) if test]
+        final_classes = " ".join(non_empty).strip()
         final_attrs.update({'class': final_classes})
-        return flatatt(final_attrs)
+        return final_attrs
+
+    @property
+    def attr_string(self):
+        """
+        Returns a flattened string of HTML attributes based on the
+        ``attrs`` dict provided to the class.
+        """
+        return flatatt(self.get_final_attrs())