terminal42 / contao-easy_themes

Contao extension "easy_themes"
https://unmaintained.tech/
MIT License
13 stars 11 forks source link

Remove usage of tl script #45

Closed meier-simon closed 4 years ago

meier-simon commented 4 years ago

I use this extension in Contao 4.9.3 I display modules in "Backend Module" mode. When I created a custom back end route for my own Contao extension, the following problem occurred: Whenever I am on my custom back end route, the navigation links of contao-easy_themes are created incorrectly. I have fixed this by letting the Symfony router generate the correct path based on the Contao back end route name. The constant TL_SCRIPT that was used for this is declared as deprecated anyway.

Toflar commented 4 years ago

@qzminski could you check this? I think it's okay to replace it but we'll have to raise the minimum required Contao version to 4.4 then. In 3.x there was no router nor any container :) But raising is okay now that 3.5 has been dead for a year. Maybe there's other version comparisons that could be dropped then?

meier-simon commented 4 years ago

@qzminski Does it fit like this?

meier-simon commented 4 years ago

Hello @qzminski I have inserted the function apersand() again. Can you please check it?

qzminski commented 4 years ago

I think there was a small misunderstanding – you should not DECODE ampersands but ENCODE them. So instead of having

// Decode ampersands for Contao 4.5 (see #39)
if (version_compare(VERSION, '4.5', '>=')) {
    $arrReturn[$intThemeId]['href'] = ampersand($arrReturn[$intThemeId]['href'], false);
}

just use

$arrReturn[$intThemeId]['href'] = ampersand($router->generate('contao_backend', ['do' => 'themes', 'act' => 'edit', 'id' =>  $intThemeId, 'rt' => REQUEST_TOKEN]));

and it should be fine.

meier-simon commented 4 years ago

Hello @qzminski no unfortunately it does not work as you have described it The URL will be double ampersand()which will result in a wrong URL. If you look at Fix #39, this ampersand() function is disabled with false in Contao 4.5 and later. ampersand($href, false) So my adjustment would still be correct. However, I noticed that the Symfony Router is only integrated in Contao 4.5 and later. So the Fix #39 would not be necessary anymore.

qzminski commented 4 years ago

Thanks for checking! All good then, I will merge it today and release a new hotfix version.

meier-simon commented 4 years ago

Very good Thank you Will you include the minimum version of Contao 4.5 in the composer file of the project?

qzminski commented 4 years ago

@Toflar what do you think about bumping the minimum Contao version? 🤔

Toflar commented 4 years ago

Sorry but this is no option. We will have to do an if-else check then. We can raise the minimum supported version to ^4.4 but I don't want to leave users of the LTS version behind if there's no real reason.

qzminski commented 4 years ago

I have just released 2.2.10, thank you @MeierSimon !

bytehead commented 4 years ago

You forgot to raise the minimum version for contao to 4.4 in 2.2.10, see https://github.com/terminal42/contao-easy_themes/issues/46.

qzminski commented 4 years ago

@bytehead yup I did it in 2.2.11 https://github.com/terminal42/contao-easy_themes/commit/fab61ae8fdefef088eb512c0ca21edceb3171570 😅