Page MenuHomePhabricator

[SPIKE] Discuss template partials in Vector
Open, Needs TriagePublicSpike

Description

Q1: Do we need to refactor any of the existing templates to create new components that have been suggested? If so what are the scopes of these components?

Q2: What are the guidelines for using template partials? I (@Jdlrobson) believe that we should only use template partials if they are either 1) reusable 2) used inside a loop. Template partials within template partials are a potential sign that our templates have got overly complex (Yo-Yo problem) and are likely to suffer from the same issues that we see with class inheritance. An alternative opinion might be that we want to keep the master template as simplified as possible. Let's discuss pros and cons of both approaches.

Note from @Demian: Template partials are the equivalent of composition in OOP (best practice), not inheritance (long-term issues). Building complex components with simple components is a common pattern. Its benefits are 1) reusability 2) comprehensibility 3) separation of concerns.

Outcomes

Document any decisions in the codebase.

Event Timeline

Change 585626 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] [Refactor] Split Tabs (#mw-head) component from both old and new version

https://gerrit.wikimedia.org/r/585626

Change 585625 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] [Refactor] Split content into its own template to allow reuse in legacy and new Vector

https://gerrit.wikimedia.org/r/585625

Change 585625 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] [refactor] Split content into its own template to allow reuse in legacy and new Vector

https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/ /585625

For the benefit of T246420 I've redone the 'content.mustache' separation in patch 585625.

Changes since the last time we discussed this in May:

Comparison (not too useful): https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/585625/22..24

@Jdlrobson when you have some time please give your endorsement or rejection to this change that we discussed in May and forgot to follow-up with. Now would be the right time to do it, to make the content width limiting patch cleaner. Thank you.

@Volker_E do you remember we've discussed splitting content.mustache a long time ago to remove the code duplication between modern and legacy.
I've been thinking about this change would make @nray's T246420 patch cleaner. Would you consider it? Patch 585625.

Restricted Application changed the subtype of this task from "Task" to "Spike". · View Herald TranscriptJul 5 2020, 5:00 PM

Change 585626 abandoned by Aron Manning:
[mediawiki/skins/Vector@master] [Refactor] Split Tabs (#mw-head) component from both old and new version

Reason:
To be restored when considered

https://gerrit.wikimedia.org/r/585626

Aklapper added a subscriber: Demian.

[Resetting assignee due to inactive user account]

Change 585625 abandoned by Jdlrobson:
[mediawiki/skins/Vector@master] [refactor] Split content into its own template to allow reuse in legacy and new Vector

Reason:
This code has deviated a lot from master. Not a priority right now.

https://gerrit.wikimedia.org/r/585625