How To override an existing handlebars template from plugin - Part II

(Benjamin Kampmann) #1

The original discussion about How To override an existing handlebars template from plugin has been closed, so I’ll leave the feedback here.

Did you actually test this?

I tried overwriting the menu header for a tiny plugin which just wants to add a “private message” button next to the “create topic” button for handy usage. But when I do it the way you describe it with a prefixed “javascript/”, handlebar/ember generates the template into:

Ember.TEMPLATES["javascripts/navigation/default"] = Ember.Handlebars.compile("{{bread-...");

Which, for the obvious reason that it has the wrong name, doesn’t get picked up of course. If instead you omit the “javascript/” (and of course move the file to the proper location) the application.js generation is fine

Ember.TEMPLATES["navigation/default"] = Ember.Handlebars.compile("{{bread-...");

But because the asset has the same name it seems to favor the first one it can find picked in the development environments javascript_tags-thing, making it a pain to develop and debug. Also it just feels not very thought-through nor reliable to be honest.

I personally would prefer a way to temper with templates (like inject your own stuff) before it gets compiled but I’m not sure that is easy to do.

Removing 'Users' column & not downloading the avatar images
(Vikhyat Korrapati) #2

Discourse’s resolver checks for the javascript/-prefixed version first.

I know it works because we’re using it in production. :stuck_out_tongue:

(Benjamin Kampmann) #3

Which plugin do you have that is using this? I’d love to compare mine with a working one…

(Vikhyat Korrapati) #4

One thing you could do is try removing the :client_side option from plugin.rb? Haven’t looked much but that is one difference I’ve noticed.

Add content to default template
(Michael - #5

Check out GitHub - discoursehosting/discourse-adsense: Google AdSense plugin for Discourse forum for a working example.

(Benjamin Kampmann) #6

@radq that essentially doesn’t do anything. The only option that actually does something is :server_side. Everything else is ignored, see discourse/instance.rb at master · discourse/discourse · GitHub .

Thanks to both of you for the examples. Updating to the latest tree and restarting vagrant (didn’t have decent internet the last few days, so I lagged behind by a few days), it seems to work now. Not exactly sure, what changed though and whether this might have been caused by caching or something. If anyone else can confirm that it works for them, I’d love to publish it.

I’d still like to mention that it feels awkward that you have to replace the whole template (like in this case the menu) just to add something as small as a button. If templates were super tiny that’d might be something that feels okay but already this template would interfere with any other plugin doing anything to that area.

(Régis Hanol) #7

We’re still improving our plugin support. Right now, when you’re adding an asset to your plugin, you absolutely need to clean the tmp/cache folder… This sucks, but we’ll get it sorted.

We’d be more than happy to add/provide any relevant hooks. We like to add them when there is a need for them, so do not hesitate to ask for them :wink:

(Alexandre Angelim) #8

How are you dealing with template changes in core after overriding them in plugins?
I wonder if it wouldn’t be better to be more specific when registering assets, creating something like register_template_override instead of register_asset. With that specialization we could provide a way to keep references to copies of the originals and verify changes after a fetch from upstream. Maybe a rake task could do a git diff of those registered overridden templates and output the differences.

(borhub) #9

Is there a way to override a template only for the Desktop View? I am in a situation where the ‘desktop’ override is leaking into the Mobile View, and haven’t found a way to register either

  • a Desktop-View-only template (tried adding :desktop, and :mobile to the register_asset call, but that only seems to work for CSS files)
  • a standard ‘desktop’ template and then a Mobile-View-specific template
  • not even a mobile-only template (see last line in the snippet below: the template in the /mobile/ folder never loads, even if the ‘desktop’ template is not registered at all)

Would be thankful for any hints/ideas on how to approach this. Thanks.

# SP customization: category page - use icons instead of text in table heading
#### register_asset "javascripts/discourse/templates/discovery/topics.js.handlebars"

# DOES NOTHING (attempt 1:1 copy of the default mobile template to re-override the desktop template above for Mobile View???)
register_asset "javascripts/discourse/templates/mobile/discovery/topics.js.handlebars"

cc @zogstrip @lightyear @santouras

(Régis Hanol) #10

That’s a good point. I think that when you override a template from a plugin, it will override both templates :frowning:

Am I right @eviltrout?

Swap position of activity column and posts column
(Kane York) #11

Currently the templates are actually the same for both desktop and mobile views - the display differences are through CSS and some JS if-statements.

Any reason you can’t hide the unwanted parts with mobile-only CSS?

(Robin Ward) #12

I’d like to hear the answer to this too. I am not against adding plugin outlet support for mobile, I just want to make sure we’re doing it for the right reasons.

(borhub) #13

@riking @eviltrout @zogstrip Thanks for your replies.

The Mobile-View’s /latest page topic-list template (javascripts/discourse/templates/MOBILE/discovery/topics.js.handlebars) doesn’t have the 7-column THEAD element.
So when I override the ‘desktop’ template in javascripts/discourse/templates/discovery/topics.js.handlebars, I end up with a leaked THEAD in the Mobile View, which - on narrow screens - makes the topic list occupy only 15% of the viewport width because the list’s single TDs are all under the THEAD’s first TH element (see below).

As suggested, I can workaround this specific case by setting display: none; on the THEAD in the mobile stylesheet, but I think it’s a bit hacky, and although at the moment I don’t have an example that would demand it, I believe it would make sense if the template overrides worked the same way as the CSS overrides and allowed you to specify if the template is for :desktop or :mobile.

(borhub) #14

@zogstrip @eviltrout @riking

And I have an example now…

We use the Tagger plugin. For each topic in the topic list, we now added a list of tags that are attached to the topic. Did this by overriding the template list/topic_list_item.js.handlebars and adding a new DIV with the tags there.

The mobile template for this area is quite different from the default ‘desktop’ one, so after the first override of the default or ‘desktop’ template, we now have no way to unbreak the Mobile View, because we can’t register an override for the mobile template.

I guess one way to work around this issue would be to provide a {{plugin-outlet}} in the standard templates, that we’d hook into.

cc @santouras @HAWK

(Robin Ward) #15

Okay I am totally convinced this is needed. I am going to be spending at a week this month on extensibility and the tagger plugin in and I will address this case. Unfortunately I’m on vacation next week so it will likely be the week after, more or less!

(James Bjorkman) #16

Curious if anything has happened with this?

(Sam Saffron) #17

Plugin work is on hold till we get v1 out in a couple of weeks.

(probus) #18

Has something been done to this? When can we expect a fix?

(Sam Saffron) #19


Overriding templates is now trivial from a site customization, see Sam's personal "minimal" topic list design

@eviltrout will make sure there are facilities for this in the plugin infrastructure as well

Edit existing source of the forum

Random question:

Why doesn’t this tag work from the admin interface? Clearly I must be doing something wrong.

Path: Customize > CSS/HTML > Header

<script type='text/x-handlebars' data-template-name='mobile/discovery/categories'>