VectorTemplate is written in a very imperative style. It reads top to bottom and inlines each detail needed, rendering as it goes. This approach may appear simple but is actually very rigid and makes it challenging and error prone to iterate on without knowing the whole thoroughly. Specifically:
- Skin hooks are mixed with HTML. In some cases, a transition plan may be needed to change this API lifecycle but there are probably some good refactors to be made that can minimize where hooks and HTML intermingle to better separate these concerns.
- Code is not modular. Large portions of VectorTemplate are written in lengthy functions that cannot be reused. These could be split up to enable composition, improve the flexibility for future work, and potentially improve readability (for instance, the block of code that is specific to the sidebar could be isolated to make that clear). Refactoring code into distinct functions and separate files (either Mustache templates or PHP) could be an improvement.
- Most of the code directly renders. PHP echos and inline HTML inhibit the ability to aggregate and compose components independent of their function call order. The rendering (calling echo) should be deferred.
Acceptance criteria
- VectorTemplate is split into distinct components: large functions are split and files are separated where it makes sense. When considering what to refactor, keep in mind that many of these components will need to be iterated on for new DIP changes while at the same supporting the old layout. For instance, it may make sense later on to have an "old" component and a "new" component if they diverge greatly.
- Where public API changes are not needed, hook invocations are separated from the components.
- Rendering (echo and inline HTML) is deferred.
QA
TODO: for each patch merged, update this section. It would be ideal if Edward could focus on testing an individual component instead of the entire skin. JavaScript and non-JavaScript tests should probably be performed to minimize risk.
Related prior work: https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/552123/
