Future major features

Element Library

This discussion has been locked because a year has elapsed since the last post. Please start a new discussion topic.
Picture of Frédéric Massart
Re: Element Library
Group Core developersGroup Moodle Course Creator Certificate holdersGroup Moodle HQGroup Particularly helpful MoodlersGroup Plugin developersGroup Testers

Hi guys,

I have chatted a few times with Damyon in the office, but I thought that sharing my views here would not hurt.

In general, I tend to prefer the approach B. Mainly because it does not involve much API change, but just some clarifications on how a to write a good renderer and renderable. However, there are a few things that I am not sure I agree with.

Anything can be a renderable

This is a great feature because it allows a developer to very quickly pass something to a renderer, but I'm afraid that this can quickly expand to be unmanageable and bug prone.

Say you are developing a plugin that is mainly using one class. Rather than defining a new class to implement renderable you will implement renderable to your class. So far, that's not really an issue.

Now you are working in your renderer and calling the different properties of your class to construct HTML. But you stumble upon some value for which you have not define a property, though it is accessible from a method A. So you will call the method A, and it will work just fine, but by doing so you break the rule where a renderable should only contain simple values. Anyway, let's say that this is acceptable because the method A is extremely simple.

But your plugin evolves, and that method A called in your renderer too. Now it contains much more logic than before, and it calls other methods, those methods query the database, use caching, trigger events, etc... Of course, whenever you updated the method A, you forgot that it was called from the renderer.

At this point, your renderer has become ugly, it is doing a lot of computation under the hood through the renderable that was supposed to contain only simple scalar values. Who does that hurt? Well, luckily the theme designer is not too much affected because the logic is not contained in the renderer, but we ended up lying to ourselves when we said that we would not have any more logic in renderers, we just moved it somewhere else and closed our eyes.

I can this of those typical: 'is_visible()', 'get_post_content()' or 'can_edit()'.

Now, let's say that we can live with that, and let's think about templates for a moment. Surely our ultimate goal is to move to templates, and with renderables it is extremely easy: you fetch all the properties from the object, pass them to the template, and there you go, done! But what about our method A... from a coding point of view, how would I know that I need to fetch the return value of method A? Surely we could define somewhere the list of properties/methods to fetch from a renderable, but that should not be the case... you get the point.

Oh, did I mention the private/protected properties on my renderable? And the open access to anything public, or caught by __call(), __get(), etc...?

Renderables can only be rendered by one renderer

If you consider renderables as containers of information, then there is no reason this container could not be re-used in other circumstances and renderered differently. For example, a link_renderable could contain 'text', 'url' and possibly 'icon'. This renderable can be re-used anywhere, in a dropdown, a flat list of links, an indented list of link (navigation), etc...

If there is only one way to render a renderable, that means that a link_renderable should be rendered with an icon and a text. But what if I want to display a list of icon without text? In this example I cannot call the common render_link() method because it outputs the text of the link, therefore I have to create another renderable that is icon_link_renderable, and that renderable only contains the URL and the link. But that is not a good option.

This option means that the developer makes strong decisions on how the content should be displayed because they pass a certain renderable to the renderer. In this last example, the icon_link_renderable does not contain the text, so a themer will never be able to render it with a text.

As renderables only contain information, there is no reason we should duplicate them in order to create a different output. We already have the information ready, let's just make use of it. For instance you would never know how a user_renderable should be displayed (with link, without, only picture, only name, ...) but it would always contain the information that are crucial for a themer to change its layout.

One would say that if a renderable can be rendered differently in different locations it does not help a themer. Actually, it does not change anything for a themer, or maybe it even makes it easier for them. There would be more renderers if we have less renderables, and more renderables if we have less renderers, but the naming convention of the renderers would make more sense.

For instance, you could create your own new renderer that receives a user_renderable and create your own specific output from it. Whereas if you have to overwrite the renderer username_and_picture to display just the user name of the user because of your design decisions, the name does not make sense any more.

Dom manipulation

I strongly disagree with the logic that uses some sort of dom_utils to edit the content or classes of a node that was generated in another renderer. This is placing complex logic back into the renderers, and even if they are tied to the 'display', I do not agree that should exist.

I am not sure how to solve this, but if a renderable can be passed around to other renderers, then it should be alright to overwrite the corresponding renderers, or create new ones to output the renderer the way you need.

Those are the majors concerns that I have about the specification. We have to write proper new renderers and make sure that the rules are defined for this to evolve in the right direction in the future.

Many thanks!

Average of ratings: Useful (5)