Summary

Fix up all the diff viewer anchor code.

Review Request #4296 — Created July 6, 2013 and submitted July 8, 2013, 2:21 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Fix up all the diff viewer anchor code.

The diff viewer anchor code was in a semi-working state. There were
still calls to the now-removed 'gotoAnchor', which errored out, and
there was a hacky global function for the individual DiffReviewableViews
to scroll anchors.

Now the views handle all anchor click events. DiffReviewableView
triggers events when the file header or a chunk is clicked, and
DiffViewerPageView reacts to that. The PageView also handles all clicks
from the index that comes before the diff.

Testing Done

Tested all anchors manually.

Unit tests pass.

Issues

Description	From	Last Updated
Should this call e.preventDefault()?	david	July 8, 2013, 2:21 a.m.

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
  Ignored Files:
    reviewboard/templates/diffviewer/diff_file_fragment.html
    reviewboard/templates/diffviewer/changeindex_entry.html
    reviewboard/templates/diffviewer/diff_fragment_error.html
    reviewboard/templates/diffviewer/changeindex.html
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/static/rb/js/pages/views/diffViewerPageView.js

This is a review from Review Bot.
  Tool: Pyflakes
  Processed Files:
  Ignored Files:
    reviewboard/templates/diffviewer/diff_file_fragment.html
    reviewboard/templates/diffviewer/changeindex_entry.html
    reviewboard/templates/diffviewer/diff_fragment_error.html
    reviewboard/templates/diffviewer/changeindex.html
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/static/rb/js/pages/views/diffViewerPageView.js

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

The issue has been dropped. Show all issues

Should this call e.preventDefault()?

chipx86 July 7, 2013, 2:40 a.m.

The return false will do that. Since the <a> doesn't point to another page, the worst that happens is selectAnchorByName throws some error and the default behavior still jumps the anchor to the right place, just without highlighting (which isn't such a bad failure case).

Ship it!

```
Ship It!
```

Status:: Completed

Ship it!

```
Ship It!
```