Make draft comments in diffviewer and text based attachments resizable and movable

Review Request #7032 - Created March 9, 2015 and updated

Wu Di
Review Board
master
2f1b795...
reviewboard, students

Currently if a draft comment is made with the wrong range of lines, there is no way to correct it other than to delete it and make a new one. This fix aims to make the comment bubbles movable and resizable to update the line range.

Manual testing by dragging comment blocks.

Dragging:
Checked that the comment block can only be dragged if it is a new draft comment and not one made to an existing comment thread.

Resizing:
Checked that begin row cannot be resized to rows after end row.
Checked that end row cannot be resized to rows before begin row.
Checked that begin/end row is resized to a valid row (not headers).
Checked that dragging can be done across compressed diff headers.
Checked that dragging cannot be done across different files.

Moving:
Checked that line range stays the same when moving.
Checked that comment block can only be moved within visible lines.
Checked that line range cannot contain invalid rows.
Checked that dragging cannot be done across different files.

Triggering of comment dialog:
Checked that the comment dialog is triggered on click of the comment block.
Checked that the comment dialog is triggered on click of a row number in the diffviewer.

Loading file attachments...

  • 6
  • 19
  • 0
  • 25
Description From Last Updated
It's not immediately obvious to me how these are used. At the very least, there should be some documentation somewhere ... Christian Hammond Christian Hammond
This is probably a good place for that documentation I mentioned. Christian Hammond Christian Hammond
Best to pull out moveState into a local variable first, since it'll save on attribute lookups. Christian Hammond Christian Hammond
Given that we're doing this, I'd like you to look into what jquery provides, since they have all this done ... Christian Hammond Christian Hammond
A lot of this code looks the same as above. Can you factor it out into a mixin? Christian Hammond Christian Hammond
Why this change? Christian Hammond Christian Hammond
Review Bot
Wu Di
Review Bot
Wu Di
Review Bot
Wu Di
Review Bot
Wu Di
Review Bot
Wang Jun Sun
Wu Di
Review Bot
Wu Di
Review Bot
Christian Hammond
Wu Di
Review Bot
Wu Di
Review request changed
Review Bot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/views/textBasedCommentBlockView.js
        reviewboard/static/rb/css/pages/diffviewer.less
        reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
        reviewboard/static/rb/js/models/textBasedCommentBlockModel.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentBlockModel.js
        reviewboard/static/rb/js/views/textBasedReviewableView.js
        reviewboard/static/rb/js/views/textCommentRowSelector.js
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/views/textBasedCommentBlockView.js
        reviewboard/static/rb/css/pages/diffviewer.less
        reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
        reviewboard/static/rb/js/models/textBasedCommentBlockModel.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentBlockModel.js
        reviewboard/static/rb/js/views/textBasedReviewableView.js
        reviewboard/static/rb/js/views/textCommentRowSelector.js
    
    
Loading...