Installation of third party themes in 1.1.0

[Occam]

For me it is one of the most annoying "improvements" of 1.1.0 that 3rd party themes can only be installed very complicated. The switch from Niara to the default bootstrap and back succeeds without problems. But in order to install a theme from Sunnytoo, for example, you have to activate every single theme module after the installation. Now you may of course consider setting the installed custom modules to "enabled " in the theme's config.xml. This is not enough for thirty bees: They will then be activated, but still not displayed in the frontend, no matter if you clear cache or recompile the theme. You need to reset each and every module to make this work.

This is very confusing for the normal user, because after changing the theme you see only a white page with logo in the frontend - without error message! Because thirty bees does not detect this bug as an error.

In my opinion, this bug of 1.1.0 should be fixed as soon as possible, because it severely restricts compatibility. I'm not sure if it's just the strongly decimated AdminThemeController, probably there are also other changes involved. In 1.0.8 it definitely worked.
(Tested with a Sunnytoo Transformer theme adapted to thirty bees.)

[DRMasterChief]

Hi Occam, i have updated my test shop to 1.1.0  and i am using NOT Niara or standard theme,  i am using a 3rd party theme and this was flawless from 1.0.8 to 1.1.0  , also all the modules shows in BO.  Can you please give more details about your problem?  What do you mean with 'custom modules'  and maybe this only happens to  Transformer theme? 

[Kleijn36]

@DRMasterChief Occam is talking about the install of a NEW template i think (not update the shop)

I had the same experience with a clean TB 1.1.0 install and Panda Theme. After install the theme i had to activate every single theme module.

[Occam]

I am not sure if this only applies to Transformer; actually, I don't think so.

33 minutes ago, DRMasterChief said:

What do you mean with 'custom modules' 

Sunnytoo themes came along with own modules that sometimes replace the default ones.

33 minutes ago, DRMasterChief said:

also all the modules shows in BO

Sure they do, that's what I said.

33 minutes ago, DRMasterChief said:

i have updated my test shop to 1.1.0

You missed the point. After the update my theme worked. But when I switch to one of the default themes and then switch back to Transformer, then there is the trouble I spoke off.

Edited November 27, 2019 by Occam

[Occam]

3 minutes ago, Kleijn36 said:

I had the same experience with a clean TB 1.1.0 install and Panda Theme

That's what I expected. But you must know that the configuration file of Panda and Transformer only forces the installation of the modules, not the activation. If you change the configuration file and add the module activations command to this file, thirty bees will display the modules as activated, but will further ignore the activation.

The only way is either to deactivate and then activate every single module or to reset them one by one.

[datakick]

I've moved this discussion to the separate thread, as it has nothing to do with 1.1.1 release. 

[datakick]

I agree that the theme installation in 1.1.0 is not ideal, and *some* things should be improved or fixes. 

But let's start by explaining what is wrong, and why

Every theme contains config.xml file that describes

• modules that should be installed / enabled
• modules to be disabled
• hooks positions

The idea of this config file is that the theme will always look the same when you install it. Especially hook positions are important, because they allow theme developer to say

• what module hooks should be enabled. Not all content is required, for example some theme can decide that it doesn't want to display reviews in product miniatures
• order of module content -- for example, in left column display block categories first, then recent blog posts,...
• transplant module hooks -- when you install module, it is displayed as developer sees fits. But many times, module hooks can be transplanted to different location. For example, block categories can be displayed in right column instead of left column  

When the theme is installed, these steps are performed:

1. all modules that are marked as disabled are disabled
2. all modules that should be installed are installed
3. all modules that should be enabled are enabled
4. hooks for all enabled/installed modules are unhooked, and replaced by hook definitions from config.xml file

The reason this discussion exists is the step #4.

Prior version 1.1.0 there were few bugs. Module hooks were not unhooked correctly during theme installation, they were just somehow merged together. This resulted in non-deterministic result - every time the theme was installed it looked differently. Modules were ordered in wrong order. Some content was displayed multiple times, because theme transplanted hook to new location, but didn't remove the original hook. Some hooks that should not be displayed were displayed. Etc...

So, this was fixed. Now the theme config.xml file is the only source of truth regarding displayable content. 

And it works nicely for niara / community theme, because these themes have correct config.xml files. Unfortunately, many (most) commercial themes don't. They are shipped with wrong config.xml files. Sometimes, they don't contain hook positions at all. Or the hook list is only a subset of what's really required. The result is crippled theme after installation. Hooks are unhooked by core, but they are not hooked back because the theme does not say so. 

I believe both thirty bees and theme developers are at fault here.

Theme developers should have respected the api contract, and include all modules hooks into theme config.xml file. Then we wouldn't have these problems. But I'm not surprised theme developers didn't do that - because of the pre-existing bug I described above they weren't forced to. The theme always installed somehow, and that was good enough for them.

Thirty bees, on the other hand, should not be so eager to enforce these API contracts. There should have been some transition period, for example, when big red warning would be displayed during theme installation, informing user that the theme config.xml is not correctly composed. This would (hopefully) force the theme developers to fix their products. 

[piet]

14 hours ago, Kleijn36 said:

I had the same experience with a clean TB 1.1.0 install and Panda Theme. After install the theme i had to activate every single theme module.

It was also happening in 1.0.7 version as well. I had to do the configuration of the Panda theme manualy .

[datakick]

4 minutes ago, piet said:

It was also happening in 1.0.7 version as well. I had to do the configuration of the Panda theme manualy .

That was a different problem, with the similar result 🙂 And one of the reasons why the theme overhaul project was done for 1.1.0

[datakick]

I've filed an github issue to track this:

https://github.com/thirtybees/thirtybees/issues/1117

Also, there's another existing bug issue directly related to this

https://github.com/thirtybees/thirtybees/issues/1104

[piet]

9 minutes ago, datakick said:

That was a different problem, with the similar result 🙂 And one of the reasons why the theme overhaul project was done for 1.1.0

You cannot make perfect software at once but I think the work you guys do is great. Thanks for that.

I have one remark. If new users experience the config issue as described in this topic they will look for solutions  in the best case or leave and try somethng else. Good documentation how to setup TB with third parties themes is the key I think.  

[Traumflug]

3 hours ago, datakick said:

Thirty bees, on the other hand, should not be so eager to enforce these API contracts. There should have been some transition period, for example, when big red warning would be displayed during theme installation, informing user that the theme config.xml is not correctly composed.

Nice idea, but probably not doable. I see no chance to detect whether a module isn't mentioned because the theme developer doesn't want it to be installed, or whether it isn't mentioned by sloppiness.

Fixing themes is trivial, though. Install the theme, get its appearance right, then exporting it creates a correct config.xml.

[Occam]

4 hours ago, datakick said:

And it works nicely for niara / community theme, because these themes have correct config.xml files. Unfortunately, many (most) commercial themes don't.

From the programmer's point of view, this all sounds plausible and logical. For a merchant, however, only usability in practice counts. So if a lot of 3rd party themes can be installed in PrestaShop without any problems and this doesn't work in a small niche product like thrity bees, they will probably blame it on thirty bees and orientate themselves otherwise. Thirty bees just isn't big enough to tell third parties how to develop their products. And this argument doesn't help either:

4 hours ago, datakick said:

Theme developers should have respected the api contract, and include all modules hooks into theme config.xml file. Then we wouldn't have these problems. But I'm not surprised theme developers didn't do that - because of the pre-existing bug I described above they weren't forced to. The theme always installed somehow, and that was good enough for them.

Therefore I strongly endorse your proposal not be so eager to enforce these API contracts and to be content for the time being with a warning notice. Because this is not about the beauty of the PHP code but only about the practicality and compatibility of the software. @Traumflug may disagree, but I doubt that Thirty bees will be able to hold its own on the market in the long term with such an absolute claim.

13 minutes ago, Traumflug said:

Install the theme, get its appearance right, then exporting it creates a correct config.xml.

That would probably be the best way to scare the potential users away. Because the competing shop ssystems can do it without such detours.

Edited November 28, 2019 by Occam

[datakick]

33 minutes ago, Traumflug said:

Nice idea, but probably not doable. I see no chance to detect whether a module isn't mentioned because the theme developer doesn't want it to be installed, or whether it isn't mentioned by sloppiness.

Fixing themes is trivial, though. Install the theme, get its appearance right, then exporting it creates a correct config.xml.

Of course, it's not possible to verify correctness of the config.xml file. But it's possible to detect *missing* sections.

My battle plan is this:

For every module mentioned in theme config file, find all displayable hooks this module implements (displayLeftColumn, Header, displayProductFooter). If no such hook exists, then the module does not have impact on theme/front end at all, and should not be mentioned in theme config file in the first place.

If module implement any displayable hook, then we need to look into <hooks> section of the config file. If there are any entries for this module, we should consider this module to be fully managed by theme, and don't intervene. Simply replace all displayable hooks with hook list defined in config file, like we are already doing now. And hope that theme developer defined it correctly.

However, it there are no hooks defined in the <hooks> section for this module, then something is very fishy. Theme says to install module, but does not specify how it should integrate into theme. 

This is the situation that causes the problems.

Fortunately, we can detect this situation, and report it as a warning.

And we can also react to it. We can consider these modules as not-managed by theme, and don't do anything extra. Don't uninstall module's hooks, don't change hook exceptions, don't adjust position... because theme does not instruct us how to do it

I think this is a win-win solution. It allows us to gradually harden the rules, and yet maintain support with current themes.

I have a proof-of-concept of this ready for testing, and will submit it soon. I hope you can review the changes 

[vincentdenkspel]

10 minutes ago, Occam said:

That would probably be the best way to scare the potential users away. Because the competing shop ssystems can do it without such detours.

What competing shop systems are you referring to ? 
They may not have this 'issue' but will have other issues such as lack of support, very high costs etc.

[datakick]

15 minutes ago, Occam said:

Because this is not about the beauty of the PHP code but only about the practicality and compatibility of the software.

We didn't implement these changes because of the beauty of code, but for correctness. That's very different

[Occam]

Trust me, I am fully aware of this difference. My choice of words was just a little exaggerated. But I have moderated the Prestashop forum long enough to know what interests the average user. He doesn't want to know how or why, he only cares that it runs out of the box without much effort.

[DRMasterChief]

1 hour ago, Occam said:

Trust me, I am fully aware of this difference. My choice of words was just a little exaggerated. But I have moderated the Prestashop forum long enough to know what interests the average user. He doesn't want to know how or why, he only cares that it runs out of the box without much effort.

Thats the point!   (interesting discussion here, btw.)

The world gets more and more visual, clickable and maybe 'stupidal'. Not one young merchant who looks for a shop system is interested in such things,  he see a 'nice' theme and will install it with a one-click-solution - if it doesnt work, it doesnt fit...  and hop to the next shop system. 

[datakick]

Ok, branch issue-1104 contains proposed fix for this issue. Anyone who can,  please test and let me know how it works for you. Simply use core updater to forward to this branch.

 

[Traumflug]

21 hours ago, Occam said:

For a merchant, however, only usability in practice counts.

What you call 'usability' was pure luck. Themes not defining what they need relied on the previous theme doing the right thing.

Even the default theme had three distinct flavors: the one in config/xml/themes/themenname.xml, the one in the theme's config.xml, installation when doing a fresh shop installation was yet another flavor. Highly unreliable behavior.

[Traumflug]

2 hours ago, datakick said:

branch issue-1104 contains proposed fix

There is Module::getNotThemeRelatedModules() in AdminThemesController which intends to do something similar. Automatic detection is better than a handcrafted list, of course, still the new code might duplicate more existing code than necessary or allow to deprecate more of previous code.

Regarding these additional warnings: a simple "Theme configuration was unclear about the following modules, please review whether these modules are (un)installed the way you need them: <X>, <Y>, <Z>, ...," would be sufficient for my taste, but I take we have quite distinct opinions about what to burden merchants with.

[Occam]

4 hours ago, datakick said:

Ok, branch issue-1104 contains proposed fix for this issue. Anyone who can,  please test and let me know how it works for you. Simply use core updater to forward to this branch.

 

I'm afraid, that this does not solve the problem of theme incompatibilty with thirty bees. It is just more puzzling for merchants, incl. those (which are legion!) who don't even know what a hook is. And the results in the frontend are poor. You still need to reset most of the active modules to make the theme work properly. Non-experts would not understand that and instead complain about bugs or lack of compatibility.

Hence my question again: Does thirty bees really have the market power to dictate to theme (and module) developers how to write their software? Or is this puristic approach (however justified it may be) the direct path to meaninglessness at the time being? After all, it's by no means a situation where developers are rushing to create special adaptations for thirty bees.

1 hour ago, Traumflug said:

but I take we have quite distinct opinions about what to burden merchants with.

Just for the record: if you were a merchant, you would have other burdens and would understand such challenges only as an imposition. A merchant does not want to have to care about the software. He just wants it to work for him. He is neither interested in hooks nor in disputes with a theme developer who would definitely only answer that thirty bees causes these problems because they do not occur with PrestaShop.

Just my 2 cents ....

Edited November 29, 2019 by Occam

[datakick]

42 minutes ago, Occam said:

I'm afraid, that this does not solve the problem of theme incompatibilty with thirty bees.

It does. It behaves exactly as I described above -- if the theme *mentions* some module hook in config.xml, then this config is used. Otherwise, hooks are not touched for this module (but warning is emitted).

For themes with correct config file, the result is the same. For themes with incorrect config file, the the result is still functioning theme + couple of warnings. In 1.1.0, the result was non-functioning theme. That's a difference

Quote

It is just more puzzling for merchants, incl. those (which are legion!) who don't even know what a hook is.

Of course, we could just hide these warnings. Merchants would be happy that *it's all goooood*. But the truth is, it's not. Merchants should verify that the mentioned modules were installed correctly. And theme developer should fix their theme. 

Quote

Hence my question again: Does thirty bees really have the market power to dictate to theme (and module) developers how to write their software?

Or is this puristic approach (however justified it may be) the direct path to meaninglessness at the time being? After all, it's by no means a situation where developers are rushing to create special adaptations for thirty bees.

Again, it's about correctness. We know that the theme contains errors, is non-deterministic, not-specific. We can still try to install it, and the result will *most likely* be ok. But the problem is that it might not be ok. And that's not the situation we want to officially support. 

Quote

Just for the record: if you were a merchant, you would have other burdens and would understand such challenges only as an imposition. A merchant does not want to have to care about the software. He just wants it to work for him. He is neither interested in hooks nor in disputes with a theme developer who would definitely only answer that thirty bees causes these problems because they do not occur with PrestaShop.

Just my 2 cents ....

When you have theme that is not correctly written, then you can

• either have a system that installs the theme, says everything is ok and green, even though you can clearly see it's not. This is how prestashop do it 
• or you can have a system that at least warns you something is wrong, and points you to the problematic parts

 

 

[datakick]

5 hours ago, Traumflug said:

Regarding these additional warnings: a simple "Theme configuration was unclear about the following modules, please review whether these modules are (un)installed the way you need them: <X>, <Y>, <Z>, ...," would be sufficient for my taste, but I take we have quite distinct opinions about what to burden merchants with.

You are probably right here, it's not necessary to frighten users. I'll try to come up with a little less hardcore messages

[Occam]

So at the moment, no theme, not even the Niara theme that comes with tb by default, meets the strict criteria?

Not to mention the following warning concerning the Niara theme:

Warning: Theme installed or enabled following modules but didn't provide hook list for them.

Theme should always provide list of displayable hooks for managed modules in order to achieve consistent results. If no hooks are specified in config.xml file, module hook list will remain unchanged. If this is wanted behaviour, theme developer should make it explicit by adding manageHooks="false" into module entry

Please contact theme developer and request correction of theme config.xml file

Are you serious?

 

Edited November 29, 2019 by Occam