Page MenuHomePhabricator

Split BannerPresenter
Closed, ResolvedPublic8 Estimated Story Points

Description

Our current BannerPresenter class does too much. It should be split into the following parts:

  • Tracker (see T327626: Extract event tracking into an interface)
  • "DisplayDecision" class that does the *initial* display decision during the display delay (usually 5-7 seconds), based on events from "Plugins"
    • Viewport measurement
    • event handlers (user clicks search bar, visual editor etc)
    • Page type on wikipedia.org
  • Move "close" event handler out into the banner components.
  • Resize component/composable that interacts with the skin adjuster (but is just for the resize event)

Acceptance Criteria:

  • We have a banner setup with a simple "hello world" banner that has the following features:
    • display decision logic
    • tracking (of close button and display decision outcome)
    • close button (closes the banner and sets the cookie, no soft-close at the moment)
    • resize (wp.org skin adjustment) features.
  • Both unit and banner tests work

Implementation Details:

  • The new BannerPresnter keeps state for PENDING / DISPLAY / HIDDEN and interacts with tracker (for the 2 state changes) and SkinAdjuster.
  • it does not
    • animate
    • keep state for sub-banner (initial, fullpage, softclose, etc)
  • "DisplayDecision" might be a class or a component. The mentioned "Plugins" can be their own classes or methods inside the "DisplayDecision" class
  • Wrap mw object in an interface and inject it, use a null implementation for wp.de banners to reduce if statements.
  • Use some form of dependency injection (provide/inject, property passing) for wm API, skin adjuster and tracking.
  • The "close" event handler (that handles the click on the close button) could become a composable (useClose)

todo: extract the following into its own ticket

Proposed structure of "orchestrating" components:

<BannerPresenter>
    <BannerConductor>
      <MainBanner />
      <SoftcloseBanner>
   </BannerConductor>
</BannerPresenter>

BannerPresenter = Keeps state for PENDING / DISPLAY / HIDDEN and interacts with tracker and SkinAdjuster.
BannerConductor = Keeps state *which* banner is shown, passes "real" close event (= the one that hides all banners, sets the "do not display again" cookie with the mw api and) to the BannerPresenter and calls the SkinAdjuster. Needs to to be flexible/generic enough to put any number of banners in.

Either BannerPresenter or BannerConductor are responsible for adding a resize event listener in SkinAdjuster. Pay attention which class to use for measuring the actual size of the banner (inside the resize handler). It must be unique! Instead of a resize handler that's always on, the API of the SkinAdjuster allows for "watching" a class (with automated resize on requestAnimationFrame) and another method to stop watching (also called when the banner hides).

Related Objects

StatusSubtypeAssignedTask
ResolvedNone
ResolvedNone