Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

first draft of making module templates customizable #4547

Closed
wants to merge 1 commit into from
Closed

first draft of making module templates customizable #4547

wants to merge 1 commit into from

Conversation

Toflar
Copy link
Member

@Toflar Toflar commented Jul 20, 2012

This is my idea of how we could try to make every module's template configurable by the users.
This would give us _so much more_ flexibility, I can't even emphasize this enough!

In this PR I tried to use the $strTemplate variable of every module and it works perfectly fine. Every module gets a new modtpl field in tl_module.
Obviously this is not really backward compatible as - to make it a proper implementation - we would need to rename some of the templates, I guess.
E.g. the search module has different templates you can choose from in the "template settings". They'd become completely obsolete as this is in fact exactly the module template overriding possibility we want to have in every module.

Also I don't know whether taking $strTemplate from every module's class is a good idea. Probably it would be better to make this configurable with a global array. Or even better:
Make a Module::getTemplateChoices that returns $strTemplate by default and can be overwritten in every module. If it returns null then the field doesn't need to be displayed in the module options. But this option came to my mind while writing this comment so I'm not gonna change it again as it's a PoC anyway :)

Again, it's a foundation so other developers and maybe also @leofeyer have a common ground to start from for discussions and improvements.

Feel free to fork my branch and send me pull requests so this one gets automatically updated :-)

@leofeyer
Copy link
Member

leofeyer commented Aug 2, 2012

I am against it (like I said before). Does not mean that the decision has been made though.

@Toflar
Copy link
Member Author

Toflar commented Aug 2, 2012

Hmm, I don't understand how you cannot like having more flexibility without affecting any other figures (such as performance losses). It doesn't harm the existing user experience, it just provides more flexibility :-) And in the end Contao is a pro tool, isn't it? ;-)

@qzminski
Copy link
Member

qzminski commented Aug 3, 2012

There is already an extension for this: http://www.contao.org/en/extension-list/view/template_override.10010009.en.html

@leofeyer
Copy link
Member

leofeyer commented Aug 3, 2012

Can I close the pull request then?

@Toflar
Copy link
Member Author

Toflar commented Aug 3, 2012

I just don't think an extension is the way to go here. Really not. I'll just post the link to that PR in the IRC dev channel. Let's see what other dev's opinions are :)

@MacKP
Copy link

MacKP commented Aug 3, 2012

I totaly agree with Toflar. The way it is at the Moment is totaly confused for all People who used there own Templates (ore try to). Everyone needs an extension ore other ways in the Template to have more than one Template for Modules without any configuration for that. The catalog goes this way, you can choose the mod_ and all other templates at every Point. You don't have to, but you CAN. It would make it realy much easyer for all.

regards

@may17
Copy link

may17 commented Aug 9, 2012

I totally agree with Toflar and MacKP. We build a similar extension as qzminski did (but not available in the ER) but i think it would be such a nice core functionality and a solution out of the box, approachable for every contao user.

@dominikzogg
Copy link
Contributor

@leofeyer I would wish an explanation, why you are against it.

@backbone87
Copy link
Contributor

I am voting for this feature, too.
I would go a step further: For advanced "users" ALL templates used, should be customizable on a "per-usage" basis. Another part where this is not possible is for example the pagination. You can only change it on a "per-theme" basis, which does not always satisfy the requirements.

@xat
Copy link

xat commented Aug 9, 2012

Toflar, may17, MacKP, dominikzogg, backbone87: +1

@kikmedia
Copy link

kikmedia commented Aug 9, 2012

I totally agree with @Toflar @may17 @MacKP and I personally think this should be a core functionality, not an extension (like @qzminski suggested). In this case I don't want to be depending from extern ressources.

@backbone87
Copy link
Contributor

To clarify my intensions: I need this feature within my daily work, but I do not have taken a look at the implementation in this pull, yet. I basically do not "really" care how it is achieved, but it is definitely a common need within a large part of my projects.

@dominikzogg
Copy link
Contributor

@backbone87 +1

@tristanlins
Copy link
Contributor

I am against it (like I said before).

Where can I read what you said before?

@frontendschlampe
Copy link

+1

@blairwinans
Copy link

+1 on this as well. Having a flexible frontend template system = being able to override the same template in more than one instance.

@aschempp
Copy link
Member

aschempp commented Aug 9, 2012

Interesting discussion. It looks like i'm the only one with leos opinion. I've never had the urgent need for this solution, though I'm not saying it aint useful. Maybe @Toflar can convince me. It really depends on the solution, it must be very simple to use or beginners will get confused. Especially with things like news, where there already are selectable templates.

@tristanlins
Copy link
Contributor

@aschempp sometimes I need a module twice but in different layouts, for example the news list, but in one case I need more HTML elements around the list to build the design. The selectable list item template does not help me here, because I need the additional HTML elements around the list, not around every item.

Currently I solve this by add logic to the mod_newslist template:

<?php if ($this->id == X): ?>
  <div class="mod_newslist"><div class="wrapperA"><div class="wrapperB">...</div></div></div>
<?php else: ?>
  <div class="mod_newslist">...</div>
<?php endif; ?>

This is not realy user friendly :-(

@aschempp
Copy link
Member

I understand your example. I'm just saying that it was always solveable for me (= in my cases). On your example, I'd simply put the container on every element and ignore it when I don't need it... Or sometimes I use CSS classes to trigger PHP behavior.

@tristanlins
Copy link
Contributor

Yes there are everytime many solutions for one problem, for me this is the simplest and cleanest way ;-)

@cliffparnitzky
Copy link
Member

+1

@qzminski
Copy link
Member

+1, that's why I had to create the extension.

@Toflar
Copy link
Member Author

Toflar commented Aug 10, 2012

@aschempp it's as simple as that.

Or sometimes I use CSS classes to trigger PHP behavior.

Apart from the fact that this is dirty and really uncool it's still something that only developers can do. But we want to give that possibility to everybody :)

In fact the current implementation is inconsistent. My solution (in whatever way it is going to be implemented in the end - doesn't matter too much) is much more consistent. In my proposal, every module has it's module template and the user can change it if he/she likes. In the current solution, some modules do provide the option to change the module template and some don't. And if they do, the respective dropdowns for the choice have different names. The search module, for instance, provides the possibility to choose between mod_search_advanced and mod_search_simple by offering a field called searchType (https://github.com/contao/core/blob/master/system/modules/frontend/ModuleSearch.php#L98).

This is completely obsolete with my PR as you can do that using modTpl and as additional benefit you can even define your own templates - as many as you like.

@Toflar Toflar closed this Dec 14, 2012
@Toflar
Copy link
Member Author

Toflar commented Dec 14, 2012

Oh, reopen please. I'm still very convinced that this is really needed! I've just moved my branch to https://github.com/Toflar/contao-core-fork/tree/archive/module-templates

@leofeyer
Copy link
Member

Hm, I cannot reopen it, because it is a pull request. Still you are right, this is needed indeed!

@Toflar
Copy link
Member Author

Toflar commented Dec 14, 2012

Ah shit. Want me to recreate one or are you gonna cherry-pick it?

@leofeyer
Copy link
Member

Your changes are still there. Also, the discussion has to be preserved as well.

@leofeyer
Copy link
Member

I also tried to reopen using the API, but it just gave me

{
  "errors": [

  ],
  "message": "Validation Failed"
}

@leofeyer
Copy link
Member

  1. Remove template selection for registration and personal data module
  2. Remove template selection for login module
  3. We must override $this->strTemplate in constructor

All changed in 61b7777.

Also, why can I override the ce_html template, but not mod_html?

This has been fixed already.

@aschempp
Copy link
Member

@qzminski
Copy link
Member

Ah, sorry, you are right.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.