Add a central infobox manager and independent infobox views.

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

Christian Hammond
Review Board
release-3.0.x
6ca8e93...
reviewboard

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.

  • 0
  • 0
  • 6
  • 2
  • 8
Description From Last Updated
Checks run (1 failed, 1 succeeded)
flake8 passed.
JSHint failed.

JSHint

David Trowbridge
  1. 
      
  2. Or a review request.

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

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

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

  6. 
      
Christian Hammond
Review request changed

Checks run (1 failed, 1 succeeded)

flake8 passed.
JSHint failed.

JSHint

David Trowbridge
  1. Ship It!
  2. 
      
Christian Hammond
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (a65ae50)
Loading...