Summary

(WIP) Diff Comment Reply Pop Up Dialog

Review Request #10315 — Created Nov. 8, 2018 and updated Dec. 14, 2018, 11:30 p.m.

Information

Owner

cdkushni

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

Commit

e90a986...

Reviewers

Groups

reviewboard, students

People

Description

Set up the view class for handling the popup dialog.

Need to remove old tooltip popup.

Handling basic popup functionality.

Handles comment selection from commentlist.

Selecting a comment will expand it to fill the dialog.

Replying in the dialog box (WIP) - Text field with action

buttons implemented, needs styling and reply posting.

Have issuebar inserted into comment when expanded.

Need to query user data from a comment to get avatars.

UI Design Aesthetics (WIP).

Testing Done

Issues

Description	From	Last Updated
There are a couple of places in your code with trailing whitespace	bolariinwa	Nov. 17, 2018, 7:23 p.m.
Col: 21 Expected '===' and instead saw '=='.	reviewbot	Nov. 17, 2018, 12:21 p.m.
Col: 7 'CommentTitleBarView' was used before it was defined.	reviewbot	Nov. 30, 2018, 12:01 p.m.
Col: 11 'test' is defined but never used.	reviewbot	Nov. 30, 2018, 12:01 p.m.
Col: 144 Missing semicolon.	reviewbot	Nov. 30, 2018, 12:01 p.m.
Col: 34 Missing semicolon.	reviewbot	Nov. 30, 2018, 12:11 p.m.
Col: 11 'urls' is defined but never used.	reviewbot	Nov. 30, 2018, 12:10 p.m.
Col: 7 'CommentTitleBarView' was used before it was defined.	reviewbot	Nov. 30, 2018, 12:02 p.m.
Col: 11 'timeStamp' is defined but never used.	reviewbot	Nov. 30, 2018, 12:10 p.m.
Col: 11 'test' is defined but never used.	reviewbot	Nov. 30, 2018, 12:09 p.m.
Col: 69 Missing semicolon.	reviewbot	Nov. 30, 2018, 12:09 p.m.
Since this is an ES6 file you should be able to put a trailing comma after '_onMouseLeave'	Malcolm	Dec. 2, 2018, 8:32 a.m.
There seems to be some extra white space here. In addition to that on line 57 and 58, the ending …	Malcolm	Dec. 2, 2018, 8:32 a.m.
Col: 11 'urls' is defined but never used.	reviewbot	Dec. 14, 2018, 11:31 p.m.
Col: 7 'CommentTitleBarView' was used before it was defined.	reviewbot	Dec. 14, 2018, 11:31 p.m.
Col: 11 'timeStamp' is defined but never used.	reviewbot	Dec. 14, 2018, 11:31 p.m.
Col: 26 ['viewEl'] is better written in dot notation.	reviewbot	Dec. 14, 2018, 11:30 p.m.
Col: 15 'reviewRequestEditor' is defined but never used.	reviewbot	Dec. 14, 2018, 11:30 p.m.
Col: 7 'ReplyTextFieldView' is defined but never used.	reviewbot	Dec. 14, 2018, 11:30 p.m.
Col: 11 'urls' is defined but never used.	reviewbot	Dec. 14, 2018, 11:30 p.m.
Col: 16 Duplicate key 'comment'.	reviewbot	Dec. 14, 2018, 11:30 p.m.
Col: 38 ['diffCommentsData'] is better written in dot notation.	reviewbot	Dec. 14, 2018, 11:30 p.m.
Col: 38 ['rView'] is better written in dot notation.	reviewbot	Dec. 14, 2018, 11:30 p.m.
Col: 7 'CommentTitleBarView' was used before it was defined.	reviewbot	Dec. 14, 2018, 11:30 p.m.
Col: 15 'commentIssueManager' is defined but never used.	reviewbot	Dec. 14, 2018, 11:30 p.m.
Col: 15 'interactive' is defined but never used.	reviewbot	Dec. 14, 2018, 11:30 p.m.
Col: 15 'commentID' is defined but never used.	reviewbot	Dec. 14, 2018, 11:30 p.m.

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 21
 Expected '===' and instead saw '=='.
```

Description:

~		Started by setting up the view class for handling the popup dialog.
~		Working on methods of handling multiple comment popup dialogs.
	~	Set up the view class for handling the popup dialog.
	~	Handling basic popup functionality(WIP).
	+	Handles comment selection from commentlist.
	+	Selecting a comment will expand it to fill the dialog.
	+	Replying in the dialog box (WIP).
	+	UI Design Aesthetics (WIP).

Commit:

903f486f4981cb63e74c157f0fe383ab8853e611

6b180fb93d3c6aa45803549283f51a4bef08bb3c

Diff:

Revision 2 (+1047 -3)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/views/abstractCommentBlockView.es6.js
	reviewboard/static/rb/js/views/abstractReviewableView.es6.js
	reviewboard/static/rb/js/views/replyCommentDialogView.es6.js

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 7
 'CommentTitleBarView' was used before it was defined.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 11
 'test' is defined but never used.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 144
 Missing semicolon.
```

Fix it!

An issue was opened. Show all issues

There are a couple of places in your code with trailing whitespace

Description:

		Set up the view class for handling the popup dialog.
~		Handling basic popup functionality(WIP).
	~	Need to remove old tooltip popup.
	+	Handling basic popup functionality.
		Handles comment selection from commentlist.
		Selecting a comment will expand it to fill the dialog.
~		Replying in the dialog box (WIP).
	~	Replying in the dialog box (WIP) - Text field with action
	+	buttons implemented, needs styling and reply posting.
	+	Have issuebar inserted into comment when expanded.
	+	Need to query user data from a comment to get avatars.
		UI Design Aesthetics (WIP).

Commit:

6b180fb93d3c6aa45803549283f51a4bef08bb3c

c7e59534a6144bfd7fb920aab304eb992bd0045d

Diff:

Revision 3 (+1180 -3)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/views/abstractCommentBlockView.es6.js
	reviewboard/static/rb/js/views/abstractReviewableView.es6.js
	reviewboard/static/rb/js/views/replyCommentDialogView.es6.js

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 3)
The issue has been dropped. Show all issues
```
Col: 34
 Missing semicolon.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 3)
The issue has been dropped. Show all issues
```
Col: 11
 'urls' is defined but never used.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 3)
An issue was opened. Show all issues
```
Col: 7
 'CommentTitleBarView' was used before it was defined.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 3)
The issue has been dropped. Show all issues
```
Col: 11
 'timeStamp' is defined but never used.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 11
 'test' is defined but never used.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 69
 Missing semicolon.
```

Commit:

c7e59534a6144bfd7fb920aab304eb992bd0045d

18019b970cad8ca75c81812bce38529fb44cdeca

Diff:

Revision 4 (+1175 -3)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/views/abstractCommentBlockView.es6.js
	reviewboard/static/rb/js/views/abstractReviewableView.es6.js
	reviewboard/static/rb/js/views/replyCommentDialogView.es6.js

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 4)
The issue has been dropped. Show all issues
```
Col: 11
 'urls' is defined but never used.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 4)
The issue has been dropped. Show all issues
```
Col: 7
 'CommentTitleBarView' was used before it was defined.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 4)
The issue has been dropped. Show all issues
```
Col: 11
 'timeStamp' is defined but never used.
```

Fix it!

reviewboard/static/rb/js/views/abstractCommentBlockView.es6.js (Diff revision 4)

An issue was opened. Show all issues

Since this is an ES6 file you should be able to put a trailing comma after '_onMouseLeave'

reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 4)

An issue was opened. Show all issues

There seems to be some extra white space here. In addition to that on line 57 and 58, the ending > for the <a> tag could be placed on the same line as the a tag itself. Additionally the closing </a> tag can be placed on a newline with the inner <input> tag being indented within the scope of the <a> section. For example look at what you did on lines 59-61 as it looks a bit nicer.

Commit:

18019b970cad8ca75c81812bce38529fb44cdeca

e90a986c6822f67158610d88f991bd19ab549d6b

Diff:

Revision 5 (+2128 -12)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/reviewRequestPage/models/dlgReplyEditorModel.es6.js
	reviewboard/static/rb/js/reviewRequestPage/views/dlgReplyEditorView.es6.js
	reviewboard/static/rb/js/reviewRequestPage/views/reviewEntryView.es6.js
	reviewboard/static/rb/js/views/abstractCommentBlockView.es6.js
	reviewboard/static/rb/js/views/abstractReviewableView.es6.js
	reviewboard/static/rb/js/views/dlgReviewView.es6.js
	2 more

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/reviewRequestPage/views/dlgReplyEditorView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 26
 ['viewEl'] is better written in dot notation.
```
reviewboard/static/rb/js/views/dlgReviewView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 15
 'reviewRequestEditor' is defined but never used.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 7
 'ReplyTextFieldView' is defined but never used.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 11
 'urls' is defined but never used.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 16
 Duplicate key 'comment'.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 38
 ['diffCommentsData'] is better written in dot notation.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 38
 ['rView'] is better written in dot notation.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 7
 'CommentTitleBarView' was used before it was defined.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 15
 'commentIssueManager' is defined but never used.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 15
 'interactive' is defined but never used.
```
reviewboard/static/rb/js/views/replyCommentDialogView.es6.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 15
 'commentID' is defined but never used.
```