Add a central infobox manager and independent infobox views.
Review Request #9064 — Created July 10, 2017 and submitted
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 aRB.InfoboxManagerView
that manages each type of infobox on the page. It keeps track of the
active infobox, shows/hides based on themouseenter
andmouseleave
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 ofRB.BaseInfoboxView
for
each type of infobox we use, along with a jQuery$.fn.*
registration
for each (which allows us to remove some more code fromcommon.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. |
david | |
Can you switch to Function.bind instead of _.bind? |
david | |
Col: 20 A constructor name should start with an uppercase letter. |
reviewbot | |
Fat arrow functions work fine here, or at least Function.bind. |
david | |
Can't this just be .done(onDone)? |
david | |
Col: 15 Missing semicolon. |
reviewbot | |
Col: 23 'infoboxView' is defined but never used. |
reviewbot | |
Col: 20 A constructor name should start with an uppercase letter. |
reviewbot |
- Change Summary:
-
- Switched to
Function.bind
- Removed the
=>
for theonDone
handler. - Updated the docs for the class.
- Switched to
- Commit:
-
198678dc8d417f3dc4c35dca26b0ddb4b07ade1b6ca8e93f69470cd9cd9942a77684e8150e0b2dec
- Diff:
-
Revision 2 (+735 -192)