Summary

Make double-clicking on a row in a diff create a comment.

Review Request #6881 — Created Feb. 1, 2015 and submitted Feb. 9, 2015, 8:09 p.m.

Information

Owner

mike_conley

Repository

Review Board

Branch

master

Bugs

Depends On

Commit

900f7e2...

Reviewers

Groups

reviewboard

People

Description

Users of GitHub or Bugzilla Splinter might be more used to creating
comments on diffs by double-clicking on the line of code they want
to comment on. This patch adds that capability.

Testing Done

In a diff, I double-clicked on both sides of a diff, both within syntax-highlighted regions, and outside of syntax-highlighted regions. The comment editor dialog opened in all cases, as expected.

Issues

Description	From	Last Updated
Remove trailing comma	mike_conley	Feb. 6, 2015, 9:31 p.m.
Add documentation here.	mike_conley	Feb. 6, 2015, 9:30 p.m.
Is it worth adding a separate class for the code cells to check the ancestors for?	mike_conley	Feb. 7, 2015, 7:08 a.m.
Add documentation	mike_conley	Feb. 6, 2015, 9:31 p.m.
Add documentation	mike_conley	Feb. 6, 2015, 9:31 p.m.
Blank line between these.	brennie	Feb. 6, 2015, 9:32 p.m.
There should be a single var statement at the top of the function.	brennie	Feb. 6, 2015, 9:32 p.m.
Can you break this off into a summary/description?	chipx86	Feb. 9, 2015, 7:58 p.m.

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/views/textCommentRowSelector.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/views/textCommentRowSelector.js

reviewboard/static/rb/js/views/textCommentRowSelector.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Add documentation here.
```

reviewboard/static/rb/js/views/textCommentRowSelector.js (Diff revision 1)

The issue has been resolved. Show all issues

Is it worth adding a separate class for the code cells to check the ancestors for?

mike_conley Feb. 6, 2015, 9:32 p.m.
```
We'll see what the others think.
```

mike_conley Feb. 7, 2015, 7:08 a.m.

I've simplified this by checking ancestors for td.l and td.r.

reviewboard/static/rb/js/views/textCommentRowSelector.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Add documentation
```
reviewboard/static/rb/js/views/textCommentRowSelector.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Add documentation
```

reviewboard/static/rb/js/views/textCommentRowSelector.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Remove trailing comma
```

reviewboard/static/rb/js/views/textCommentRowSelector.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line between these.
```
reviewboard/static/rb/js/views/textCommentRowSelector.js (Diff revision 1)
The issue has been resolved. Show all issues
```
There should be a single var statement at the top of the function.
```

Summary:

-	[WIP] Make double-clicking on a row in a diff create a comment.
+	Make double-clicking on a row in a diff create a comment.

Description:

		Users of GitHub or Bugzilla Splinter might be more used to creating
		comments on diffs by double-clicking on the line of code they want
		to comment on. This patch adds that capability.
-
-		Question: Do we want to detect the first time the user does this, and
-		have the comment balloon offer up a link to documentation on how to
-		select a range of lines to comment on?

Commit:

-	8d8aa218b7aed4ae7b30144b81c80cee701dc356
+	1376723cbea71f39aa24782bf46d5c0318d1a2b9

Diff:

Revision 2 (+42)

Show changes

reviewboard/static/rb/js/views/textCommentRowSelector.js

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/views/textCommentRowSelector.js



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/views/textCommentRowSelector.js

Ship it!

```
Ship It!
```

I haven't looked at the code yet, but does this cover bug 3437?

mike_conley Feb. 7, 2015, 5:52 p.m.

Not intentionally, no. Though I can't seem to reproduce 3437 with or without my patch.

david Feb. 9, 2015, 5:31 p.m.

I think you should close that bug once this is pushed.

Ship it!

```
Ship It!
```

Ship it!

reviewboard/static/rb/js/views/textCommentRowSelector.js (Diff revision 2)
The issue has been resolved. Show all issues
```
Can you break this off into a summary/description?
```

Testing Done:

~		Just local testing at this point. In a diff, I double-clicked on both sides of a diff, both within syntax-highlighted regions, and outside of syntax-highlighted regions. The comment editor dialog opened in all cases, as expected.
	~	In a diff, I double-clicked on both sides of a diff, both within syntax-highlighted regions, and outside of syntax-highlighted regions. The comment editor dialog opened in all cases, as expected.

Commit:

-	1376723cbea71f39aa24782bf46d5c0318d1a2b9
+	900f7e2891b1f3f8add75b3e3eb98435082c10f9

Diff:

Revision 3 (+44)

Show changes

reviewboard/static/rb/js/views/textCommentRowSelector.js

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/views/textCommentRowSelector.js



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/views/textCommentRowSelector.js

Status: Closed (submitted)

Change Summary:

Pushed to master (90f3011)