Skip to content

Commit

Permalink
Remove default addition of the column name to <td>/<th> class-attribute
Browse files Browse the repository at this point in the history
As mentioned in #370:
#370 (comment)
  • Loading branch information
jieter committed Apr 10, 2018
1 parent 38a0edf commit 0bb23fa
Show file tree
Hide file tree
Showing 13 changed files with 134 additions and 141 deletions.
14 changes: 12 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,18 @@
- Cleaned up templates to add consistancy in what is presented accross all templates.
- Added bootstrap4.html template
- Fixed translation inconsistancies.
- **breaking change** removed the `template` argument to the table constructor, use `template_name` instead.
-
### breaking changes
- Appearance of the paginators might be different from the current 1.x templates. Use a custom template if you need to keep the appearance the same.
- Removed the `template` argument to the table constructor, use `template_name` instead.
- Stopped adding column names to the class attribute of table cells (`<td>` tags) by default. Previous behaviour can be restored by using this method on your custom table:
```python
class MyTable(tables.Table):
# columns
def get_column_class_names(self, classes_set, bound_column):
classes_set = super(MyTable, self).get_column_class_names(classes_set, bound_column)
classes_set.add(bound_column.name)
return classes_set
```

## 1.21.2 (2018-03-26)
- Moved table instantiation from `get_context_data` to `get_tables` [#554](https://github.com/jieter/django-tables2/pull/554) by [@sdolemelipone](https://github.com/sdolemelipone)
Expand Down
11 changes: 6 additions & 5 deletions django_tables2/columns/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ def attrs(self):

# we take the value for 'cell' as the basis for both the th and td attrs
cell_attrs = attrs.get('cell', {})

# override with attrs defined specifically for th and td respectively.
attrs['th'] = computed_values(attrs.get('th', cell_attrs), kwargs=kwargs)
attrs['td'] = computed_values(attrs.get('td', cell_attrs), kwargs=kwargs)
Expand All @@ -347,19 +348,19 @@ def attrs(self):

def _get_cell_class(self, attrs):
'''
return a set of the classes from the class key in ``attrs``, augmented
with the column name (or anything else added by Table.get_column_class_names()).
Return a set of the classes from the class key in ``attrs``.
'''
classes = attrs.get('class', None)
classes = set() if classes is None else {c for c in classes.split(' ') if c}
classes = set() if classes is None else set(classes.split(' '))

return self._table.get_column_class_names(classes, self)

def get_td_class(self, td_attrs):
'''
Returns the HTML class attribute for a data cell in this column
'''
return ' '.join(sorted(self._get_cell_class(td_attrs)))
classes = sorted(self._get_cell_class(td_attrs))
return None if len(classes) == 0 else ' '.join(classes)

def get_th_class(self, th_attrs):
'''
Expand All @@ -376,7 +377,7 @@ def get_th_class(self, th_attrs):
if self.order_by_alias.is_descending
else ordering_class.get('ascending', 'asc'))

return ' '.join(sorted(classes))
return None if len(classes) == 0 else ' '.join(classes)

@property
def default(self):
Expand Down
15 changes: 13 additions & 2 deletions django_tables2/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ def get_column_class_names(self, classes_set, bound_column):
Returns a set of HTML class names for cells (both td and th) of a
**bound column** in this table.
By default this returns the column class names defined in the table's
attributes, and additionally the bound column's name.
attributes.
This method can be overridden to change the default behavior, for
example to simply `return classes_set`.
Expand All @@ -611,8 +611,19 @@ def get_column_class_names(self, classes_set, bound_column):
Returns:
A set of class names to be added to cells of this column
If you want to add the column names to the list of classes for a column,
override this method in your custom table::
class MyTable(tables.Table):
...
def get_column_class_names(self, classes_set, bound_column):
classes_set = super(MyTable, self).get_column_class_names(classes_set, bound_column)
classes_set.add(bound_column.name)
return classes_set
'''
classes_set.add(bound_column.name)
return classes_set


Expand Down
6 changes: 2 additions & 4 deletions docs/pages/column-attributes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ Depending on the column, different elements are supported, however ``th``,
... name = tables.Column(attrs={'th': {'id': 'foo'}})
...
>>> # will render something like this:
'{snip}<thead><tr><th id="foo" class="name">{snip}<tbody><tr><td class="name">{snip}'
'{snip}<thead><tr><th id="foo">{snip}<tbody><tr><td>{snip}'


For ``th`` and ``td``, the column name will be added as a class name. This makes
selecting the row for styling easier. Have a look at each column's API
reference to find which elements are supported.
Have a look at each column's API reference to find which elements are supported.

If you need to add some extra attributes to column's tags rendered in the
footer, use key name ``tf``, as described in section on :ref:`css`.
Expand Down
62 changes: 23 additions & 39 deletions tests/columns/test_general.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,18 +256,9 @@ class SimpleTable(tables.Table):
table = SimpleTable([{'a': 'value'}])
root = parse(table.as_html(request))

assert root.findall('.//thead/tr/th')[0].attrib == {'key': 'value', 'class': 'a orderable'}
assert root.findall('.//tbody/tr/td')[0].attrib == {'key': 'value', 'class': 'a'}
assert root.findall('.//tfoot/tr/td')[0].attrib == {'key': 'value', 'class': 'a'}

def test_cells_are_automatically_given_column_name_as_class(self):
class SimpleTable(tables.Table):
a = tables.Column()

table = SimpleTable([{'a': 'value'}])
root = parse(table.as_html(request))
assert root.findall('.//thead/tr/th')[0].attrib == {'class': 'a orderable'}
assert root.findall('.//tbody/tr/td')[0].attrib == {'class': 'a'}
assert root.findall('.//thead/tr/th')[0].attrib == {'key': 'value', 'class': 'orderable'}
assert root.findall('.//tbody/tr/td')[0].attrib == {'key': 'value'}
assert root.findall('.//tfoot/tr/td')[0].attrib == {'key': 'value'}

def test_th_are_given_orderable_class_if_column_is_orderable(self):
class SimpleTable(tables.Table):
Expand All @@ -277,9 +268,9 @@ class SimpleTable(tables.Table):
table = SimpleTable([{'a': 'value'}])
root = parse(table.as_html(request))
# return classes of an element as a set
classes = lambda x: set(x.attrib['class'].split())
assert 'orderable' in classes(root.findall('.//thead/tr/th')[0])
assert 'orderable' not in classes(root.findall('.//thead/tr/th')[1])
classes = lambda x: set(x.attrib.get('class', '').split())
self.assertIn('orderable', classes(root.findall('.//thead/tr/th')[0]))
self.assertNotIn('orderable', classes(root.findall('.//thead/tr/th')[1]))

# Now try with an ordered table
table = SimpleTable([], order_by='a')
Expand Down Expand Up @@ -443,10 +434,10 @@ class Table(tables.Table):
table = Table(Person.objects.all())
html = table.as_html(request)
# cell should affect both <th> and <td>
assert '<th data-length="2" class="orderable person">' in html
assert '<td data-length="2" class="person">' in html
self.assertIn('<th data-length="2" class="orderable">', html)
self.assertIn('<td data-length="2">', html)
# td should only affect <td>
assert '<td class="first_name status-2">' in html
self.assertIn('<td class="status-2">', html)

def test_computable_td_attrs_defined_in_column_class_attribute(self):
'''Computable attrs for columns, using custom Column'''
Expand All @@ -465,14 +456,8 @@ class Table(tables.Table):
html = table.as_html(request)
root = parse(html)

assert root.findall('.//tbody/tr/td')[0].attrib == {
'data-test': '2',
'class': 'last_name'
}
assert root.findall('.//tbody/tr/td')[1].attrib == {
'data-test': '2',
'class': 'last_name'
}
self.assertEqual(root.findall('.//tbody/tr/td')[0].attrib, {'data-test': '2'})
self.assertEqual(root.findall('.//tbody/tr/td')[1].attrib, {'data-test': '2'})

def test_computable_td_attrs_defined_in_column_class_attribute_record(self):
'''Computable attrs for columns, using custom column'''
Expand All @@ -495,11 +480,10 @@ class Table(tables.Table):
html = table.as_html(request)
root = parse(html)

assert root.findall('.//tbody/tr/td')[0].attrib == {
self.assertEqual(root.findall('.//tbody/tr/td')[0].attrib, {
'data-first-name': 'Jan',
'data-last-name': 'Pietersz.',
'class': 'person'
}
'data-last-name': 'Pietersz.'
})

def test_computable_column_td_attrs_record_header(self):
'''
Expand All @@ -523,15 +507,15 @@ class Table(tables.Table):
html = table.as_html(request)
root = parse(html)

assert root.findall('.//thead/tr/th')[0].attrib == {
'class': 'first_name orderable',
self.assertEqual(root.findall('.//thead/tr/th')[0].attrib, {
'class': 'orderable',
'data-first-name': 'header',
}
assert root.findall('.//tbody/tr/td')[0].attrib == {
'class': 'first_name status-Jan',
})
self.assertEqual(root.findall('.//tbody/tr/td')[0].attrib, {
'class': 'status-Jan',
'data-first-name': 'Jan',
}
assert root.findall('.//tbody/tr/td')[1].attrib == {
'class': 'first_name status-Sjon',
})
self.assertEqual(root.findall('.//tbody/tr/td')[1].attrib, {
'class': 'status-Sjon',
'data-first-name': 'Sjon',
}
})
45 changes: 20 additions & 25 deletions tests/columns/test_linkcolumn.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ class UnicodeTable(tables.Table):
'table': UnicodeTable(dataset)
}))

assert 'Brädley' in html
assert '∆yers' in html
assert 'Chr…s' in html
assert 'DÒble' in html
self.assertIn('Brädley', html)
self.assertIn('∆yers', html)
self.assertIn('Chr…s', html)
self.assertIn('DÒble', html)

def test_link_text_custom_value(self):
class CustomLinkTable(tables.Table):
Expand All @@ -51,8 +51,8 @@ class CustomLinkTable(tables.Table):

html = CustomLinkTable(dataset).as_html(build_request())

assert 'foo::bar' in html
assert 'Doe John' in html
self.assertIn('foo::bar', html)
self.assertIn('Doe John', html)

def test_link_text_escaping(self):
class CustomLinkTable(tables.Table):
Expand All @@ -68,10 +68,8 @@ class CustomLinkTable(tables.Table):

html = CustomLinkTable(dataset).as_html(build_request())

expected = '<td class="editlink"><a href="{}">edit</a></td>'.format(
reverse('person', args=(1, ))
)
assert expected in html
expected = '<td ><a href="{}">edit</a></td>'.format(reverse('person', args=(1, )))
self.assertIn(expected, html)

def test_null_foreign_key(self):
class PersonTable(tables.Table):
Expand All @@ -84,7 +82,7 @@ class PersonTable(tables.Table):
table = PersonTable(Person.objects.all())
html = table.as_html(build_request())

assert '<td class="occupation">—</td>' in html
self.assertIn('<td >—</td>', html)

def test_linkcolumn_non_field_based(self):
'''Test for issue 257, non-field based columns'''
Expand All @@ -96,37 +94,37 @@ class Table(tables.Table):

html = Table(Person.objects.all()).as_html(build_request())

expected = '<td class="delete_link"><a href="{}">delete</a></td>'.format(
expected = '<td ><a href="{}">delete</a></td>'.format(
reverse('person_delete', kwargs={'pk': willem.pk})
)
assert expected in html
self.assertIn(expected, html)

def test_kwargs(self):
class PersonTable(tables.Table):
a = tables.LinkColumn('occupation', kwargs={'pk': A('a')})

table = PersonTable([{'a': 0}, {'a': 1}])

assert reverse('occupation', kwargs={'pk': 0}) in table.rows[0].get_cell('a')
assert reverse('occupation', kwargs={'pk': 1}) in table.rows[1].get_cell('a')
self.assertIn(reverse('occupation', kwargs={'pk': 0}), table.rows[0].get_cell('a'))
self.assertIn(reverse('occupation', kwargs={'pk': 1}), table.rows[1].get_cell('a'))

def test_html_escape_value(self):
class PersonTable(tables.Table):
name = tables.LinkColumn('escaping', kwargs={'pk': A('pk')})

table = PersonTable([{'name': '<brad>', 'pk': 1}])
assert table.rows[0].get_cell('name') == '<a href="/&amp;&#39;%22/1/">&lt;brad&gt;</a>'
self.assertEqual(table.rows[0].get_cell('name'), '<a href="/&amp;&#39;%22/1/">&lt;brad&gt;</a>')

def test_a_attrs_should_be_supported(self):
class TestTable(tables.Table):
col = tables.LinkColumn('occupation', kwargs={'pk': A('col')},
attrs={'a': {'title': 'Occupation Title'}})

table = TestTable([{'col': 0}])
assert attrs(table.rows[0].get_cell('col')) == {
self.assertEqual(attrs(table.rows[0].get_cell('col')), {
'href': reverse('occupation', kwargs={'pk': 0}),
'title': 'Occupation Title'
}
})

def test_td_attrs_should_be_supported(self):
'''LinkColumn should support both <td> and <a> attrs'''
Expand All @@ -143,16 +141,13 @@ class Table(tables.Table):
table = Table(Person.objects.all())

a_tag = table.rows[0].get_cell('first_name')
assert 'href="{}"'.format(reverse('person', args=(person.pk, ))) in a_tag
assert 'style="color: red;"' in a_tag
assert person.first_name in a_tag
self.assertIn('href="{}"'.format(reverse('person', args=(person.pk, ))), a_tag)
self.assertIn('style="color: red;"', a_tag)
self.assertIn(person.first_name, a_tag)

html = table.as_html(build_request())

td_tag_1 = '<td style="background-color: #ddd;" class="first_name">'
td_tag_2 = '<td class="first_name" style="background-color: #ddd;">'

assert td_tag_1 in html or td_tag_2 in html
self.assertIn('<td style="background-color: #ddd;">', html)

def test_defaults(self):
class Table(tables.Table):
Expand Down
Loading

0 comments on commit 0bb23fa

Please sign in to comment.