Summary

Redesign the issue summary table UI to speed up the process of addressing issues.

Review Request #6997 — Created Feb. 28, 2015 and discarded March 31, 2016, 3:02 p.m.

Information

Owner

terfan

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard, students

People

Description

Redesign the UI to make the issue summary table more useful, by:

Making issue descriptions expand so that full content is viewable on click. This change allows the user to conveniently see full issue descriptions in-table, without having to click the description and be redirected to the comment itself.

A new div in review_issue_summary_table.html contains full comment text and is initially hidden.
The CSS class, 'issue-summary-collapsed,' is applied to this hidden div when the summary is collapsed. JS methods, onPreviewClicked and onFullClicked handle user clicks.

Adding buttons to fix/drop issues in the issue summary table.

Each entry in the issue summary table now has a set of Fix and Drop buttons, visible when expanded, from which you can address the issue and the status of the issue will update everywhere. You can also continue to use the previously existing buttons in the review to fix, drop, or reopen, and the table will be updated accordingly.

Testing Done

Manual testing (expandable descriptions):
- Added long (past the 20 word cutoff mark) comments to a review request.
- Able to click the description in the issue summary table to see the full comment text.
- If the full text is being displayed, able to switch back to the 20 word version upon click.
(buttons):
- Clicking the Fix or Drop button when a description is expanded fades the issue out of the table and updates its status at the actual place in the review. It stays upon refreshing the page.

Added new commentIssueBar unit tests and ran js-tests. All passed.

Files

Issues

Description	From	Last Updated
There's three elements on here (the text, the buttons, and the See Comment), and none of them align. We should …	chipx86	April 13, 2015, 12:53 p.m.
No blank line here.	brennie	March 13, 2015, 3:48 p.m.
Trailing whitespace.	brennie	March 13, 2015, 3:48 p.m.
Trailing whitespace.	brennie	March 13, 2015, 3:48 p.m.
Should begin with a capital	brennie	March 13, 2015, 3:53 p.m.
Use single quotes.	brennie	March 13, 2015, 3:48 p.m.
Use a multiline comment.	brennie	March 13, 2015, 3:50 p.m.
Trailing whitespace.	brennie	March 13, 2015, 3:51 p.m.
Use single quotes. Here and elsewhere.	brennie	March 13, 2015, 3:51 p.m.
Space :D	SU Sunxperous	March 16, 2015, 2:39 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 16, 2015, 3:01 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 16, 2015, 3:01 p.m.
Make sure your docstrings follow the proper format: """One-line summary.""" or: """One-line summary. Multi-line description. """	chipx86	April 13, 2015, 2:49 p.m.
No need for the None parameter. It's implied.	chipx86	April 13, 2015, 2:49 p.m.
Everything in here is alphabetical but overflow/margin. Also, for margin, use 0 instead of 0px.	chipx86	April 13, 2015, 2:50 p.m.
Best to use either percentages, or to have a constant defined along with the other constant definitions, so that it's …	chipx86	April 14, 2015, 10:41 a.m.
A useful JavaScriptism is to do: !!this.options.inTable. That will keep true as true, but turn undefined, null, and false into …	chipx86	April 14, 2015, 10:28 a.m.
Blank line between these.	chipx86	April 13, 2015, 2:50 p.m.
The indentation needs to be 4 spaces, not 2.	chipx86	April 13, 2015, 2:51 p.m.
While here, can you remove the * in this line?	chipx86	April 13, 2015, 2:51 p.m.
Can you sort these alphabetically by selector?	chipx86	April 14, 2015, 10:44 a.m.
This needs a doc comment.	chipx86	April 16, 2015, 6:28 a.m.
Make sure to remove the trailing whitespace, here and below.	chipx86	April 13, 2015, 2:52 p.m.
Blank line between code and comments. All comments should follow sentence casing, with a trailing period.	chipx86	April 14, 2015, 8:22 a.m.
This is missing a var. It also should be called $hiddenDiv, since it's a jQuery element reference. Note that you …	chipx86	April 16, 2015, 3:45 p.m.
This is also missing a var, and needs to be named $link. These same comments all apply to the following …	chipx86	April 16, 2015, 6:51 a.m.
Best not to hard-code CSS here. Can we use class names instead? Same below where you set display: none. In …	chipx86	April 16, 2015, 8:22 a.m.
Needs a doc comment.	chipx86	April 16, 2015, 6:59 a.m.
Space after the comma.	chipx86	April 14, 2015, 8:25 a.m.
Missing a doc comment.	chipx86	April 16, 2015, 7:05 a.m.
Best to pull out $(this) into $this, so we don't have to wrap it more than once.	chipx86	April 16, 2015, 4:53 p.m.
If this shouldn't happen, we need to console.assert it.	chipx86	April 16, 2015, 7:31 a.m.
I don't believe you need to parseInt this. Can you check whether it comes back as an integer? I believe …	chipx86	April 16, 2015, 3:16 p.m.
This is really, really wordy, and hard to maintain and review. Please break it up into multiple lines where possible. …	chipx86	April 16, 2015, 7:27 a.m.
reviewtags and staticfiles are not used.	chipx86	April 14, 2015, 8:37 a.m.
Can you instead have display: none on issue-container? Also, each button label must be localized.	chipx86	April 16, 2015, 2:58 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 16, 2015, 3:01 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 16, 2015, 3:01 p.m.
Docstrings should be of the form: """Single line summary. Multi-line description. """	brennie	April 17, 2015, 7:26 a.m.
Remove trailing whitespace.	brennie	April 16, 2015, 3:49 p.m.
Method doccomments should use multi-line comment syntax, e.g /* * Hide ... */	brennie	April 16, 2015, 3:48 p.m.
Method doccomments should use multi-line comment syntax, e.g /* * Hide ... */	brennie	April 16, 2015, 3:48 p.m.
Remove trailing whitespace.	brennie	April 16, 2015, 3:46 p.m.
Method doccomments should use multi-line comment syntax, e.g /* * Create ... */	brennie	April 16, 2015, 3:48 p.m.
Can we assign $(this) to a variable (e.g., $this) and use that?	brennie	April 17, 2015, 7:24 a.m.
Same here.	brennie	April 17, 2015, 7:25 a.m.

Tool: Pyflakes
Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js

Description:

		Resolved merge conflicts in review_issue_summary_table.html (integrated markdown text and expandable summaries)

~		Adding the buttons (they don't show up/work yet)
	~	Update: buttons now show up in the right place but they're not fully functional yet (clicking fix or drop doesn't affect the actual state of the issue)

Commit:

4fe17d79ce84fb995f2ffe16fead9bdbbc8a1c43

4fe42e9abf6856a453817c492811aff015fcd761

Diff:

Revision 2 (+121 -22)

Show changes

	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/.DS_Store
	reviewboard/static/rb/js/views/commentIssueBarView.js
	reviewboard/static/rb/js/views/issueSummaryTableView.js
	reviewboard/templates/reviews/review_issue_summary_table.html
	reviewboard/templates/reviews/table_comment_issue.html

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/.DS_Store
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/.DS_Store
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js

<p>You should add <code>.DS_Store</code> to your <code>~/.gitconfig</code>. Please remove the <code>.DS_Store</code> file from the review request.</p>

terfan March 13, 2015, 6:35 p.m.

I can't find a .gitconfig file, does making a .gitignore file work? And how do I remove the file from this review request? (or will it automatically be removed when I update it with my git commit, after I've added .DS_Store to a gitignore)

Sunxperous March 15, 2015, 7:02 a.m.

You can configure a global .gitignore file: git config --global core.excludesfile /Users/yourself/.gitignore or see this StackOverflow thread. Then, I believe you will need to remove the existing .DS_Store with the command git rm --cached ./.DS_Store as it will not be removed automatically. In the future, no .DS_Store files will be added should you have the global .gitignore file with .DS_Store as an entry.

brennie March 17, 2015, 10:17 a.m.
```
Oops! I did mean /.gitignore. Sorry!
```
TE

terfan March 17, 2015, 10:36 a.m.
```
Worked. Thanks to both of you!
```

reviewboard/static/rb/css/pages/reviews.less (Diff revision 2)
The issue has been resolved. Show all issues
```
No blank line here.
```
reviewboard/static/rb/js/views/commentIssueBarView.js (Diff revision 2)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```
reviewboard/static/rb/js/views/commentIssueBarView.js (Diff revision 2)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```
reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 2)
The issue has been resolved. Show all issues
```
Should begin with a capital
```
reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 2)
The issue has been resolved. Show all issues
```
Use single quotes.
```
reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 2)
The issue has been resolved. Show all issues
```
Use a multiline comment.
```
reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 2)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```
reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 2)
The issue has been resolved. Show all issues
```
Use single quotes. Here and elsewhere.
```

Description:: +
From review request 6910 (Make descriptions in the issue summary table expandable so full comments are viewable in table on click):
+ Added a new div in review_issue_summary_table.html that contains full comment text and is initially hidden.
+ Added a new CSS class, 'issue-summary-collapsed,' to be applied to this hidden div when the summary is collapsed.
+ Added two new JS methods, onPreviewClicked and onFullClicked, for handling user clicks.
+
+
This change allows the user to conveniently see full issue descriptions in-table, without having to click the description and be redirected to the comment itself.
+
+
+

Resolved merge conflicts in review_issue_summary_table.html (integrated markdown text and expandable summaries)

Update: buttons now show up in the right place but they're not fully functional yet (clicking fix or drop doesn't affect the actual state of the issue)
Testing Done:: +
Manual testing (expandable descriptions):
+ - Added long (past the 20 word cutoff mark) comments to a review request.
+ - Able to click the description in the issue summary table to see the full comment text.
+ - If the full text is being displayed, able to switch back to the 20 word version upon click.

Commit:

4fe42e9abf6856a453817c492811aff015fcd761

017a106d34fb06edfeccf35fe5002c12002ffdd3

Diff:

Revision 3 (+121 -21)

Show changes

	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/.DS_Store
	reviewboard/static/rb/js/views/commentIssueBarView.js
	reviewboard/static/rb/js/views/issueSummaryTableView.js
	reviewboard/templates/reviews/review_issue_summary_table.html
	reviewboard/templates/reviews/table_comment_issue.html

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/.DS_Store
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/.DS_Store
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Space :D
```

Can you provide screenshots?

Also, make sure you're following our guidelines for the review request description: https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/

TE

terfan March 17, 2015, 8:40 a.m.
```
Done!
```

Summary:: [WIP] Adding buttons to fix/drop issues in the issue summary table
[WIP] Redesign the issue summary table UI to speed up the process of addressing issues.
Description:: ~
From review request 6910 (Make descriptions in the issue summary table expandable so full comments are viewable in table on click):
~
Redesign the UI to make the issue summary table more useful, by:
- Added a new div in review_issue_summary_table.html that contains full comment text and is initially hidden.
- Added a new CSS class, 'issue-summary-collapsed,' to be applied to this hidden div when the summary is collapsed.
- Added two new JS methods, onPreviewClicked and onFullClicked, for handling user clicks.

~
This change allows the user to conveniently see full issue descriptions in-table, without having to click the description and be redirected to the comment itself.
~
[Completed] Making issue descriptions expand so that full content is viewable on click. This change allows the user to conveniently see full issue descriptions in-table, without having to click the description and be redirected to the comment itself.

~
~
A new div in review_issue_summary_table.html contains full comment text and is initially hidden.
+ The CSS class, 'issue-summary-collapsed,' is applied to this hidden div when the summary is collapsed. JS methods, onPreviewClicked and onFullClicked handle user clicks.

~
Resolved merge conflicts in review_issue_summary_table.html (integrated markdown text and expandable summaries)
~
[WIP] Adding buttons to fix/drop issues in the issue summary table.

~
Update: buttons now show up in the right place but they're not fully functional yet (clicking fix or drop doesn't affect the actual state of the issue)
~
Buttons now show up in the right place but are not fully functional yet (clicking fix or drop doesn't affect the actual state of the issue).
Added Files:

Commit:

017a106d34fb06edfeccf35fe5002c12002ffdd3

4c14dab9f3d9ad2d297058969660913c0a82d2c9

Diff:

Revision 4 (+121 -21)

Show changes

	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/views/commentIssueBarView.js
	reviewboard/static/rb/js/views/issueSummaryTableView.js
	reviewboard/templates/reviews/review_issue_summary_table.html
	reviewboard/templates/reviews/table_comment_issue.html

Tool: Pyflakes
Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/commentIssueBarView.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/commentIssueBarView.js

Description:

		Redesign the UI to make the issue summary table more useful, by:

		[Completed] Making issue descriptions expand so that full content is viewable on click. This change allows the user to conveniently see full issue descriptions in-table, without having to click the description and be redirected to the comment itself.

		A new div in review_issue_summary_table.html contains full comment text and is initially hidden.
		The CSS class, 'issue-summary-collapsed,' is applied to this hidden div when the summary is collapsed. JS methods, onPreviewClicked and onFullClicked handle user clicks.

		[WIP] Adding buttons to fix/drop issues in the issue summary table.

~		Buttons now show up in the right place but are not fully functional yet (clicking fix or drop doesn't affect the actual state of the issue).
	~	Buttons are now functional (clicking fix or drop affects the actual state of the issue), but do not redisplay properly when the issue is reopened (reappear but faded out).

Testing Done:

		Manual testing (expandable descriptions):
		- Added long (past the 20 word cutoff mark) comments to a review request.
		- Able to click the description in the issue summary table to see the full comment text.
~		- If the full text is being displayed, able to switch back to the 20 word version upon click.
	~	- If the full text is being displayed, able to switch back to the 20 word version upon click.
	+	(buttons):
	+	- Clicking the Fix or Drop button when a description is expanded fades the issue out of the table and updates its status at the actual place in the review. It stays upon refreshing the page.

Commit:

4c14dab9f3d9ad2d297058969660913c0a82d2c9

6be79dba65efb2ce74bdb47d6194b00120eebe4d

Diff:

Revision 5 (+120 -21)

Show changes

	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/views/commentIssueBarView.js
	reviewboard/static/rb/js/views/issueSummaryTableView.js
	reviewboard/templates/reviews/review_issue_summary_table.html
	reviewboard/templates/reviews/table_comment_issue.html

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/commentIssueBarView.js



Tool: Pyflakes
Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/commentIssueBarView.js

Commit:

6be79dba65efb2ce74bdb47d6194b00120eebe4d

54f07a76df9a93fc6c911707a71eb54780d5167e

Diff:

Revision 6 (+146 -21)

Show changes

	reviewboard/reviews/templatetags/reviewtags.py
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/views/commentIssueBarView.js
	reviewboard/static/rb/js/views/issueSummaryTableView.js
	reviewboard/templates/reviews/review_issue_summary_table.html
	reviewboard/templates/reviews/table_comment_issue.html

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/commentIssueBarView.js



Tool: Pyflakes
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/commentIssueBarView.js

reviewboard/reviews/templatetags/reviewtags.py (Diff revision 6)
The issue has been resolved. Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

Description:

		Redesign the UI to make the issue summary table more useful, by:

		[Completed] Making issue descriptions expand so that full content is viewable on click. This change allows the user to conveniently see full issue descriptions in-table, without having to click the description and be redirected to the comment itself.

		A new div in review_issue_summary_table.html contains full comment text and is initially hidden.
		The CSS class, 'issue-summary-collapsed,' is applied to this hidden div when the summary is collapsed. JS methods, onPreviewClicked and onFullClicked handle user clicks.

~		[WIP] Adding buttons to fix/drop issues in the issue summary table.
	~	[Completed] Adding buttons to fix/drop issues in the issue summary table.

~		Buttons are now functional (clicking fix or drop affects the actual state of the issue), but do not redisplay properly when the issue is reopened (reappear but faded out).
	~	Buttons are now functional (clicking fix or drop affects the actual state of the issue regardless of whether it was pressed in the table or in the review).

Testing Done:

		Manual testing (expandable descriptions):
		- Added long (past the 20 word cutoff mark) comments to a review request.
		- Able to click the description in the issue summary table to see the full comment text.
		- If the full text is being displayed, able to switch back to the 20 word version upon click.
		(buttons):
~		- Clicking the Fix or Drop button when a description is expanded fades the issue out of the table and updates its status at the actual place in the review. It stays upon refreshing the page.
	~	- Clicking the Fix or Drop button when a description is expanded fades the issue out of the table and updates its status at the actual place in the review. It stays upon refreshing the page.
	+	Added new commentIssueBar tests and ran js-tests.

Commit:

54f07a76df9a93fc6c911707a71eb54780d5167e

fd528c901da6fd3daef55a140790b34c95f8f673

Diff:

Revision 7 (+187 -20)

Show changes

	reviewboard/reviews/templatetags/reviewtags.py
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/views/commentIssueBarView.js
	reviewboard/static/rb/js/views/issueSummaryTableView.js
	reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js
	reviewboard/templates/reviews/review_issue_summary_table.html
	reviewboard/templates/reviews/table_comment_issue.html

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js

reviewboard/reviews/templatetags/reviewtags.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

Fix it!

first issue expanded.png (Revision 1)

An issue was opened. Show all issues

There's three elements on here (the text, the buttons, and the See Comment), and none of them align. We should strive to have them align as best as possible.

terfan April 14, 2015, 8:43 a.m.

I've since fixed that issue (I think this image was just a mockup) so that the buttons and See Comment are aligned, but personally I thought it looked better with those 2 elements indented. I'll attached 2 more photos, one of how it currently looks that way, and another with all 3 elements aligned.

reviewboard/reviews/templatetags/reviewtags.py (Diff revision 7)

The issue has been resolved. Show all issues

Make sure your docstrings follow the proper format:

"""One-line summary."""

or:

"""One-line summary.

Multi-line description.
"""

reviewboard/reviews/templatetags/reviewtags.py (Diff revision 7)
The issue has been resolved. Show all issues
```
No need for the None parameter. It's implied.
```

reviewboard/static/rb/css/pages/reviews.less (Diff revision 7)

The issue has been resolved. Show all issues

Everything in here is alphabetical but overflow/margin.

Also, for margin, use 0 instead of 0px.

reviewboard/static/rb/css/pages/reviews.less (Diff revision 7)

The issue has been resolved. Show all issues

Best to use either percentages, or to have a constant defined along with the other constant definitions, so that it's easier to find and update this if we make major changes to the styling.

reviewboard/static/rb/js/views/commentIssueBarView.js (Diff revision 7)

The issue has been resolved. Show all issues

A useful JavaScriptism is to do: !!this.options.inTable. That will keep true as true, but turn undefined, null, and false into false.

reviewboard/static/rb/js/views/commentIssueBarView.js (Diff revision 7)
The issue has been resolved. Show all issues
```
Blank line between these.
```

reviewboard/static/rb/js/views/commentIssueBarView.js (Diff revision 7)

The issue has been resolved. Show all issues

The indentation needs to be 4 spaces, not 2.

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)
The issue has been resolved. Show all issues
```
While here, can you remove the * in this line?
```
reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you sort these alphabetically by selector?
```
reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)
The issue has been resolved. Show all issues
```
This needs a doc comment.
```
reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)
The issue has been resolved. Show all issues
```
Make sure to remove the trailing whitespace, here and below.
```

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)

The issue has been resolved. Show all issues

Blank line between code and comments.

All comments should follow sentence casing, with a trailing period.

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)

The issue has been resolved. Show all issues

This is missing a var. It also should be called $hiddenDiv, since it's a jQuery element reference.
Note that you must have one var statement for all variables, comma-separated, one per line, and it must be at the top of the function.
Also, is there a class name you can reference in the ancestry instead of doing parent().parent()? It'll help reduce breakages if we ever need to alter the DOM here.

terfan April 16, 2015, 8:29 a.m.

There is a class name, but since there is an element with that class name for each entry in the table, is there some way I can select the closest matching element? Having to extract the comment id and add it as a property of each of these elements seems a bit involved (but I'm happy to do it if that's the best option).

brennie April 16, 2015, 3:09 p.m.

jQuery has the .closest() selector which finds the closest ancestor element that maches. If the $el.parent().parent() element has the class .grandparent then you could do $el.closest('.grandparent').find('div.issue-full').

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)

The issue has been resolved. Show all issues

This is also missing a var, and needs to be named $link.

These same comments all apply to the following function.

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)

The issue has been resolved. Show all issues

Best not to hard-code CSS here. Can we use class names instead?
Same below where you set display: none.
In general, though, if you need to hard-code CSS, you shouldn't modify style, but rather should .css('display', 'inline-block').

terfan April 16, 2015, 8:22 a.m.

jQuery fading in and out edits the display value inline, so it overrides any classes I add.

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)
The issue has been resolved. Show all issues
```
Needs a doc comment.
```
reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)
The issue has been resolved. Show all issues
```
Space after the comma.
```
reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)
The issue has been resolved. Show all issues
```
Missing a doc comment.
```

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)

The issue has been resolved. Show all issues

Best to pull out $(this) into $this, so we don't have to wrap it more than once.

terfan April 14, 2015, 10:58 a.m.

It's not working without the parentheses, is there something else I should be doing? $(this) is also used in line 255 (original code, not added by me).

brennie April 16, 2015, 3:09 p.m.

I believe Christrian means that you should do:

var $this = $(this),
    dest = $(this).find('...'),
    ...;

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)
The issue has been resolved. Show all issues
```
If this shouldn't happen, we need to console.assert it.
```

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 7)

The issue has been dropped. Show all issues

I don't believe you need to parseInt this. Can you check whether it comes back as an integer? I believe .date() takes care of this for you.

terfan April 14, 2015, 10:53 a.m.

It comes back as a string without the parseInt, that was the bug that took me so long to figure out.

reviewboard/templates/reviews/review_issue_summary_table.html (Diff revision 7)

The issue has been resolved. Show all issues

This is really, really wordy, and hard to maintain and review. Please break it up into multiple lines where possible. You can use {% spaceless %}...{% endspaceless %} if there are issues with spaces causing problems.

Also, note that the "See comment" must be localized.

reviewboard/templates/reviews/table_comment_issue.html (Diff revision 7)
The issue has been resolved. Show all issues
```
reviewtags and staticfiles are not used.
```

reviewboard/templates/reviews/table_comment_issue.html (Diff revision 7)

The issue has been resolved. Show all issues

Can you instead have display: none on issue-container?

Also, each button label must be localized.

terfan April 14, 2015, 10:50 a.m.

Each button needs to individually have display:none because the js code uses the issues status to individually determine which one(s) to show.
I based this file off of the existing comment_issue.html, which I did not write, and I think the buttons also aren't localized there. Should I do both at the same time?

Added Files:

Commit:

fd528c901da6fd3daef55a140790b34c95f8f673

93548d5f53ab54d9d82d4d982326c99b5665260a

Diff:

Revision 8 (+203 -19)

Show changes

	reviewboard/reviews/templatetags/reviewtags.py
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/views/commentIssueBarView.js
	reviewboard/static/rb/js/views/issueSummaryTableView.js
	reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js
	reviewboard/templates/reviews/review_issue_summary_table.html
	reviewboard/templates/reviews/table_comment_issue.html

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js

reviewboard/reviews/templatetags/reviewtags.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

Commit:

93548d5f53ab54d9d82d4d982326c99b5665260a

f882f5f2ffadf7078249a9836b38138dcf009029

Diff:

Revision 9 (+197 -19)

Show changes

	reviewboard/reviews/templatetags/reviewtags.py
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/views/commentIssueBarView.js
	reviewboard/static/rb/js/views/issueSummaryTableView.js
	reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js
	reviewboard/templates/reviews/review_issue_summary_table.html
	reviewboard/templates/reviews/table_comment_issue.html

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js



Tool: Pyflakes
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js

reviewboard/reviews/templatetags/reviewtags.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

Commit:

f882f5f2ffadf7078249a9836b38138dcf009029

5b526e2420094c21427c4dd8247506de8135cacf

Diff:

Revision 10 (+198 -19)

Show changes

	reviewboard/reviews/templatetags/reviewtags.py
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/views/commentIssueBarView.js
	reviewboard/static/rb/js/views/issueSummaryTableView.js
	reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js
	reviewboard/templates/reviews/review_issue_summary_table.html
	reviewboard/templates/reviews/table_comment_issue.html

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js

reviewboard/reviews/templatetags/reviewtags.py (Diff revision 10)

The issue has been resolved. Show all issues

Docstrings should be of the form:

"""Single line summary.

Multi-line description.
"""

reviewboard/static/rb/js/views/commentIssueBarView.js (Diff revision 10)
The issue has been resolved. Show all issues
```
Remove trailing whitespace.
```

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 10)

The issue has been resolved. Show all issues

Method doccomments should use multi-line comment syntax, e.g

/*
 * Hide ...
 */

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 10)

The issue has been resolved. Show all issues

Method doccomments should use multi-line comment syntax, e.g

/*
 * Hide ...
 */

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 10)
The issue has been resolved. Show all issues
```
Remove trailing whitespace.
```

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 10)

The issue has been resolved. Show all issues

Method doccomments should use multi-line comment syntax, e.g

/*
 * Create ...
 */

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 10)

The issue has been resolved. Show all issues

Can we assign $(this) to a variable (e.g., $this) and use that?

reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 10)
The issue has been resolved. Show all issues
```
Same here.
```

Summary:

[WIP] Redesign the issue summary table UI to speed up the process of addressing issues.

Redesign the issue summary table UI to speed up the process of addressing issues.

Description:

		Redesign the UI to make the issue summary table more useful, by:

~		[Completed] Making issue descriptions expand so that full content is viewable on click. This change allows the user to conveniently see full issue descriptions in-table, without having to click the description and be redirected to the comment itself.
	~	Making issue descriptions expand so that full content is viewable on click. This change allows the user to conveniently see full issue descriptions in-table, without having to click the description and be redirected to the comment itself.

		A new div in review_issue_summary_table.html contains full comment text and is initially hidden.
		The CSS class, 'issue-summary-collapsed,' is applied to this hidden div when the summary is collapsed. JS methods, onPreviewClicked and onFullClicked handle user clicks.

~		[Completed] Adding buttons to fix/drop issues in the issue summary table.
	~	Adding buttons to fix/drop issues in the issue summary table.

~		Buttons are now functional (clicking fix or drop affects the actual state of the issue regardless of whether it was pressed in the table or in the review).
	~	Each entry in the issue summary table now has a set of Fix and Drop buttons, visible when expanded, from which you can address the issue and the status of the issue will update everywhere. You can also continue to use the previously existing buttons in the review to fix, drop, or reopen, and the table will be updated accordingly.

Testing Done:

		Manual testing (expandable descriptions):
		- Added long (past the 20 word cutoff mark) comments to a review request.
		- Able to click the description in the issue summary table to see the full comment text.
		- If the full text is being displayed, able to switch back to the 20 word version upon click.
		(buttons):
~		- Clicking the Fix or Drop button when a description is expanded fades the issue out of the table and updates its status at the actual place in the review. It stays upon refreshing the page.
~		Added new commentIssueBar tests and ran js-tests.
	~	- Clicking the Fix or Drop button when a description is expanded fades the issue out of the table and updates its status at the actual place in the review. It stays upon refreshing the page.
	~
	+	Added new commentIssueBar unit tests and ran js-tests. All passed.

Commit:

5b526e2420094c21427c4dd8247506de8135cacf

1ad4a4f56102a47b3cde615e9a30449a53d2bee9

Diff:

Revision 11 (+207 -19)

Show changes

	reviewboard/reviews/templatetags/reviewtags.py
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/views/commentIssueBarView.js
	reviewboard/static/rb/js/views/issueSummaryTableView.js
	reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js
	reviewboard/templates/reviews/review_issue_summary_table.html
	reviewboard/templates/reviews/table_comment_issue.html

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/table_comment_issue.html
    reviewboard/static/rb/js/views/commentIssueBarView.js
    reviewboard/templates/reviews/review_issue_summary_table.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/views/issueSummaryTableView.js
    reviewboard/static/rb/js/views/tests/commentIssueBarViewTests.js

Status:: Discarded

	+	From review request 6910 (Make descriptions in the issue summary table expandable so full comments are viewable in table on click):
	+	Added a new div in review_issue_summary_table.html that contains full comment text and is initially hidden.
	+	Added a new CSS class, 'issue-summary-collapsed,' to be applied to this hidden div when the summary is collapsed.
	+	Added two new JS methods, onPreviewClicked and onFullClicked, for handling user clicks.
	+
	+	This change allows the user to conveniently see full issue descriptions in-table, without having to click the description and be redirected to the comment itself.
	+
	+
	+
		Resolved merge conflicts in review_issue_summary_table.html (integrated markdown text and expandable summaries)

		Update: buttons now show up in the right place but they're not fully functional yet (clicking fix or drop doesn't affect the actual state of the issue)