[SPIKE] Discuss template partials in Vector
Closed, ResolvedPublicSpike
Actions

Assigned To

Authored By

	Jdlrobson
	May 26 2020, 5:32 PM

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?

#content.mw-body to Content.mustache, Content.less and legacy/Content.less,
Common root of VectorTabs (#mw-head) to Tabs (.mw-tabs)
Logo (.mw-logo) -> done: patch 577682

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.

Details

	Subject	Repo	Branch	Lines +/-
	[refactor] Split content into its own template to allow reuse in legacy and new Vector	mediawiki/skins/Vector	master	+54 -88
	[Refactor] Split Tabs (`#mw-head`) component from both old and new version	mediawiki/skins/Vector	master	+41 -35

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Resolved	Goal	ovasileva	T242131 [GOAL] Deploy new header to at least one test wiki
Resolved		Jdlrobson	T240856 [EPIC] New header for desktop improvements project
Resolved	Spike	Volker_E	T249073 [Spike 4hrs] Discuss and agree on nomenclature and rules for template partial usage and CSS organization from experience with header
Resolved	Spike	Jdlrobson	T253669 [SPIKE] Discuss template partials in Vector

Event Timeline

Jdlrobson created this task.May 26 2020, 5:32 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 26 2020, 5:32 PM

Jdlrobson mentioned this in T249073: [Spike 4hrs] Discuss and agree on nomenclature and rules for template partial usage and CSS organization from experience with header.May 26 2020, 5:41 PM

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

Jdlrobson moved this task from Needs triage to Technical on the Vector (legacy skin) board.May 29 2020, 1:13 AM

• Demian added a parent task: T249073: [Spike 4hrs] Discuss and agree on nomenclature and rules for template partial usage and CSS organization from experience with header.Jun 18 2020, 4:22 AM

• Demian mentioned this in T249676: [EPIC] Desktop Improvements tasks (catalog).Jun 18 2020, 4:27 AM

• Demian claimed this task.Jun 25 2020, 6:57 AM

• Demian mentioned this in W2550 Inactive tasks.

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

• Demian mentioned this in T246420: Limit content width, and refine alignment & styling of relevant elements.Jul 1 2020, 6:15 PM

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:

Added 2 new template data for the jumplink target hrefs.
Only the innerHTML of #content is refactored. The layouts can decide what tag and classes to use on the element.
CSS modifications dropped, that's been handled in for ex: https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/606044

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.

• Demian added a project: User-Demian.Jul 3 2020, 11:49 PM

@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.

• Demian moved this task from Incoming to In progress on the User-Demian board.Jul 4 2020, 1:41 AM

• Demian moved this task from In progress to Awaiting Review on the User-Demian board.

• Demian added a project: Spike.Jul 5 2020, 5:00 PM

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

• Demian moved this task from Awaiting Review to Denied on the User-Demian board.Nov 2 2020, 12:05 AM

[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

cc @cjming @bwang
Perhap it's time to actually do this spike and talk about why and when we want to use partials given the recent conversations on user menu.

Jdlrobson assigned this task to ovasileva.May 26 2021, 3:50 PM

Jdlrobson moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog board.

Jdlrobson moved this task from Needs Prioritization to Triaged but Future on the Web-Team-Backlog board.Jun 3 2021, 8:25 PM

Will plan a Monday chat.

Jdlrobson removed ovasileva as the assignee of this task.Jun 21 2021, 11:25 PM

Jdlrobson triaged this task as Low priority.

Jdlrobson added a subscriber: ovasileva.

Jdlrobson removed a project: Spike.Jun 23 2021, 6:37 PM

Jdlrobson edited projects, added Spike; removed Patch-For-Review.

Team chatted about this. We're quite happy with how template partials are being used in Vector and the approach to pulling out new partials when we work on new features and need to support old and new versions.

We're not so sure about the recent approach of using separate templates that compile to html- because of the Skin::makeLink function.
We'll explore 2 different approaches to doing this (using helper functions and using data)

[SPIKE] Discuss template partials in VectorClosed, ResolvedPublicSpikeActions