Page MenuHomePhabricator

MFA: Allow our View's to support easy adding of children
Closed, DeclinedPublic

Description

Currently its really difficult for a MobileFrontend component to have several child components.
This is evident in the fact that we have many ways of doing this.

Method 1: toHtmlString

Certain classes have a method toHtmlString which allows us to inject the HTML of a component into a larger component

Consider a template

<div>
    {{{icon}}}
</div>

We do the following:

var myIcon = new Icon( options );
template.render( {
    icon: myIcon.toHtmlString() 
} );

The problem with this, is all the other data inside myIcon is lost. As @Krinkle points out in T149909 this is an extremely wasteful way just to get some HTML and the output is not tied to the code that created it.

Method 2: templatePartials
Identical to method 2, but we make use of a template partial:

<div>
    {{#myIcon}}{{>anchor}}{{/myIcon}
</div>
var myIcon = new Icon( options );
template.render( {
    myIcon: myIcon.options
} );

Exact same problems as method 1.

Method 3: View.prototype.postRender
Any View can extend postRender to add subcomponents.

Template:

<div>
    <icon-placeholder></icon-placeholder>
</div>

code:

postRender: function () {
    var myIcon = new Icon( {} );
    this.$( 'icon-placeholder' ).replaceWith( myIcon.$el );
}

This is great as it means the icon.$el is linked up to the component itself.
However, it's a little annoying as every component needs to extend the parent class and provide its own postRender
This is why we have lots of classes that extend Overlay's.
It's not great if you want to use composition rather than inheritance.

Method 4: A new way - children

If our view's had a concept of children, we could pass other components to them.
This avoids the LoadingOverlay extends Overlay problem as LoadingOverlay becomes an Overlay with a child

	return new Overlay( {
		className: 'overlay overlay-loading',
		noHeader: true,
		children: [
			spinner()
		]
	} );

Under the hood, this would work similar to "method 3" but automate that in such a way that no inheritance is needed to be made use of. We would need to designate an area for children to go (in my POC below I suggest a data-children attribute, and if absent, simply append to the top level DOM node)

The problem with this approach is where to put the children.
It is similar to how React does stuff but with a bit of a limitation.
For instance in React (jsx) you can do the following:

<div>
    <ChildOne />
    <div>
        <AnotherChild />
        <div>
            {children}
        </div>
    </div>
</div>

However in our View's an element would only be able to have a sequence of children - not children in different places.

I think this is okay and not too much of a trade off.
A POC exists in : https://gerrit.wikimedia.org/r/468186 and I'd love us to pursue it further.

Related Objects

StatusAssignedTask
OpenNone
OpenNone
OpenNone
ResolvedJdlrobson
OpenNone
ResolvedJdlrobson
Resolvedpmiazga
ResolvedJdlrobson
DeclinedNone
ResolvedJdlrobson
DeclinedNone
ResolvedNiedzielski
DuplicateReedy
ResolvedNiedzielski
ResolvedJdlrobson
ResolvedJdlrobson
DuplicateNone
Resolvedpmiazga
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 15 2018, 9:31 PM
Jdlrobson updated the task description. (Show Details)Nov 15 2018, 9:32 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson triaged this task as High priority.Nov 22 2018, 9:21 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)

Change 475576 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] pageIssuesOverlay is a factory function that returns an Overlay

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

We chatted about this during super happy dev team. The team is naturally a bit concerned about adding a workaround (option 4). Using a tried and tested alternative e.g. Preact was quite popular and we're going to explore that a little more. In the mean time, option 3 could be preferable. I am currently working on T206036 which I hope might find us a way to break out of composition.

Using a tried and tested alternative e.g. Preact was quite popular and we're going to explore that a little more.

During your exploration, you might consider Vue since WMDE have experimented with server-side rendering Vue templates in PHP: https://github.com/wmde/php-vuejs-templating

Jdlrobson closed this task as Declined.Dec 18 2018, 10:34 PM

During your exploration, you might consider Vue since WMDE have experimented with server-side rendering Vue templates in PHP

Yup.

Gonna decline this bug (we can reopen this later) as I think the apprehension to build something new is valid and T206036#4832740 seems like a way forward using what we have.

Change 475576 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] pageIssuesOverlay is a factory function that returns an Overlay

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