• 
      

    Render the popup for the list of integrations in JavaScript.

    Review Request #10440 — Created March 6, 2019 and submitted

    Information

    Djblets
    release-1.0.x

    Reviewers

    The new integrations popup was previously hard-coded into the page,
    hidden by default and then wrapped in the JavaScript view, much like the
    configuration list until a recent change. That information has since
    been added to the list of data passed to the view, meaning we're sending
    it over the wire twice. This isn't efficient and isn't worth doing,
    particularly if the popup's never going to be used.

    This change updates the popup rendering to happen dynamically in the
    JavaScript view the first time it's clicked. This is done in a way that
    minimizes render time while also allowing subclasses to override the
    display.

    The Add Integration button has also received an icon. This uses the same
    look (a white plus symbol on a green circle) that the Add button for
    condition fields use, which we'll be able to use in more places
    throughout our config form UIs.

    Tested that the popup displayed properly and that integrations could be
    added.

    Unit tests pass.

    Summary ID
    Render the popup for the list of integrations in JavaScript.
    The new integrations popup was previously hard-coded into the page, hidden by default and then wrapped in the JavaScript view, much like the configuration list until a recent change. That information has since been added to the list of data passed to the view, meaning we're sending it over the wire twice. This isn't efficient and isn't worth doing, particularly if the popup's never going to be used. This change updates the popup rendering to happen dynamically in the JavaScript view the first time it's clicked. This is done in a way that minimizes render time while also allowing subclasses to override the display. The Add Integration button has also received an icon. This uses the same look (a white plus symbol on a green circle) that the Add button for condition fields use, which we'll be able to use in more places throughout our config form UIs.
    764da093df5f8ea8f6551d9c83072d82f1f4f0e4

    Description From Last Updated

    What's the idea behind waiting to run _.template?

    daviddavid

    Col: 36 Missing semicolon.

    reviewbotreviewbot

    E501 line too long (86 > 79 characters)

    reviewbotreviewbot
    Checks run (2 failed)
    flake8 failed.
    JSHint failed.

    flake8

    JSHint

    david
    1. 
        
    2. Show all issues

      What's the idea behind waiting to run _.template?

      1. An attempt toward reducing startup time of the JS side of the codebase. _.template isn't exactly free, and we run it for every single view that's in the JS bundle, even if the view is never used. If we're smarter about when we compile templates, we can reduce both memory usage and startup time in the browser.

        In the case here, we can guarantee that only one of the two templates is ever going to be used (we either use the "there's no integrations" template or we use the integration item template multiple times), and that's only if the popup is ever clicked (which won't be the case when just visiting the Team Admin page in RBCommons).

      2. Makes sense.

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