Page MenuHomePhabricator

[Dev] Adopt template partials in Vector and revise sidebar component
Closed, ResolvedPublic3 Estimated Story Points

Description

NOTE: I believe this task will help implement T243281 more effortlessly.

From what I can see with the new version of Vector the easiest path to maintenance would be to maintain 2 master templates or to maintain a single template with conditionals related to version number.

In both scenarios we would benefit from the use of template partials to make index.mustache more readable and easier to rearrange.

Acceptance criteria

  • There is a single call to processTemplate inside VectorTemplate.
  • Any new components are documented in storybook and have a template associated.
  • Any revised components are updated in storybook
  • Consider updating VectorTemplateTest or adding tests elsewhere

Developer notes

Associated patches show an incremental way to do this.

For context on how this helps T243281 read T243281#5871879 and then consult POC Vector patch

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 17 2020, 7:56 PM

Change 572738 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] POC: Template partials

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

Jdlrobson triaged this task as High priority.Feb 17 2020, 7:57 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson renamed this task from [Dev] Adopt template partials in Vector to [Dev] Adopt template partials in Vector and revise sidebar component.Feb 17 2020, 8:04 PM
Jdlrobson updated the task description. (Show Details)
nray added a subscriber: nray.Feb 20 2020, 7:26 PM

I've been looking at @Jdlrobson's POC patches related to template partials in trying to determine how to do T243281 (Note the note at the top of the task!). So far they look good to me, but I'm trying to work out if the following AC is the right approach:

There is a single call to processTemplate inside VectorTemplate.

Namely, I'm wondering if we could achieve more/better composition if we made VectorTemplate's execute method be entirely responsible for the the composition of components (and thus have multiple ->processTemplate() calls each rendering a different component with the data it needs) vs. having the execute method be responsible for the composition of data/props AND having mustache match that composition structure through partials composed of partials (which is what I think this AC would require).

If my description sounds entirely vague or nonsensical to you, it's probably because it is! I'm having trouble articulating the approach I'm proposing, but I'm spending some time today to work out a POC patch that will hopefully explain it much better!

Change 574019 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] POC: Embrace the Power of the "children" Prop

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

nray added a comment.Feb 21 2020, 3:58 PM

If my description sounds entirely vague or nonsensical to you, it's probably because it is! I'm having trouble articulating the approach I'm proposing, but I'm spending some time today to work out a POC patch that will hopefully explain it much better!

POC Patch at https://gerrit.wikimedia.org/r/574019 that builds on Jon's work. Not sure if that approach is worth it or not, but I hope that clears up what I was aiming for (namely having more reusable components)

Change 572738 abandoned by Jdlrobson:
POC: Template partials

Reason:
abandoning for now

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

Moving to upcoming on the assumption that T113095 happens first and is a precursor to estimating this.

nray added a comment.Feb 24 2020, 8:42 PM

Change 572738 abandoned by Jdlrobson:
POC: Template partials

Reason:
abandoning for now

@Jdlrobson your POC patches looked good to me for the most part. Could you provide more context as to why they were abandoned?

Change 572738 restored by Jdlrobson:
POC: Template partials

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

ovasileva set the point value for this task to 3.Feb 26 2020, 5:13 PM
Niedzielski updated the task description. (Show Details)Feb 26 2020, 5:14 PM

Change 574019 abandoned by Nray:
POC: Embrace the Power of the "children" Prop

Reason:
see Icdb5e3c21208747084e891de2574241c650730d2 instead

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

Merge blocked on T113095 but that should hopefully be unblocked today.

Change 572738 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Use template partials rather than HTML strings

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

Demian updated the task description. (Show Details)Mar 4 2020, 12:54 AM
Jdlrobson closed this task as Resolved.Mar 11 2020, 5:06 PM
Jdlrobson updated the task description. (Show Details)