Page MenuHomePhabricator

Have a consistent way of doing Views within Views
Closed, DuplicatePublic


Sometimes we need to put a View inside another View. A good example is the Icon class which sometimes gets embedded in a template using


And sometimes as


We need a consistent way of embedding these. Right now the existence of two ways causes confusion in code review.

Developer notes

We discussed this in the CR session on 17th August 2017

As @pmiazga rightly pointed out in the TalkOverlay we create views in 3 different ways - template data e.g. headerButtons raw HTML (e.g.heading), using templatePartials and passing the options of a View class (footerAnchor).

There's also a 4th way (toHtmlString)

The ability to use template partials adds to this problem. We should standardise on one approach and stick to it.

Related: T149909

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: Web-Team-Backlog.
Jdlrobson moved this task to Triaged but Future on the Web-Team-Backlog board.
Jdlrobson subscribed.
Jdlrobson edited projects, added MobileFrontend; removed Web-Team-Backlog.
Jdlrobson set Security to None.
Niedzielski subscribed.

What are the pros and cons of each? {{{icon}}} seems clear and concise.





are both flawed in that they only copy across the HTML and the Icon is stateless. Doing this means any event handlers need to be redefined by the thing using the class.

I think the approach most compatible with how we are doing things is to use the postRender method to update the template
e.g. template would look like this:

<div><Icon data-key='foo' /></div>


postRender: function () {
  this.$('.icon[data-key=foo]').replaceWith( new Icon( { name: foo } ) )

I think long term, we'd look to avoid partials altogether (or at least abandon the use of template inheritance)

function TalkOverlayContent() {
TalkOverlayContent.prototype = {
  template: 'TalkOverlayContent.hogan'

var overlay = new Overlay( { content: new TalkOverlayContent(), header: new OverlayHeader() } );

Does this seem like a reasonable approach?