Summary

Fix various issues with configforms table views.

Review Request #6050 — Created July 3, 2014 and submitted July 5, 2014, 1:24 a.m.

Information

Owner

chipx86

Repository

Djblets

Branch

release-0.8.x

Bugs

Depends On

Commit

d4865dd...

Reviewers

Groups

djblets

People

Description

There were a few of bad assumptions made when using

TableView/TableItemView for configform item lists.
One was that there was already a <tbody> defined. This was the case in

the few places where we used it before, but isn't not always going to be

the case. We now add one if it's not already there.
The second bad assumption was that actions go on the element. In the

case of TableItemView, this is a <tr>. Now, ListItemView/TableItemView

subclasses can define a function to return where the actions should be

added. For TableItemView, this defaults to the last cell in the row.
The third bad assumption was that, by default, ListItemView would be

used if not specified otherwise, even for TableView. Now, ListView

subclasses can define their default.
Along with this, the template for TableItemView has been cleaned up to

remove extra whitespace, which matches a change made recently for

ListItemView.
Unit tests were added for all this. Some existing tests had to be

cleaned up, though, since they polluted prototype-level data.

Testing Done

Unit tests pass.

The issues I was hitting in some new code using TableItemView and TableView
were fixed.

Issues

Description	From	Last Updated
This indentation is kind of bizarre. Is it crucial to avoid extra whitespace at the beginning? This should be 1-space …	david	July 3, 2014, 6:23 p.m.

Tool: Pyflakes
Processed Files:
    djblets/settings.py

Ignored Files:
    djblets/static/djblets/js/configForms/views/listView.js
    djblets/static/djblets/js/configForms/views/tests/tableItemViewTests.js
    djblets/static/djblets/js/configForms/views/tests/listItemViewTests.js
    djblets/static/djblets/js/configForms/views/listItemView.js
    djblets/static/djblets/js/configForms/views/tableItemView.js
    djblets/static/djblets/js/configForms/views/tableView.js
    djblets/static/djblets/js/configForms/views/tests/tableViewTests.js



Tool: PEP8 Style Checker
Processed Files:
    djblets/settings.py

Ignored Files:
    djblets/static/djblets/js/configForms/views/listView.js
    djblets/static/djblets/js/configForms/views/tests/tableItemViewTests.js
    djblets/static/djblets/js/configForms/views/tests/listItemViewTests.js
    djblets/static/djblets/js/configForms/views/listItemView.js
    djblets/static/djblets/js/configForms/views/tableItemView.js
    djblets/static/djblets/js/configForms/views/tableView.js
    djblets/static/djblets/js/configForms/views/tests/tableViewTests.js

djblets/static/djblets/js/configForms/views/tableItemView.js (Diff revision 1)

The issue has been resolved. Show all issues

This indentation is kind of bizarre. Is it crucial to avoid extra whitespace at the beginning?

This should be 1-space indented no matter what.

chipx86 July 3, 2014, 3:17 p.m.

There were display issues I fixed in the past with ListItemView before by removing the excess whitespace. We do want this, as that excess whitespace will be preserved and mess with any alignment of text in the display. We should have this fixed by default for TableItemView, to match.

I can change this so there's no indentation whatsoever, but I don't want to add the single spaces within the strings.

chipx86 July 3, 2014, 6:19 p.m.

Also oops, I realize now that I did 2 spaces. That didn't click earlier.

Change Summary:

Removed the indentation altogether for the TableItemView template.

Commit:

9fbda29109ac982db617f02fab613eb0e5b71000

d4865dd3e7a18e2276111bd1fb7f3b1b585a1370

Diff:

Revision 2 (+220 -10)

Show changes

	djblets/settings.py
	djblets/static/djblets/js/configForms/views/listItemView.js
	djblets/static/djblets/js/configForms/views/listView.js
	djblets/static/djblets/js/configForms/views/tableItemView.js
	djblets/static/djblets/js/configForms/views/tableView.js
	djblets/static/djblets/js/configForms/views/tests/listItemViewTests.js
	djblets/static/djblets/js/configForms/views/tests/tableItemViewTests.js
	djblets/static/djblets/js/configForms/views/tests/tableViewTests.js

Tool: PEP8 Style Checker
Processed Files:
    djblets/settings.py

Ignored Files:
    djblets/static/djblets/js/configForms/views/listView.js
    djblets/static/djblets/js/configForms/views/tests/tableItemViewTests.js
    djblets/static/djblets/js/configForms/views/tests/listItemViewTests.js
    djblets/static/djblets/js/configForms/views/listItemView.js
    djblets/static/djblets/js/configForms/views/tableItemView.js
    djblets/static/djblets/js/configForms/views/tableView.js
    djblets/static/djblets/js/configForms/views/tests/tableViewTests.js



Tool: Pyflakes
Processed Files:
    djblets/settings.py

Ignored Files:
    djblets/static/djblets/js/configForms/views/listView.js
    djblets/static/djblets/js/configForms/views/tests/tableItemViewTests.js
    djblets/static/djblets/js/configForms/views/tests/listItemViewTests.js
    djblets/static/djblets/js/configForms/views/listItemView.js
    djblets/static/djblets/js/configForms/views/tableItemView.js
    djblets/static/djblets/js/configForms/views/tableView.js
    djblets/static/djblets/js/configForms/views/tests/tableViewTests.js

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-0.8.x (ecdbe83)