• 
      

    Add a central infobox manager and independent infobox views.

    Review Request #9064 — Created July 10, 2017 and submitted

    Information

    Review Board
    release-3.0.x
    6ca8e93...

    Reviewers

    This is a large cleanup of our infobox code. Originally, we only had
    infoboxes for users, and they didn't need to do anything other than
    display some HTML. Over time, this became more complex. The user infobox
    had more interaction requirements with its date/time display, and we
    added new types of infoboxes, all sharing the same logic.

    Infobox display management was left up to each infobox instance (of
    which there was one per type of infobox). While you'd never see more
    than one instance of a type of infobox on the screen at the same time,
    it was possible to get two infoboxes of two different types to appear at
    the same time.

    This change separates out infobox display management/target registration
    from the infoboxes themselves. There's now a RB.InfoboxManagerView
    that manages each type of infobox on the page. It keeps track of the
    active infobox, shows/hides based on the mouseenter and mouseleave
    events (along with the smarts for allowing the user to move the mouse
    from the target element to the infobox without causing anything to
    hide), and handles creating/populating the instances of the infoboxes.

    It also has a utility function for creating the $.fn.* methods we use
    to register infoboxes on elements, like those with the .user class.
    This allows us to maintain that compatibility and ease of use in a more
    straight-forward way.

    The infobox views themselves handle the actual rendering and interaction
    for the infobox content. There's a subclass of RB.BaseInfoboxView for
    each type of infobox we use, along with a jQuery $.fn.* registration
    for each (which allows us to remove some more code from common.e6.js).

    This is the first part of a larger cleanup that will lead to more
    reusable infoboxes, and even allow for extensions to more easily provide
    their own infoboxes.

    Tested the dashboard, All Review Request page, Groups/Users pages, and
    the review request page to check each type of infobox.

    Tested moving between valid targets of the same type and of different
    types to make sure that infoboxes displayed and transitioned corerctly.

    Tested moving between the target and the infobox or vice-versa kept the
    infobox on-screen without triggering a new transition.

    Tested that previously-cached infobox contents were shown immediately on
    hover.

    Tested that the user infobox's date/time handling (both the "timesince"
    filter and the hover display on the time) were working correctly.

    New unit tests pass.

    Description From Last Updated

    Or a review request.

    daviddavid

    Can you switch to Function.bind instead of _.bind?

    daviddavid

    Col: 20 A constructor name should start with an uppercase letter.

    reviewbotreviewbot

    Fat arrow functions work fine here, or at least Function.bind.

    daviddavid

    Can't this just be .done(onDone)?

    daviddavid

    Col: 15 Missing semicolon.

    reviewbotreviewbot

    Col: 23 'infoboxView' is defined but never used.

    reviewbotreviewbot

    Col: 20 A constructor name should start with an uppercase letter.

    reviewbotreviewbot
    Checks run (1 failed, 1 succeeded)
    flake8 passed.
    JSHint failed.

    JSHint

    david
    1. 
        
    2. Show all issues

      Or a review request.

    3. Show all issues

      Can you switch to Function.bind instead of _.bind?

    4. Show all issues

      Fat arrow functions work fine here, or at least Function.bind.

    5. Show all issues

      Can't this just be .done(onDone)?

    6. 
        
    chipx86
    Review request changed
    Change Summary:
    • Switched to Function.bind
    • Removed the => for the onDone handler.
    • Updated the docs for the class.
    Commit:
    198678dc8d417f3dc4c35dca26b0ddb4b07ade1b
    6ca8e93f69470cd9cd9942a77684e8150e0b2dec

    Checks run (1 failed, 1 succeeded)

    flake8 passed.
    JSHint failed.

    JSHint

    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-3.0.x (a65ae50)