Issue 3201: Put a link next to rich-text fields.

Review Request #5378 — Created Feb. 1, 2014 and submitted

b.ramnani
Review Board
master
3201
reviewboard
chipx86, david, smacleod

Appended the markdown link next to the buttons in 'Description', 'Testing Done', 'Comments' and 'Reply to comments' fields.

For each view, checked the state of all combinations of buttons pressed/unpressed to make sure the markdown link appears when beginning edit and disappears when cancelling edit. For reviewReply and reviewRequest views, opened multiple text editors at the same time to check if the markdown informaiton appears.


Description From Last Updated

Can you use CSS to right-align this within its parent? I think I'd also like it if it said something ...

daviddavid

Leave this blank line here.

daviddavid

Instead of using to split this across lines, I think I'd prefer joining an array of strings: $('.body-top').append([ '<a class="markdown-info" ...

daviddavid

Same here with string joins.

daviddavid

This line has trailing whitespace.

daviddavid

Please put a space between if and (.

daviddavid

Same here with string joins.

daviddavid

Please put a space between if and (.

daviddavid

Please put a space between if and (.

daviddavid

This should be indented two spaces, and have a space between the : and the value.

daviddavid

Leave this line here.

daviddavid

These look like they're indented 3 spaces instead of 4.

daviddavid

The strings should be indented 4 spaces in from the previous alignment. It might be worth moving all the chained ...

daviddavid

Same here with indentation and spaces at the ends of each string.

daviddavid
david
  1. Please attach screenshots of the changed UIs.

    You also have several places in your code that use tabs. We only use spaces for indentation.

    1. Made the required changes

  2. 
      
B.
david
  1. This is looking better.

    I didn't see code or screenshots for doing this to the comment dialog. Can you also show a screenshot of the review dialog with multiple comments in the draft?

    1. Please also but the bug number into the "Bugs" field.

  2. Can you use CSS to right-align this within its parent?

    I think I'd also like it if it said something like "This field uses Markdown" instead of just "Markdown Field".

  3. Instead of using to split this across lines, I think I'd prefer joining an array of strings:

    $('.body-top').append([
        '<a class="markdown-info" ',
        '   href="..." ',
        '   target="_blank">...</a>'
        ].join(''));
    
  4. Same here with string joins.

  5. This line has trailing whitespace.

  6. Same here with string joins.

  7. 
      
david
  1. 
      
  2. Leave this blank line here.

  3. Please put a space between if and (.

  4. Please put a space between if and (.

  5. Please put a space between if and (.

  6. 
      
B.
B.
david
  1. 
      
  2. reviewboard/static/rb/css/reviews.less (Diff revision 3)
     
     

    This should be indented two spaces, and have a space between the : and the value.

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

    Leave this line here.

  4. reviewboard/static/rb/js/views/reviewDialogView.js (Diff revision 3)
     
     
     
     
     

    These look like they're indented 3 spaces instead of 4.

  5. The strings should be indented 4 spaces in from the previous alignment.

    It might be worth moving all the chained methods down and indent them only 4 spaces. Something like this:

    $draftComment
        .find('pre.reviewtext')
        .inlineEditor('buttons')
        .append([
            '<a class="markdown-info" '
            ...
        ]);
    

    You also need to add spaces at the ends of these strings, because the join won't insert them. As it is, this would create the string "<a class="markdown-info"href="..."target="...">"

  6. reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 3)
     
     
     
     
     
     

    Same here with indentation and spaces at the ends of each string.

  7. 
      
B.
david
  1. I'm going to make some small changes (a little bit to the style and adding localization) and push this. Thanks!

  2. 
      
B.
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (997ad1a). Thanks!
Loading...