Summary

Add smarter objects selector widget into Djblets

Review Request #10214 — Created Oct. 10, 2018 and submitted Feb. 1, 2019, 3:56 p.m.

Information

Owner

skaefer143

Repository

Djblets

Branch

master

Bugs

Depends On

Blocks

10215

Commit

25eac0d...

Reviewers

Groups

djblets, students

People

Description

This change adds a smarter objects selector widget from Review Board 4.0
into Djblets. This is a form field widget and Backbone view, which must
be extended to be useful.

See /r/10215.

Testing Done

I ran all tests with ./tests/runtests.py. No errors were thrown.

With the /r/10215 Review Board patch, I navigated to
admin/db/scmtools/repository/add/. Next to "Users with access:", the
smarter user selector widget appeared. This widget extends the smarter
objects selector widget.

Issues

Description	From	Last Updated
Remove period from Summary.	gojeffcho	Nov. 28, 2018, 6:37 p.m.
Blank line between these.	brennie	Oct. 12, 2018, 9:38 a.m.
You need to move this template into Djblets as well. If anyone were to use this outside of Review Board, …	brennie	Oct. 17, 2018, 7:10 p.m.
E501 line too long (84 > 79 characters)	reviewbot	Oct. 12, 2018, 9:38 a.m.
E303 too many blank lines (2)	reviewbot	Oct. 12, 2018, 9:38 a.m.
E122 continuation line missing indentation or outdented	reviewbot	Oct. 12, 2018, 9:38 a.m.
This needs to be Djblets. not RB..	brennie	Oct. 19, 2018, noon
"This is to be used" sounds a bit awkward - if it must be used, I would specify that, or …	gojeffcho	Nov. 28, 2018, 6:44 p.m.
This comment is really confusing to me, especially the part that says "but the siteconfig field template special cases when …	shoven	Nov. 28, 2018, 8:21 p.m.
.related-object-selector might be a more clear class name (since it's for the relatedObjectSelectorView)?	shoven	Nov. 28, 2018, 7:34 p.m.
Could also use _.each($items, function($item) {...}); or $items.forEach(function($item) {...}); rather than a for loop (I typically subscribe to airbnb's Javascript …	shoven	Nov. 28, 2018, 8:12 p.m.
Typo: initalize --> initialize	ilaw	Dec. 7, 2018, 12:34 p.m.
Capitalization is off here. "size" should be lower case, and "Related" should be upper.	david	Dec. 7, 2018, 12:34 p.m.
Col: 38 Expected '{' and instead saw 'return'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 25 Expected '{' and instead saw 'return'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 29 Expected '{' and instead saw 'return'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 28 Expected '{' and instead saw 'score'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 37 Expected '{' and instead saw 'return'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 36 Expected '{' and instead saw 'return'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 32 'a_value' is defined but never used.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 41 'b_value' is defined but never used.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 33 Expected '{' and instead saw 'return'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 40 Expected '{' and instead saw 'return'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 56 Expected '{' and instead saw 'options'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 52 Expected '{' and instead saw 'options'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 64 Expected '{' and instead saw 'options'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 26 'value' is defined but never used.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 48 'calculateScore' is defined but never used.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 22 Expected '{' and instead saw 'search'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 9 'cmp' was used before it was defined.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 20 Expected '{' and instead saw 'return'.	reviewbot	Dec. 14, 2018, 3:26 p.m.
Col: 20 Expected '{' and instead saw 'return'.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 9 'extend' was used before it was defined.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 26 Expected '{' and instead saw 'continue'.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 9 'trim' was used before it was defined.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 9 'escape_regex' was used before it was defined.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 9 'is_array' was used before it was defined.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 9 'DIACRITICS' was used before it was defined.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 9 'asciifold' was used before it was defined.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 9 'utils' was used before it was defined.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 25 'endbit' is defined but never used.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 13 'key' is defined but never used.	reviewbot	Dec. 14, 2018, 3:27 p.m.
Col: 17 'timeout_blur' is defined but never used.	reviewbot	Dec. 14, 2018, 3:27 p.m.

flake8 passed.

JSHint passed.

djblets/forms/widgets.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line between these.
```

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

This is used inside of Django to extract the value from the data and files dictionaries based on the name of the field (name).

See the docs.

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

The issue has been resolved. Show all issues

You need to move this template into Djblets as well. If anyone were to use this outside of Review Board, they would get a template not found error.

Change Summary:

I moved the JS Widgets into Djblets, and added a new initialization argument called using_avatars. This is because we can't assume in Djblets that the widget is using an avatar service. 

I think this works for moving templates and JS files into Djblets, but let me know if I'm wrong.

Commit:

a14bf9f21bfea84a84f52667c1081f2c3b6afead

250c8898785a7c90a6067e09db05573a4873e8a1

Diff:

Revision 2 (+560)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/forms/templates/djblets_forms/admin/related_user_widget.html
	djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js
	djblets/static/djblets/js/admin/views/relatedUserSelectorView.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
```
E501 line too long (84 > 79 characters)
```
djblets/forms/widgets.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E303 too many blank lines (2)
```
djblets/forms/widgets.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E122 continuation line missing indentation or outdented
```

Change Summary:

Fixed some style issues.

Commit:

250c8898785a7c90a6067e09db05573a4873e8a1

7a98d1c8363c8cf2b122b472e3acafd6e1cd04a2

Diff:

Revision 3 (+560)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/forms/templates/djblets_forms/admin/related_user_widget.html
	djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js
	djblets/static/djblets/js/admin/views/relatedUserSelectorView.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Moved relatedUserWidget and relevant files back into Review Board repository. There are problems with the order the JS files are run, though, so widget will not currently render.

Description:

		[WIP] Add smarter objects selector widget into Djblets.

		This change adds a smarter objects selector widget from
~		Review Board 4.0 into Djblets.
	~	Review Board 4.0 into Djblets. See /r/10215.

Commit:

7a98d1c8363c8cf2b122b472e3acafd6e1cd04a2

0b7202b89ca5f26a2164f5a11ae523f961f02ebc

Diff:

Revision 4 (+373 -1)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/static/djblets/css/defs.less
	djblets/static/djblets/css/ui/related-object-selector.less
	djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Widget now displays correctly by changing the name in staticbundles.py.

Summary:

[WIP] Add smarter objects selector widget into Djblets.

Add smarter objects selector widget into Djblets.

Description:

-		[WIP] Add smarter objects selector widget into Djblets.
-
		This change adds a smarter objects selector widget from
~		Review Board 4.0 into Djblets. See /r/10215.
	~	Review Board 4.0 into Djblets. This is a general widget
	+	and Backbone view, which must be extended to be useful.
	+
	+	See /r/10215.

Testing Done:

~		[Work in Progress]
~		I ran all tests with `./tests/runtests.py`. No errors were thrown.
	~	I ran all tests with `./tests/runtests.py`. No errors were thrown.
	~
	+	With the `/r/10215` Review Board patch, I navigated to
	+	`admin/db/scmtools/repository/add/`. Next to "Users with access:", the
	+	smarter user selector widget appeared. This widget extends the smarter
	+	objects selector widget.

Commit:

0b7202b89ca5f26a2164f5a11ae523f961f02ebc

a18614f119e87dd9f19b7f558e1b79663d955103

Diff:

Revision 5 (+376 -1)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/static/djblets/css/defs.less
	djblets/static/djblets/css/ui/related-object-selector.less
	djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Description:: ~
This change adds a smarter objects selector widget from
~ Review Board 4.0 into Djblets. This is a general widget
~ and Backbone view, which must be extended to be useful.
~
This change adds a smarter objects selector widget from Review Board 4.0
~ into Djblets. This is a form field widget and Backbone view, which must
~ be extended to be useful.

See /r/10215.

djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js (Diff revision 5)
The issue has been resolved. Show all issues
```
This needs to be Djblets. not RB..
```

Change Summary:

Defines the RelatedObjectSelectorView with Djblets., not RB..

Commit:

a18614f119e87dd9f19b7f558e1b79663d955103

f2eb68434da59308852eb2b31f3731da5e0264c1

Diff:

Revision 6 (+376 -1)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/static/djblets/css/defs.less
	djblets/static/djblets/css/ui/related-object-selector.less
	djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commit:

f2eb68434da59308852eb2b31f3731da5e0264c1

5440d0efede1158efcba185d70b0147decebefb7

Diff:

Revision 7 (+376)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/static/djblets/css/defs.less
	djblets/static/djblets/css/ui/related-object-selector.less
	djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues
```
Remove period from Summary.
```

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

The issue has been resolved. Show all issues

"This is to be used" sounds a bit awkward - if it must be used, I would specify that, or if it is recommended but permissible, should be used, maybe? Or is it potentially better to change it to the imperative voice and say "Use this with..."?

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

The issue has been resolved. Show all issues

This comment is really confusing to me, especially the part that says "but the siteconfig field template special cases when is_hidden is True" (it seems like it's missing a verb perhaps?). If you could reword this comment a bit to make it a bit more clear that would be great!

djblets/static/djblets/css/ui/related-object-selector.less (Diff revision 7)
The issue has been resolved. Show all issues
```
.related-object-selector might be a more clear class name (since it's for the relatedObjectSelectorView)?
```

djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js (Diff revision 7)

The issue has been dropped. Show all issues

Could also use _.each($items, function($item) {...}); or $items.forEach(function($item) {...}); rather than a for loop (I typically subscribe to airbnb's Javascript guidelines which recommends something like this over for loops but maybe a mentor can weigh in on their preference for Javascript style).

skaefer143 Nov. 28, 2018, 8:22 p.m.

I didn't actually write this code, it came over from a previous implementation. I think this code has to do with placing items in the list if the text string is a different length in a different locale. I tried implementing your code, but it didn't work for me. I was getting some weird glitches, with the widget unable to select and display multiple objects. I might just be doing something wrong also. I'm going to leave the code as is. I did learn something new today though!

Change Summary:

Fixed some comments, as well as changed some html classes to be more descriptive.

Summary:

Add smarter objects selector widget into Djblets.

Add smarter objects selector widget into Djblets

Commit:

5440d0efede1158efcba185d70b0147decebefb7

9ab8887e7ced52b29be4b9d763babd2aeb43689e

Diff:

Revision 8 (+376)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/static/djblets/css/defs.less
	djblets/static/djblets/css/ui/related-object-selector.less
	djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

djblets/static/djblets/css/defs.less (Diff revision 8)
The issue has been resolved. Show all issues
```
Capitalization is off here. "size" should be lower case, and "Related" should be upper.
```

djblets/forms/widgets.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Typo: initalize --> initialize
```
djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js (Diff revision 8)
```
Maybe change wording to "to make necessary API requests"
```

Change Summary:

Fix some comment grammar.

Commit:

9ab8887e7ced52b29be4b9d763babd2aeb43689e

391c5cd2b4c6652da2aa3ed2a13e79d861482dca

Diff:

Revision 9 (+375)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/static/djblets/css/defs.less
	djblets/static/djblets/css/ui/related-object-selector.less
	djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

fix the placeholder widget string from being cut off by selectize

Commit:

391c5cd2b4c6652da2aa3ed2a13e79d861482dca

90464dea73be37de19dd8df0ba0358183a33d301

Diff:

Revision 10 (+381)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/static/djblets/css/defs.less
	djblets/static/djblets/css/ui/related-object-selector.less
	djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

move selectize.js out of reviewboard, into djblets.

Commit:

90464dea73be37de19dd8df0ba0358183a33d301

25eac0de866363b0725ee91d1a5d923a3f7f0c5a

Diff:

Revision 11 (+4442)

Show changes

	djblets/staticbundles.py
	djblets/forms/widgets.py
	djblets/static/djblets/css/defs.less
	djblets/static/djblets/css/selectize.default-0.12.1.css
	djblets/static/djblets/css/ui/related-object-selector.less
	djblets/static/djblets/js/README.selectize
	djblets/static/djblets/js/selectize-0.12.1.js
	djblets/static/djblets/js/admin/views/relatedObjectSelectorView.es6.js

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

Warning: Showing 30 of 51 failures.

skaefer143 Dec. 14, 2018, 3:26 p.m.

These are all failures from selectize-0.12.1.js, so I'm going to drop them.

djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 38
 Expected '{' and instead saw 'return'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 25
 Expected '{' and instead saw 'return'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 29
 Expected '{' and instead saw 'return'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 28
 Expected '{' and instead saw 'score'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 37
 Expected '{' and instead saw 'return'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 36
 Expected '{' and instead saw 'return'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 32
 'a_value' is defined but never used.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 41
 'b_value' is defined but never used.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 33
 Expected '{' and instead saw 'return'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 40
 Expected '{' and instead saw 'return'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 56
 Expected '{' and instead saw 'options'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 52
 Expected '{' and instead saw 'options'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 64
 Expected '{' and instead saw 'options'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 26
 'value' is defined but never used.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 48
 'calculateScore' is defined but never used.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 22
 Expected '{' and instead saw 'search'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 9
 'cmp' was used before it was defined.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 20
 Expected '{' and instead saw 'return'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 20
 Expected '{' and instead saw 'return'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 9
 'extend' was used before it was defined.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 26
 Expected '{' and instead saw 'continue'.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 9
 'trim' was used before it was defined.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 9
 'escape_regex' was used before it was defined.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 9
 'is_array' was used before it was defined.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 9
 'DIACRITICS' was used before it was defined.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 9
 'asciifold' was used before it was defined.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 9
 'utils' was used before it was defined.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 25
 'endbit' is defined but never used.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 13
 'key' is defined but never used.
```
djblets/static/djblets/js/selectize-0.12.1.js (Diff revision 11)
The issue has been dropped. Show all issues
```
Col: 17
 'timeout_blur' is defined but never used.
```

Ship it!

Going to make some small compat updates and land this. Thanks!

Status:: Completed
Change Summary:: Pushed to release-2.0.x (17b1668)