Redesign the user infobox.

Review Request #8074 — Created March 24, 2016 and submitted

Information

Review Board
release-3.0.x
ae01554...

Reviewers

This change is the first of two major improvements to the user infobox. This
one makes the infobox itself much lighter weight (with a lighter border color
and much lighter drop-shadow), and tweaks the design a bit.

This involves several changes:

  • Instead of listing the username and real name on separate lines, we now
    prefer the real name if it's available, and put the username in parens on
    the same line.
  • The avatar is now even bigger.
  • The avatar is flush with the sides of the infobox.
  • The user's local time is shown (computed by their preferred timezone).

There's also some code cleanup in here. I've created a new view for infoboxes,
which simplifies the code in common.es6.js. I also discovered while doing this
that the way we were fetching the infobox contents wouldn't send the
If-None-Match header. I've fixed that up, and made the client-side caching
better so if we have data cached client side, we'll show that immediately and
then update it when we get data back from the server.

This also updates the version of moment to the latest stable.

Looked at a bunch of user infoboxes and saw everything looked nice.


Description From Last Updated

Can we add more padding? Maybe 1em? This is looking very crowded.

chipx86chipx86

'ChangeDescription' imported but unused

reviewbotreviewbot

This should probably be a constant as we're also using it above in .user_infobox.

brenniebrennie

Doc comments throughout.

brenniebrennie

/* */

brenniebrennie

Mention in description that we're updating momentjs?

brenniebrennie

Blank line between these.

chipx86chipx86

Should we do UTC or configured server time?

chipx86chipx86

Multi-line comments should use: /* * */

chipx86chipx86

We chain this elsewhere, like: view.render().$el.appendTo(document.body); Minor thing, but saves some chars.

chipx86chipx86

Can we add literal quotes for <a>, so generated docs don't have issues?

chipx86chipx86

Same here.

chipx86chipx86

Blank line between these.

chipx86chipx86

Are we no longer doing one const/var/etc. keyword per grouping? I feel like consistency is good here.

chipx86chipx86

Blank line between these.

chipx86chipx86

Same question as above.

chipx86chipx86

Missing localization.

chipx86chipx86

Space before />. (Technically required for non-XML.)

chipx86chipx86

Missing localization.

chipx86chipx86

Missing localization.

chipx86chipx86
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/lib/js/moment-2.0.0.min.js
        reviewboard/templates/accounts/user_infobox.html
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/css/common.less
        reviewboard/static/rb/js/ui/views/infoboxView.es6.js
        reviewboard/static/lib/js/moment-timezone-0.5.2.js
        reviewboard/static/lib/js/moment-2.12.0.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/lib/js/moment-2.0.0.min.js
        reviewboard/templates/accounts/user_infobox.html
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/css/common.less
        reviewboard/static/rb/js/ui/views/infoboxView.es6.js
        reviewboard/static/lib/js/moment-timezone-0.5.2.js
        reviewboard/static/lib/js/moment-2.12.0.js
    
    
  2. reviewboard/reviews/views.py (Diff revision 1)
     
     
    Show all issues
     'ChangeDescription' imported but unused
    
  3. 
      
brennie
  1. 
      
  2. reviewboard/static/rb/css/common.less (Diff revision 1)
     
     
    Show all issues

    This should probably be a constant as we're also using it above in .user_infobox.

  3. reviewboard/static/rb/js/common.es6.js (Diff revision 1)
     
     

    You're not using a fat-arrow here because jQuery binds this to be the element, correct?

  4. Show all issues

    Doc comments throughout.

  5. Show all issues

    /* */

  6. reviewboard/staticbundles.py (Diff revision 1)
     
     
    Show all issues

    Mention in description that we're updating momentjs?

  7. 
      
david
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/lib/js/moment-2.0.0.min.js
        reviewboard/templates/accounts/user_infobox.html
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/css/common.less
        reviewboard/static/rb/js/ui/views/infoboxView.es6.js
        reviewboard/static/lib/js/moment-timezone-0.5.2.js
        reviewboard/static/lib/js/moment-2.12.0.js
        reviewboard/static/rb/css/defs.less
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/lib/js/moment-2.0.0.min.js
        reviewboard/templates/accounts/user_infobox.html
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/css/common.less
        reviewboard/static/rb/js/ui/views/infoboxView.es6.js
        reviewboard/static/lib/js/moment-timezone-0.5.2.js
        reviewboard/static/lib/js/moment-2.12.0.js
        reviewboard/static/rb/css/defs.less
    
    
  2. 
      
brennie
  1. Ship It!
  2. 
      
chipx86
  1. 
      
  2. Show all issues

    Can we add more padding? Maybe 1em? This is looking very crowded.

    1. I can't really add more padding without it overflowing the height of the box.

    2. That's going to be a problem for extensions then, won't it?

    3. Incidentally, adding more padding is one reason why I was hoping to drop the joined date.

    4. I think if padding breaks the design, we need to work more on the design before this can go in. We can't say "extension authors can now add additional content to the info box, but it will break if you want more than a line or two of text."

    5. Is the infobox actually a fixed height, or will it grow as content is added? If so, and this is about having just too much default content for us to be able to add padding, then we should rework the content. Maybe shrink the current time indicator and e-mail address. Hard to really tell how it'll look, because of the Retina image on the screenshot.

    6. Content added by extensions goes in a new section under both the image and the user data. Like this:

  3. reviewboard/reviews/views.py (Diff revision 2)
     
     
    Show all issues

    Should we do UTC or configured server time?

    1. All our config defaults to UTC (as does the field in the profile). I think the best solution would be to add some location-based timezone setting for all users.

  4. reviewboard/static/rb/css/common.less (Diff revision 2)
     
     
     
    Show all issues

    Multi-line comments should use:

    /*
     *
     */
    
  5. reviewboard/static/rb/js/common.es6.js (Diff revision 2)
     
     
     
    Show all issues

    We chain this elsewhere, like:

    view.render().$el.appendTo(document.body);
    

    Minor thing, but saves some chars.

  6. Show all issues

    Can we add literal quotes for <a>, so generated docs don't have issues?

  7. Show all issues

    Same here.

  8. Show all issues

    Blank line between these.

  9. Show all issues

    Are we no longer doing one const/var/etc. keyword per grouping? I feel like consistency is good here.

    1. For var I think it's fine to group them because there we want "one statement at the top of the function" (and linters prefer that). For let/const I'm doing one per definition because they can appear anywhere.

  10. Show all issues

    Blank line between these.

  11. Show all issues

    Same question as above.

  12. 
      
david
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/lib/js/moment-2.0.0.min.js
        reviewboard/templates/accounts/user_infobox.html
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/css/common.less
        reviewboard/static/rb/js/ui/views/infoboxView.es6.js
        reviewboard/static/lib/js/moment-timezone-0.5.2.js
        reviewboard/static/lib/js/moment-2.12.0.js
        reviewboard/static/rb/css/defs.less
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/lib/js/moment-2.0.0.min.js
        reviewboard/templates/accounts/user_infobox.html
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/css/common.less
        reviewboard/static/rb/js/ui/views/infoboxView.es6.js
        reviewboard/static/lib/js/moment-timezone-0.5.2.js
        reviewboard/static/lib/js/moment-2.12.0.js
        reviewboard/static/rb/css/defs.less
    
    
  2. 
      
david
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/lib/js/moment-2.0.0.min.js
        reviewboard/templates/accounts/user_infobox.html
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/css/common.less
        reviewboard/static/rb/js/ui/views/infoboxView.es6.js
        reviewboard/static/lib/js/moment-timezone-0.5.2.js
        reviewboard/static/lib/js/moment-2.12.0.js
        reviewboard/static/rb/css/defs.less
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/lib/js/moment-2.0.0.min.js
        reviewboard/templates/accounts/user_infobox.html
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/css/common.less
        reviewboard/static/rb/js/ui/views/infoboxView.es6.js
        reviewboard/static/lib/js/moment-timezone-0.5.2.js
        reviewboard/static/lib/js/moment-2.12.0.js
        reviewboard/static/rb/css/defs.less
    
    
  2. 
      
chipx86
  1. 
      
  2. reviewboard/reviews/views.py (Diff revisions 2 - 4)
     
     
     
    Show all issues

    Blank line between these.

  3. Show all issues

    Missing localization.

  4. Show all issues

    Space before />. (Technically required for non-XML.)

    1. According to whom? If I'm reading the HTML5 spec correctly, that's not true.

      That said, HTML5 seems to encourage "void tags" (like br) to not have closing tags at all, so I'll change it to be <br>

    2. Used to be the case at least. Some browsers would ignore the tag.

  5. Show all issues

    Missing localization.

  6. reviewboard/templates/accounts/user_infobox.html (Diff revisions 2 - 4)
     
     
     
    Show all issues

    Missing localization.

  7. 
      
david
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/lib/js/moment-2.0.0.min.js
        reviewboard/templates/accounts/user_infobox.html
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/css/common.less
        reviewboard/static/rb/js/ui/views/infoboxView.es6.js
        reviewboard/static/lib/js/moment-timezone-0.5.2.js
        reviewboard/static/lib/js/moment-2.12.0.js
        reviewboard/static/rb/css/defs.less
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/lib/js/moment-2.0.0.min.js
        reviewboard/templates/accounts/user_infobox.html
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/css/common.less
        reviewboard/static/rb/js/ui/views/infoboxView.es6.js
        reviewboard/static/lib/js/moment-timezone-0.5.2.js
        reviewboard/static/lib/js/moment-2.12.0.js
        reviewboard/static/rb/css/defs.less
    
    
  2. 
      
chipx86
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.0.x (1577842)