Make descriptions in the issue summary table expandable so full comments are viewable in table on click

Review Request #6910 — Created Feb. 5, 2015 and discarded

Information

Review Board
master

Reviewers

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.

Manual testing:
- 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.


Description From Last Updated

There should only be one var statement at the top of the function.

brenniebrennie

Space after comma, I believe. :)

SU Sunxperous

Any particular reason you're not just using .toggle()?

brenniebrennie

Space after ,

SU Sunxperous

And space after comma, and the next line (151) as well.

SU Sunxperous
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/templates/reviews/review_issue_summary_table.html
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/views/issueSummaryTableView.js
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/templates/reviews/review_issue_summary_table.html
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/views/issueSummaryTableView.js
    
    
  2. 
      
mike_conley
  1. Hey Teresa - would you mind attaching some screenshots to show us what this looks like in practice?

    1. Sure, although screenshots won't capture the animation--should I also try to screencast a video?

    2. Oh, yes please, if it's not too much trouble.

  2. 
      
TE
TE
WU
  1. <p>Instead of having a separate hidden div, can this be done using css?</p>
    <div class="codehilite"><pre>width: 100%;
    white-space: nowrap;
    overflow: hidden;
    text-overflow: ellipsis;
    </pre></div>

    1. Thanks for the suggestion! I tried your idea but I think it actually complicates it more, because you can't just set the width of the cell to be 100% or any specific value, as you want it to change the table width dynamically with any window resizing.

    2. I think you can use an outer div to workaround the cell width, something like this:

      div.issue-comment-wrapper {
          position: relative;
          div.issue-preview {
              width: 100%;
              white-space: nowrap;
              overflow: hidden;
              text-overflow: ellipsis;
              position: absolute;
          }
      }
      

      The width of the outer div will be adjusted dynamically with window sizing.

    3. In this case though, if I just take away the white-space nowrap when I want the full text displayed, the cell doesn't expand to fit the rest of the text, it just overflows into the next rows.

    4. That's due to the position absolute property. You do not need the properties under .issue-preview when you want the full text displayed, so you can remove the class from the div when you click to expand.
    5. Ah, got it to work, although with this approach the ellipsis can't be made into a link that can be used to expand as David suggested below. But if we decide on something else, I will consider your idea!

  2. 
      
david
  1. Hmm, just a question about the interaction here. How will people get to the actual review comment?

    1. The next steps in the project (as was outlined by Christian) are to add buttons for resolving and reopening issues, as well as the number of replies. When you click the number of replies it will take you to the actual comment. But that's a good point, perhaps I should just mark this review as a WIP and wait until I have added the other features to submit it for real consideration?

      Other suggestions for how to handle it are welcome. I might try to make some mock-ups of different options.

    2. Perhaps we can make the ellipsis a link, and use that to expand, while clicking on the row keeps the current behavior? I feel like jumping to the comment to see context and be able to reply is the most important part of the table.

    3. Not a bad idea, although I'm not sure how discoverable the expand feature will be if we go with that. What about a link to the comment once the cell is expanded? (see attached photo, 'alternative_option')

    4. That would work, though I'd suggest always showing that link (not just when the cell is expanded).

    5. Updated. Let me know what you think (see expandedv2 image)

    6. What does it look like before you expand?

    7. The first and third comments are not expanded (only the middle one is).

    8. Can we put "See comment" on the same row as the text when not expanded? This will make each line twice the current height.

    9. Done. (see updated images collapsed.png and expanded.png)

    10. I like that a lot!

  2. 
      
TE
TE
SU
  1. 
      
  2. Space after comma, I believe. :)

  3. And space after comma, and the next line (151) as well.

  4. 
      
TE
brennie
  1. 
      
  2. reviewboard/static/rb/js/views/issueSummaryTableView.js (Diff revision 1)
     
     
     
     
     
     

    There should only be one var statement at the top of the function.

  3. Any particular reason you're not just using .toggle()?

    1. Nope! Thanks for the suggestion.

  4. 
      
david
  1. Teresa,

    It looks like you marked Barret's issues as 'fixed' but haven't yet uploaded a new revision. When you get a moment, can you do so? (I know it's spring break, so no huge rush)

    1. I accidentally uploaded the changes as a whole new review request here last week (https://reviews.reviewboard.org/r/6997/). My mistake! Is there a way I can combine the two?

    2. There's no way to combine them. I'd just choose one to move forward on (maybe the new one), and then edit the description to link to the other. Whichever one is obsolete should be closed as "Discarded".

  2. 
      
TE
Review request changed

Status: Discarded

Loading...