Page MenuHomePhabricator

VisualEditor's and OOUI's toolbars are conceptually incompatible
Open, LowPublic8 Estimated Story Points

Description

VisualEditor's and OOUI's toolbars are conceptually incompatible. Yet somehow we managed to implement one of top of the other.

Lifecycle of OOUI toolbars:

  • (constructor)
  • (attached to DOM)
  • #initialize
    • #setup
    • #reset
  • #destroy

Lifecycle of VisualEditor toolbars (approximate, as each target does it slightly differently):

  • (constructor)
  • #setup
  • (attached to DOM)
  • #initialize
    • #detach
    • #setup
    • (attached to DOM)
  • #destroy

Problems:

  • #setup is called before #initialize. This is just wrong.
  • In VisualEditor, each target has one toolbar and multiple surfaces. This necessitates detaching and reattaching the toolbar from surfaces, and thus from the DOM, which is not supported by OOUI (you'd have to #destroy it and #initialize again).
  • #initialize is called after the toolbar is attached, but only called once per toolbar. #initialize is explicitly meant to calculate stuff after the toolbar is attached, and the stuff becomes invalid when it is reattached somewhere else. This affects upcoming narrow-display styles for the toolbar (https://gerrit.wikimedia.org/r/#/c/193282/).
  • We insert toolbars inside toolbars, which make the #setup and #initialize mess even worse. The actions toolbar was replaced with right-aligned toolgroups. We also (in VE-MW) insert the Save button into the toolbar long after it is attached, initialized and set up, which breaks #initialize's calculations again. Save button is a now a normal tool

Event Timeline

matmarex raised the priority of this task from to Needs Triage.
matmarex updated the task description. (Show Details)
matmarex added subscribers: matmarex, Krinkle, Esanders.

I tried to rework this, but I officially give up.

Is this Technical-Debt ? If so feel free to associate that project.

Volker_E renamed this task from VisualEditor's and OOjs UI's toolbars are conceptually incompatible to VisualEditor's and OOUI's toolbars are conceptually incompatible.Jan 17 2018, 12:53 AM
Volker_E updated the task description. (Show Details)
Deskana lowered the priority of this task from Medium to Low.Jan 17 2018, 1:10 PM
Deskana set the point value for this task to 8.

Toolbars-in-toolbars is no longer a thing (sorry Xzibit), which should make things a bit easier.