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)