[go: up one dir, main page]

Page MenuHomePhabricator

Decouple Watchstar from Toast and CtaDrawer
Closed, DeclinedPublic

Description

When an anonymous user clicks the watchstar, a ctaDrawer prompting them to sign-up or log-in is displayed. To achieve this, the Watchstar class imports the CtaDrawer class, instantiates a new CtaDrawer, and caches that instance as a property of Watchstar (i.e. Watchstar.drawer).

The toast notification uses a different mechanism altogether (mw.notify coupled with localstorage), and is imported into Watchstar as an instance and triggered within the Watchstar ajax call-back.

Directly instantiating CtaDrawer from within Watchstar creates a tight-coupling of the two classes and makes Watchstar directly responsible for CtaDrawers behaviour. doesn’t watchstar have enough on it’s mind already?

Watchstar shouldn’t have to bear the responsibility of showing/hiding CtaDrawer as well as toast. These responsibilities should be delegated to an impartial third-party controller. This controller could also keep track of the state of both entities to ensure they are never displayed at the same time.

It might also be beneficial if this state controller was flexible enough to handle other toasts/drawers (e.g. reference drawer).

Event Timeline

Jdlrobson triaged this task as Medium priority.Aug 2 2019, 5:04 PM
Jdlrobson subscribed.

This task is several years old. I don't see much value in keeping in open longer. All code can be reevaluated if and when we do T281930.