This change has been marked as completed.

Describe the completed change (optional):

Pushed to release-0.10.x (f10b5ac)

Summary

Add the ListEditWidget

Review Request #8982 — Created June 1, 2017 and submitted 8 years ago

Information

Owner

brennie*

Repository

Djblets

Branch

release-0.10.x

Bugs

Depends On

~~8980~~

Blocks

8987

Reviewers

Groups

djblets

People

Description*

The ListEditWidget provides an interface for editing a delimited
list of values as a list of input fields.

Testing Done

Used this with an upcoming change for Review Board.

Files

Page-Shot-2017-6-5 Change OAuth Application Administration Review Board.png

Issues

Description	From	Last Updated
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
Col: 73 Missing semicolon.	reviewbot	8 years ago
Col: 72 Expected ')' and instead saw ';'.	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
No space between function and ()	david	8 years ago
Shouldn't have trailing commas in non-ES6 code.	david	8 years ago
, optional	david	8 years ago
, optional	david	8 years ago
You're using the variable name item in both the inner and outer loops. How about different names?	david	8 years ago
Trailing comma?	david	8 years ago
Missing Args/Returns	david	8 years ago
Trailing whitespace.	david	8 years ago
No spaces around the =	david	8 years ago
We have a default value (empty object) for options. What happens if this is constructed with that default? It looks …	david	8 years ago
Trailing comma?	david	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
Col: 68 Missing semicolon.	reviewbot	8 years ago
Col: 64 Missing semicolon.	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
W293 blank line contains whitespace	reviewbot	8 years ago
Can we change this around a little bit so we have: const $entry = $(...) .insertBefore(this._$addBtn); $entry .find(...) .on(...) .end() …	david	8 years ago
There are no open issues

flake8 failed.

JSHint failed.

flake8

djblets/forms/widgets.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```

JSHint

djblets/static/djblets/js/forms/views/listEditView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 73
 Missing semicolon.
```
djblets/static/djblets/js/forms/views/listEditView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 72
 Expected ')' and instead saw ';'.
```

Diff:

Revision 2 (+304)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/forms/templates/djblets_forms/list_edit_widget.html
	djblets/static/djblets/css/forms/list_edit.less
	djblets/static/djblets/js/forms/views/listEditView.es6.js

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/forms/widgets.py (Diff revision 2)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 2)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 2)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 2)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```

Diff:

Revision 3 (+309)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/forms/templates/djblets_forms/list_edit_widget.html
	djblets/static/djblets/css/forms/list_edit.less
	djblets/static/djblets/js/forms/views/listEditView.es6.js

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/forms/widgets.py (Diff revision 3)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 3)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 3)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 3)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```

Diff:

Revision 4 (+295)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/forms/templates/djblets_forms/list_edit_widget.html
	djblets/static/djblets/css/forms/list_edit.less
	djblets/static/djblets/js/forms/views/listEditView.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

```
Screenshots?
```
djblets/forms/templates/djblets_forms/list_edit_widget.html (Diff revision 4)
The issue has been resolved. Show all issues
```
No space between function and ()
```
djblets/forms/templates/djblets_forms/list_edit_widget.html (Diff revision 4)
The issue has been resolved. Show all issues
```
Shouldn't have trailing commas in non-ES6 code.
```
djblets/forms/widgets.py (Diff revision 4)
The issue has been resolved. Show all issues
```
, optional
```
djblets/forms/widgets.py (Diff revision 4)
The issue has been resolved. Show all issues
```
, optional
```

djblets/forms/widgets.py (Diff revision 4)

The issue has been resolved. Show all issues

You're using the variable name item in both the inner and outer loops. How about different names?

djblets/forms/widgets.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing comma?
```
djblets/forms/widgets.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Missing Args/Returns
```
djblets/static/djblets/js/forms/views/listEditView.es6.js (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```
djblets/static/djblets/js/forms/views/listEditView.es6.js (Diff revision 4)
The issue has been resolved. Show all issues
```
No spaces around the =
```

djblets/static/djblets/js/forms/views/listEditView.es6.js (Diff revision 4)

The issue has been dropped. Show all issues

We have a default value (empty object) for options. What happens if this is constructed with that default? It looks like we probably would end up with broken undefineds in the DOM.

brennie 8 years ago

This view is not intended to be used outside of the widget.

david 8 years ago

In that case, options probably shouldn't have a default?

djblets/static/djblets/js/forms/views/listEditView.es6.js (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing comma?
```

Change Summary:

Added unit tests. Fixed some bugs. Addressed David's feedback.

Diff:

Revision 5 (+469 -1)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/forms/templates/djblets_forms/list_edit_widget.html
	djblets/static/djblets/css/forms/list_edit.less
	djblets/static/djblets/js/forms/views/listEditView.es6.js
	djblets/static/djblets/js/forms/views/tests/listEditViewTests.es6.js

Added Files:

Page-Shot-2017-6-5 Change OAuth Application Administration Review Board.png

Checks run (2 failed)

flake8 failed.

JSHint failed.

flake8

djblets/forms/widgets.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```

JSHint

djblets/static/djblets/js/forms/views/tests/listEditViewTests.es6.js (Diff revision 5)
The issue has been resolved. Show all issues
```
Col: 68
 Missing semicolon.
```
djblets/static/djblets/js/forms/views/tests/listEditViewTests.es6.js (Diff revision 5)
The issue has been resolved. Show all issues
```
Col: 64
 Missing semicolon.
```

Diff:

Revision 6 (+469 -1)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/forms/templates/djblets_forms/list_edit_widget.html
	djblets/static/djblets/css/forms/list_edit.less
	djblets/static/djblets/js/forms/views/listEditView.es6.js
	djblets/static/djblets/js/forms/views/tests/listEditViewTests.es6.js

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/forms/widgets.py (Diff revision 6)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
djblets/forms/widgets.py (Diff revision 6)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```

Diff:

Revision 7 (+469 -1)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/forms/templates/djblets_forms/list_edit_widget.html
	djblets/static/djblets/css/forms/list_edit.less
	djblets/static/djblets/js/forms/views/listEditView.es6.js
	djblets/static/djblets/js/forms/views/tests/listEditViewTests.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

djblets/static/djblets/js/forms/views/listEditView.es6.js (Diff revision 7)

The issue has been resolved. Show all issues

Can we change this around a little bit so we have:

const $entry = $(...)
    .insertBefore(this._$addBtn);

$entry
    .find(...)
        .on(...)
    .end()
    .find(...)
        .on(...)
    .end();

Status:: Completed
Change Summary:: Pushed to release-0.10.x (f10b5ac)