One feature that Discourse doesn’t currently offer is the ability for themes to have some sort of settings or variables that would work similar to how site/plugin settings work i.e. theme settings would be defined with default values by theme developers, and then site owners could override those default values and they’d still be able to get new updates for the theme without worrying about losing their configurations.
How should this work?
Obviously this is not complete specifications - this is just a rough idea on how I see this working. I’m keen to hear your ideas and thoughts
a) For site owners
I think all we need to do here is add a simple button somewhere on the theme page. that navigates to a new route /admin/customize/themes/:theme_id/settings and the new route will display the settings which will look like this:
I think a new field next to CSS and HTML fields that lets theme developers type their theme settings in YAML (or JSON?) format should do the job. Mockup:
(Note “help” is the text that will be displayed below the setting that explains what the setting does)
c) Supported data types
I think we should have setting types that support these data types:
Integer
String
Boolean
Enum
List
d) Accessing theme settings in theme JS code
How will theme developers access theme settings in their JS code? I’m thinking something like Discourse.ThemeSettings.theme_setting_01 but this won’t help with naming collision (when two themes have a setting with the same name) which I’m wondering how should be handled?
For now that’s all I have. Happy to hear how you see this working
Localisation for the settings help text right? Should we have another field dedicated for translations, or should translations go in the settings field like this ?
setting_01:
help:
en: "foo bar"
fr: "foo bar french"
It works pretty much exactly like the OP describes except for one little thing: I created a separate “Setting” tab for the YAML field instead of nesting it under the Common tab. To me it makes for sense to have its own tab. Open to revert it back if you like the proposal in the OP.
@Osama I feel we do need a few refinements if you feel up to it:
Not a huge fan of having this in the UI, I am fine for all the config to happen in the remote git repo. This is very much a feature for ninja theme creators and not the general public.
@sam Is there a reason why the local settings JS variable only includes settings that are explicitly used like this settings.some_setting_name and doesn’t just include all settings of the theme?
I’ll admit I don’t have a real use-case, I was testing it and noticed that I had to that to be able use my setting. Yeah let’s wait and see if anyone will run into a problem because of this.
Is there by chance a way to access theme settings in a script of type="text/x-handlebars"?
I’ve been able to use settings.name to get the value in a script of type="text/discourse-plugin" (and it’s awesome I must say!), just wondering if it’s possible to get a setting value in a handlebars script somehow?
I’m still getting my bearings with all of this, so forgive me if there’s something simple I’m missing to handle this
Oh yeah … PR completely welcome on this. Just need to figure out how to shuffle the data there. (we can’t do a search replace here, we would have to pass in the setting objects to the context)
@sam I’m looking into this right now and my plan to go about this is to create a little handlebars helper that would take a key and value as params and inject them into the template context. Then on the server side we get all text/x-handlebars scripts and before they’re compiled, we prepend to them something this like for each theme setting:
It’s not working properly in raw handlebar templates. Here incorrect context variable hash.data.root is used instead of hash.hash.context. But even fixing it not resolving the problem since the context itself carrying incorrect value.
In this line instead to set this object to context variable the raw handlebar precompiler setting constant string value "this" to the context(context = "this"). It is working perfectly in normal handlebars since it using different precompiler.
Ah, apologize about that. It seems my implementation worked in some cases but not all because I tested it by overriding the list/topic-list-item.raw template and it worked fine. Could you tell me which raw template my implementation failed to work with so I can debug this further?
Yes, after debugging it further I found that not all the raw templates raising the issue. It only happens when a theme include a raw-plugin-outlet connector template. For example when I use the raw plugin outlet “topic-list-before-status” in below theme it breaks the UI.
So the issue may not be in precompiler. Sorry for my previous misguided reply. But we have to resolve this issue before supporting theme settings for raw templates.
There is actually an issue with the precompiler that it doesn’t recognize this as the template’s context object. However, I just realized that we don’t really need the precompiler to recognize this because the helper function here is bound to the template context object, so this inside the helper function refers to the template context, which is exactly what we need here. Will put together PR to demonstrate what I mean.
?
