4.1 Child templates 1/2 #35874

dgrammatiko · 2021-10-22T11:18:35Z

Pull Request for Issue # .

Summary of Changes

This PR

moves Cassiopeia and Atum to the new mode (eg supporting child templates)
moves the assets for the system templates to the media folder
~~patches tinyMCE to use the correct fallback editor.css~~

Testing Instructions

DO NOT USE PATCHETESTER FOR THIS ONE. Download the package instead

If you're using Git:
It needs npm I and also a change in the db table #_template_styles: the column inheritable should become 1 for all the styles of the 2 templates
If you're installing the installable package
No extra steps
Check that the Joomla Install doesn't have any visual regression
Check that the back end template is not missing any assets and doesn't have any visual regression (skip the extension templates, there's a PR [4.1] Child templates 2/2 [clean] #35998 for that)
CHANGE THE DEFAULT EDITOR TO NONE!!! (tinyMCE has its own PR [4.1] TinyMCE changes for child templates 3/3 #36011)
Check that the front end template is not missing any assets and doesn't have any visual regression
Check the Error pages
Check that if you delete the file media/templates/cassiopeia/css/editor.css and try to edit an article (assuming that tinyMCE is the default editor) the media/system/css/editor.css is in the list of loaded CSS files

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

There are no visual differences, the changes are in the XML definition of the templates and some static files moved to the media folder

Documentation Changes Required

Still needed:

The com_templates is not supporting yet the new mode PR: [4.1] Child templates 2/2 [clean] #35998
There is another PR for tinyMCE and the editor.css [4.1] TinyMCE changes for child templates 3/3 #36000

richard67 · 2021-10-22T11:26:58Z

@richard67 I need your expertise here for the sql changes and also how we are gonna handle the moving of the existing static folders to the media folder (I'm thinking of 2 functions: one runs preflight copying the css/js/images/scss to media and one post flight removing all these folders from the template/../...)

@dgrammatiko Which SQL changes? I can't see any change in the PR yet for new installations, so I have no idea what to do on updates. And which folders moving? If the update packages contain what we ship with the core at the new place, that will be there after unzip, so we just have to delete the stuff at the old place post-flight with the regular files and folder deletion, or am I missing something?

Update: Or does custom stuff like user.css has to be moved, too?

dgrammatiko · 2021-10-22T11:30:57Z

I can't see any change in the PR yet for new installations,

Will add them in a bit

or am I missing something?

Theoretically, users could have their own css/js/images in the template/{atum | cassiopeia}/{css/scss/js/images} thus we need to copy whatever exists there to media preflight and then remove those folders post flight. Or am I stretching this too far?

richard67 · 2021-10-22T11:33:11Z

Or am I stretching this too far?

@dgrammatiko I have to think about it a bit, but I think you are right. Regarding bilateral detail clarifications we can use Glip, if necessary, in order not to blow up number of comments here.

richard67 · 2021-10-22T11:37:40Z

@dgrammatiko Just a question for my understanding, sorry if that was already clarified elsewhere and I missed it: With templates which support child templates, will EVERY css and js for a template be below the media folder, also custom stuff added by the user, so below the regular template folder there won't be any (s)css and js anymore?

dgrammatiko · 2021-10-22T11:40:43Z

so below the regular template folder there won't be any (s)css and js anymore?

Yup, the folder media/templates/site/cassiopeia/css has all the super powers the templates/cassiopeia/css had (eg placing files there will override the css file, assuming the the API was used for inserting the tag in the first place). The same goes for js and images...

richard67 · 2021-10-22T11:42:06Z

@dgrammatiko Another thing is that if users did it right, they did not make any customizations (besides a user.css) in the template but made a copy of the template, especially in case of the frontend (Cassiopeia). I don't think we safely can update also such template copies to the new style, so maybe it should be documented somewhere or told in the release announcements something like "If you have made a copy of the Cassiopeia or the Atum template, ....".

dgrammatiko · 2021-10-22T11:45:17Z

I don't think we safely can update also such template copies to the new style

Oh no, template copies won't follow this pattern, this will be strictly only for the Cassiopeia and Atum. Their fork, if exists will still work (we don't touch anything in there) but will not converted to the new mode

Stuartemk · 2021-10-22T11:56:15Z

Hi everyone, @dgrammatiko With this new feature, it would be possible for the superuser to make a template for each device, for example for mobile, tablet, or desktop and thus avoid overloading everything in in a single css? And if it is possible to detect the type of device also to avoid loading several css and only serve the css corresponding to the requesting device? I thank in advance much the answer.

richard67 · 2021-10-22T11:57:17Z

@dgrammatiko GitHub shows some of the files as a deleted file and a new one instead of having been moved, e.g. the "_bootstrap.scss" file. The reason might be that you have moved the file and then made changes on the content of that file with the same commit. In such a case this happens. It can be avoided by making 2 separate commit, one for moving the file (e.g. with the "git mv" command or on GitHub's UI here, that works nice, too) and then the other commit for the changes inside.

It is not a big problem here, I think, because the history in Git is not completely lost, but I'm not sure if Git blame will work here for these files.

If @bembelimen is ok with it, leave it as it is and use my hint only for future PR's.

dgrammatiko · 2021-10-22T12:04:10Z

it would be possible for the superuser to make a template for each device, for example for mobile, tablet, or desktop and thus avoid overloading everything in in a single css?

This is already possible right now but if you use Bootstrap there's no way to have any gains as the minimum CSS served is already predefined. Will it be easier is a more interesting question and I think it will be (although I haven't explored this area of serving per device, so take this with a grain of salt)

richard67 · 2021-10-22T12:16:04Z

@dgrammatiko The build in npm is failing, see https://ci.joomla.org/joomla/joomla-cms/48041/1/7 :

Error: Can't find stylesheet to import.
@import "../../../../../../../../media/vendor/fontawesome-free/scss/fontawesome";
media/templates/administrator/atum/scss/vendor/fontawesome-free/fontawesome.scss 6:9  @import
installation/template/scss/template.scss 8:9                                          @import
installation/template/scss/template-rtl.scss 1:9                                      root stylesheet

richard67 · 2021-10-22T12:43:18Z

@dgrammatiko Regarding the update SQL I have a question. Users might have created new template styles for atum or cassiopeia. Shall we update the inheritable flag to 1 for all of them? If yes, then the update would be easy.

If you want I can make a PR for you, but I think you can do that yourself, too. Just add a new "4.1.0-2021-10-22.sql" for each database type with following one statement:

UPDATE `#__template_styles` SET `inheritable` = 1 WHERE `template` = 'atum' AND `client_id` = 1 OR `template` = 'cassiopeia' AND `client_id` = 0;

In the PostgreSQL file just replace the MySQL names quotes by ":

UPDATE "#__template_styles" SET "inheritable" = 1 WHERE "template" = 'atum' AND "client_id" = 1 OR "template" = 'cassiopeia' AND "client_id" = 0;

richard67 · 2021-10-22T12:46:32Z

@dgrammatiko For the files and folders deletion and copying present custom stuff I will check later. Normally we don't bother authors of PR's for 4.x with the files and folders deletion and do that just before the release. But in case of this PR it makes sense that we do it here together with the custom stuff copy thing.

dgrammatiko · 2021-10-22T12:49:16Z

@richard67 thanks, hopefully I didn't mess things 108250f

Yeah, the folders for this particular case need some custom handling as they might contain user created files and we don't want to delete these

richard67 · 2021-10-22T12:50:27Z

@dgrammatiko SQL stuff looks ok to me. NPM is happy, too, up to now.

richard67 · 2021-10-22T12:52:40Z

Yeah, the folders for this particular case need some custom handling as they might contain user created files and we don't want to delete these

@dgrammatiko Could you list which folders we should keep in case of present custom stuff and which we could and should delete because there should not be any user stuff?

dgrammatiko · 2021-10-22T13:03:02Z

So, as a case study let's take Atum (cassiopeia will be similar). The final structure of the template folder will be

And the media folder should have all the static folders (js,css,scss,images):

But we cannot simply delete the existing folders in the administrator/templates/atum as these might have user files, so what I was thinking as a safe upgrade path would be something like:

in the preflight we copy all these directories from administrator/templates/atum/{css,scss,js,images} to media/templates/administrator/atum/{css,scss,js,images}
Let the update overwrite as it should all the existing files in that folder media/templates/administrator/atum/{css,scss,js,images} with the ones from 4.1
In the post flight remove the dirs administrator/templates/atum/{css,scss,js,images}

The process should retain any user created static overrides (css,js,images)

richard67 · 2021-10-22T13:13:29Z

@dgrammatiko Alternatively we don't to anything pre-flight, and post-flight before we delete files and folders we go through these specific folders and copy files which exist in the old but not in the new folder.

Both will make problems if there are files which exist in the old folder but not in the new folder and which are not custom stuff. That would happen e.g. when later after this PR has been merged changes will be done in (s)css or js for these 2 templates which remove some css or js or image files, so you have them in the old but not in the new folder when updating from 4.0.x to 4.1.0 which will include your PR.

I think we should use a hard-coded list of the core old stuff as it is now and which we use as exclusion list for that copying so we only copy stuff which really has been added by the user and not old obsolete core stuff.

Update: Forget the problem. That will be handled with the regular files and folders deletion.

dgrammatiko · 2021-10-22T13:24:36Z

Alternatively we don't to anything pre-flight, and post-flight before we delete files and folders we go through these specific folders and copy files which exist in the old but not in the new folder.

That would work as well

Both will make problems if there are files which exist in the old folder but not in the new folder and which are not custom stuff.

If the files are redundant and somehow still exist in the new path obviously is not perfect but as these are static files and mostly few kb that won't create any issues (imho). Keeping a registry with the files is a maintainers nightmare thus my naive approach copy/update/delete. Anyways, I'm ok with any solution as long as users won't start blaming me that I deleted their precious files...

richard67 · 2021-10-22T13:25:25Z

@dgrammatiko Forget about the problem, I have just updated my previous comment.

richard67 · 2021-10-22T13:39:37Z

@dgrammatiko Ideally we would put the copying of custom js and css and image files to here between the files removal and the folders removal: https://github.com/joomla/joomla-cms/blob/4.1-dev/administrator/components/com_admin/script.php#L7395

The cool thing if we do that there is that we will have removed all core files at the old place at this point so what is left in the relevant old folders will be custom stuff, and the folder deletion later will delete that at the old place after we have copied it because that how "Folder::delete" works, if deletes all sub-folders and files.

richard67 · 2021-10-22T13:59:01Z

@dgrammatiko Here is a list of all folders and sub-folders which would be deleted. Do you think we should keep any custom file in all of these? Or are there certain sub-folders where we can exclude this, e.g. "vendor"?

administrator/templates/atum/css
administrator/templates/atum/css/system
administrator/templates/atum/css/system/searchtools
administrator/templates/atum/css/vendor
administrator/templates/atum/css/vendor/awesomplete
administrator/templates/atum/css/vendor/choicesjs
administrator/templates/atum/css/vendor/fontawesome-free
administrator/templates/atum/css/vendor/joomla-custom-elements
administrator/templates/atum/css/vendor/minicolors
administrator/templates/atum/images
administrator/templates/atum/images/logos
administrator/templates/atum/scss
administrator/templates/atum/scss/blocks
administrator/templates/atum/scss/pages
administrator/templates/atum/scss/system
administrator/templates/atum/scss/system/searchtools
administrator/templates/atum/scss/vendor
administrator/templates/atum/scss/vendor/awesomplete
administrator/templates/atum/scss/vendor/bootstrap
administrator/templates/atum/scss/vendor/choicesjs
administrator/templates/atum/scss/vendor/fontawesome-free
administrator/templates/atum/scss/vendor/joomla-custom-elements
administrator/templates/atum/scss/vendor/minicolors
templates/cassiopeia/css
templates/cassiopeia/css/global
templates/cassiopeia/css/system
templates/cassiopeia/css/system/searchtools
templates/cassiopeia/css/vendor
templates/cassiopeia/css/vendor/choicesjs
templates/cassiopeia/css/vendor/joomla-custom-elements
templates/cassiopeia/images
templates/cassiopeia/js
templates/cassiopeia/scss
templates/cassiopeia/scss/blocks
templates/cassiopeia/scss/global
templates/cassiopeia/scss/system
templates/cassiopeia/scss/system/searchtools
templates/cassiopeia/scss/tools
templates/cassiopeia/scss/tools/functions
templates/cassiopeia/scss/tools/variables
templates/cassiopeia/scss/vendor
templates/cassiopeia/scss/vendor/bootstrap
templates/cassiopeia/scss/vendor/choicesjs
templates/cassiopeia/scss/vendor/joomla-custom-elements
templates/cassiopeia/scss/vendor/metismenu

I think we could at least exclude that someone has added custom stuff to one of the present vendor folders, like "(s)css/vendor/joomla-custom-elements".

richard67 · 2021-10-22T14:23:10Z

@dgrammatiko And what shall we do if someone has created a custom sub-folder, e.g. "templates/cassiopeia/css/foobar"? I guess we have to copy that, too.

dgrammatiko · 2021-10-22T14:28:30Z

And what shall we do if someone has created a custom sub-folder, e.g. "templates/cassiopeia/css/foobar"? I guess we have to copy that, too.

Yes, it's not only files but could be subfolders, thus I thought it would be easier to copy things as they are from the template/... to media/... and then do the update (the process will replace all the core files) so everything would be as if they were updating any patch version.

richard67 · 2021-10-22T14:33:34Z

thus I thought it would be easier to copy things as they are from the template/... to media/... and then do the update (the process will replace all the core files)

@dgrammatiko Not sure about that, because of ongoing parallel development of 4.0.x and 4.1 it might be that we copy files which became obsolete during 4.0.x development and so would be deleted on update, but the files and folder deletion would delete them at the old but not at the new place. We would have to change the way how we generate these lists to handle that right. But I am still thinking about it.

If you want you can already have a PR from me with the files and folders deletion (but without the copying custom stuff). I have that ready here.

dgrammatiko · 2021-10-22T14:41:21Z

But I am still thinking about it.

I think we have some time here as this PR is depending on another PR which I have not even started yet 😀

Quy · 2021-11-29T17:09:38Z

Applied the other PR and still cannot create overrides. Testing locally with WampServer on Windows 10.

Quy · 2021-11-29T17:10:30Z

No errors in Console and PHP error log.

richard67 · 2021-11-29T17:14:27Z

@Quy How did you apply the patches?

If you are using patchtester, maybe that has a problem?

I had updated a clean 4.1-dev to the branch of this PR using the custom URL created by drone, and the same I did with the other PR (and then without this one here) on a clean 4.1-dev. After that I had tested both together by creating a new branch based on 4.1-dev, merging both branches of both PRs into that one and made a new installation using that branch (with composer install and nmp ci of course).

Quy · 2021-11-29T17:20:41Z

Using the prebuilt package and Joomla update to manually install the other PR.

richard67 · 2021-11-29T17:25:20Z

Hmm, maybe an problem on Windows? I've tested with a Linux server.

dgrammatiko · 2021-11-29T17:37:40Z

@Quy it seems to work fine here

Also, the changes in this PR have nothing to do with the code in com_content that handles the creation of the overrides and on top of that the layout overrides didn't had any changes here (I mean the HTML directory wasn't moved). Can you try the same in a vanilla 4.1 installation? Maybe there's something else affecting permissions or there's a bug that you just discovered. My point is that this PR should not affect in any shape or form the functionality for creating layout overrides...

Quy · 2021-11-29T17:46:43Z

Tested fine with the 4.1.0 nightly build from today. https://developer.joomla.org/nightly-builds.html

dgrammatiko · 2021-11-29T17:49:55Z

@Quy are there any JS errors? Can you share a video of the process, I'm really curious why this is failing (the code changes here are unrelated to the process)

Quy · 2021-11-29T17:57:39Z

No JS/PHP errors.

create-overrides.mp4

dgrammatiko · 2021-11-29T18:06:46Z

@Quy applying only this PR you shouldn't have any preview images in the templates list. I'm guessing there's something wrong with the packages (?). Once again this PR is NOT touching at all the com_templates which is the extension that is misbehaving here...

Quy · 2021-11-29T18:10:05Z

This installation includes the other PR. I get the same error with this PR by itself. I will try to troubleshoot later and report any findings.

Quy · 2021-11-29T19:38:24Z

The $override parameter to createOverride is empty in this PR but not in the 4.1 nightly build.

dgrammatiko · 2021-11-29T19:48:46Z

@Quy Can you clear your browser's cache reload the page and send me the list of js files loaded?

Quy · 2021-11-29T19:55:59Z

Hopefully this is it of the Create Overrides page.

dgrammatiko · 2021-11-29T20:03:28Z

@Quy try this:

open the browser inspector
right click on the module override link and inpect
check/copy the href

But ONCE AGAIN: this is unrelated to what this PR is doing!!!. This PR just moves some static files, nothing to do with Overrides!!!

Quy · 2021-11-29T20:09:07Z

http://localhost/Joomla_4.1.0-dev+pr.35874-Development-Full_Package/administrator/index.php?option=com_templates&view=template&task=template.overrides&folder=Qzpcd2FtcDY0XHd3d1xKb29tbGFfNC4xLjAtZGV2K3ByLjM1ODc0LURldmVsb3BtZW50LUZ1bGxfUGFja2FnZVxtb2R1bGVzXG1vZF9hcnRpY2xlc19hcmNoaXZl&id=218&file=aG9tZQ&33c1bf54cc8ea6301b9a91a7b4e85521=1

Quy · 2021-11-29T20:15:53Z

But ONCE AGAIN: this is unrelated to what this PR is doing!!!. This PR just moves some static files, nothing to do with Overrides!!!

I don't want to waste any more of your time. Let's get this in and worry about it later if it persists. Thank you!

dgrammatiko · 2021-11-29T20:23:50Z

So, the part folder=... is the base64 encoded part of the override that you asked the component to create
You said that the $override (i guess in the controller) is empty. But if that is failing is due to this:

joomla-cms/administrator/components/com_templates/src/Controller/TemplateController.php

Line 406 in ee526e2

$override = (string) InputFilter::getInstance(

And this part wasn't touched by this PR thus my surprise here. This shouldn't fail with this PR...

Quy · 2021-11-29T22:12:30Z

The + is the folder Joomla_4.1.0-dev+pr.35874-Development-Full_Package caused the issue and unrelated to this PR. Sorry for the false alarm.

dgrammatiko · 2021-11-29T22:15:49Z

Wow, that's actually a very good catch!!
@HLeithner I think you need to rename the packages, it seems the + will be problematic on Windows for some cases.

bembelimen · 2021-11-30T02:35:20Z

Thx, awesome job @dgrammatiko and thanks for all the feedbacks

dgrammatiko · 2021-11-30T09:58:13Z

Thanks, @bembelimen also thanks to everyone contributing and testing here, especially @richard67 for the update patches!!

HLeithner · 2021-11-30T13:16:28Z

Wow, that's actually a very good catch!! @HLeithner I think you need to rename the packages, it seems the + will be problematic on Windows for some cases.

know your environment ;-)

dgrammatiko requested a review from chmst as a code owner October 22, 2021 11:18

joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.1-dev labels Oct 22, 2021

richard67 mentioned this pull request Oct 22, 2021

[CMS PR 35874] Update deleted files and folders dgrammatiko/joomla-cms#4

Merged

alikon added the Documentation Required label Nov 29, 2021

Merge branch '4.1-dev' into 4.1-dev-child-templates

edd3015

bembelimen merged commit 579e20e into joomla:4.1-dev Nov 30, 2021

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 30, 2021

dgrammatiko deleted the 4.1-dev-child-templates branch November 30, 2021 09:56

dgrammatiko mentioned this pull request Jan 5, 2022

Assets not loaded after upgrading from 4.0 to 4.1 #36567

Closed

richard67 mentioned this pull request Mar 30, 2023

[5.0] Remove child template conversion of core templates from script.php #40253

Merged

2 tasks

4.1 Child templates 1/2 #35874

4.1 Child templates 1/2 #35874

Conversation

dgrammatiko commented Oct 22, 2021 • edited Loading

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

richard67 commented Oct 22, 2021 • edited Loading

dgrammatiko commented Oct 22, 2021

richard67 commented Oct 22, 2021

richard67 commented Oct 22, 2021

dgrammatiko commented Oct 22, 2021 • edited Loading

richard67 commented Oct 22, 2021

dgrammatiko commented Oct 22, 2021

Stuartemk commented Oct 22, 2021

richard67 commented Oct 22, 2021

dgrammatiko commented Oct 22, 2021

richard67 commented Oct 22, 2021

richard67 commented Oct 22, 2021 • edited Loading

richard67 commented Oct 22, 2021

dgrammatiko commented Oct 22, 2021

richard67 commented Oct 22, 2021

richard67 commented Oct 22, 2021

dgrammatiko commented Oct 22, 2021

richard67 commented Oct 22, 2021 • edited Loading

dgrammatiko commented Oct 22, 2021

richard67 commented Oct 22, 2021

richard67 commented Oct 22, 2021 • edited Loading

richard67 commented Oct 22, 2021

richard67 commented Oct 22, 2021

dgrammatiko commented Oct 22, 2021

richard67 commented Oct 22, 2021

dgrammatiko commented Oct 22, 2021 • edited Loading

Quy commented Nov 29, 2021

Quy commented Nov 29, 2021

richard67 commented Nov 29, 2021

Quy commented Nov 29, 2021

richard67 commented Nov 29, 2021

dgrammatiko commented Nov 29, 2021

Quy commented Nov 29, 2021

dgrammatiko commented Nov 29, 2021

Quy commented Nov 29, 2021

dgrammatiko commented Nov 29, 2021

Quy commented Nov 29, 2021

Quy commented Nov 29, 2021 • edited Loading

dgrammatiko commented Nov 29, 2021

Quy commented Nov 29, 2021

dgrammatiko commented Nov 29, 2021

Quy commented Nov 29, 2021

Quy commented Nov 29, 2021

dgrammatiko commented Nov 29, 2021

Quy commented Nov 29, 2021

dgrammatiko commented Nov 29, 2021

bembelimen commented Nov 30, 2021

dgrammatiko commented Nov 30, 2021

HLeithner commented Nov 30, 2021

dgrammatiko commented Oct 22, 2021 •

edited

Loading

richard67 commented Oct 22, 2021 •

edited

Loading

dgrammatiko commented Oct 22, 2021 •

edited

Loading

richard67 commented Oct 22, 2021 •

edited

Loading

richard67 commented Oct 22, 2021 •

edited

Loading

richard67 commented Oct 22, 2021 •

edited

Loading

dgrammatiko commented Oct 22, 2021 •

edited

Loading

Quy commented Nov 29, 2021 •

edited

Loading