[WIP] Fix rendering of first_line == None in diff comments

Review Request #10352 — Created Dec. 6, 2018 and updated

Information

Review Board
release-4.0.x
8104379...

Reviewers

Fixing rendering of None first_line


 

Description From Last Updated

It seems there's many ways that we could go about this change, potential issues: For the Django diff comment rendering …

alextechccalextechcc
Checks run (2 succeeded)
flake8 passed.
JSHint passed.
alextechcc
  1. 
      
  2. Show all issues

    It seems there's many ways that we could go about this change, potential issues:

    For the Django diff comment rendering side, we have a few options:
    1. Make a new template instead of reviewboard/templates/reviews/diff_comment_fragment.html, with diff rendering stuff removed
    2. Modify template to deal with None's / weird context passed to it
    3. Modify the context passed to the template so that it renders in a way that makes sense. (disable controls, 0 lines above, 0 lines below) This is how the current patch does it.

    There's decent chunks of code, in:
    - reviewboard/static/rb/js/resources/models/diffCommentModel.es6.js
    - reviewboard/static/rb/js/resources/models/tests/diffCommentModelTests.js

    That relies on positive, valid line numbers / number of lines.

    I'm also not sure if there's any code I'm missing here that touches / relies on line numbers.

  3. 
      
Loading...