? ? Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
16 Aug 2020

Pull Request for Issue # .

Summary of Changes

Cascade correctly for any parent/children template

Testing Instructions

Checkout this PR or download the installable at the end of the GitHub page

Check that both Cassiopeia and Atum have correct favicons
Install the templates from the PR #30192 (links in the description) and check that both parent and child templates have correct favicon (check the url)

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

avatar dgrammatiko dgrammatiko - open - 16 Aug 2020
avatar dgrammatiko dgrammatiko - change - 16 Aug 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 16 Aug 2020
Category Libraries
avatar dgrammatiko dgrammatiko - change - 16 Aug 2020
Labels Added: ?
avatar Quy
Quy - comment - 17 Aug 2020

Object not found. Have not installed templates from the other PR yet.

Before PR:
<link href="/administrator/templates/atum/favicon.ico" rel="shortcut icon" type="image/vnd.microsoft.icon">

After PR:
<link href="/templates/atum/favicon.ico" rel="shortcut icon" type="image/vnd.microsoft.icon">

avatar dgrammatiko
dgrammatiko - comment - 17 Aug 2020

@Quy you're right the logic was wrong, fixed with 93d707d

avatar Quy Quy - test_item - 20 Aug 2020 - Tested successfully
avatar Quy
Quy - comment - 20 Aug 2020

I have tested this item successfully on a15b321


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30388.

avatar laoneo laoneo - test_item - 21 Aug 2020 - Tested successfully
avatar laoneo
laoneo - comment - 21 Aug 2020

I have tested this item successfully on a15b321


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30388.

avatar Quy Quy - change - 21 Aug 2020
Status Pending Ready to Commit
avatar Quy
Quy - comment - 21 Aug 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30388.

avatar SharkyKZ
SharkyKZ - comment - 22 Aug 2020

FWIW, using media folder for template assets is a mistake.

avatar dgrammatiko
dgrammatiko - comment - 22 Aug 2020

FWIW, using media folder for template assets is a mistake.

Can you elaborate why?

avatar laoneo
laoneo - comment - 23 Aug 2020

Because CSS and JS files are not media.

avatar dgrammatiko
dgrammatiko - comment - 23 Aug 2020

Because CSS and JS files are not media.

AFAIK the majority (if not all) of the contents of the media folder is either css or js or images therefore they shouldn't be stored on that folder? Sorry I'm not getting it, for Joomla css, js, images, fonts, et al are static files. To my understanding media is what in other platforms is called assets, eg stored files needed for the rendering of the page

avatar SharkyKZ
SharkyKZ - comment - 24 Aug 2020

It's not about the folder name. There are reasons why template assets are treated differently than assets of other extensions. For instance, they should be final and non-overridable. If they're placed in media folder, they become overridable. The magic code in Joomla\CMS\HTML\HTMLHelper looks for overrides in template directory which adds some unneeded file lookups. Before #30192 something like this could have made sense. I.e. having all template assets (including overrides) in media folder and user-defined overrides in template folder. But now there are also child template folders - both in media and template directory which just makes things slow and confusing.

avatar dgrammatiko
dgrammatiko - comment - 24 Aug 2020

There are few misconceptions here:

For instance, they should be final and non-overridable. If they're placed in media folder, they become overridable.

That's wrong. The new mode templates (eg the ones that support Childs) have both their assets and the overriding folders on the media template therefore folders line css, js and images are indeed final and non-overridable (src === override)

looks for overrides in template directory which adds some unneeded file lookups.

The only extra lookups are for the child templates. There won't be a lookup eg for a current template in any of the media/templates/... Actually in order any of this to work the template needs to opt in with an <inheritable>1</inheritable>

But now there are also child template folders - both in media and template directory which just makes things slow and confusing.

Let me write this one more time: Templates that support child templates use ONLY the media folder for their OWN assets but ALSO for the OVERRIDES. Static assets for those templates (and their Childs) EXISTS ONLY IN THE MEDIA FOLDER

avatar SharkyKZ
SharkyKZ - comment - 24 Aug 2020

OK, you're right about performance. Not a fan of these differences between normal and inheritable templates though.

Are you going to expose media folder in com_templates? Template assets should be editable through Template Manager. I didn't see anything related to this in #30359.

avatar dgrammatiko
dgrammatiko - comment - 24 Aug 2020

Template assets should be editable through Template Manager.

Yes that's the plan. Everything will be as before with the slight difference that ALL static assets will live in the media folder. UX should be the same from a user's GUI perspective. For devs the only hurdle is that they have to slightly adjust their packaging so particular files/folders end up in the media folder. I know that people hate changes so you have every right to hate/blame me for this

avatar wilsonge wilsonge - change - 4 Sep 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-09-04 23:03:46
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 4 Sep 2020
avatar wilsonge wilsonge - merge - 4 Sep 2020
avatar wilsonge
wilsonge - comment - 4 Sep 2020

Thanks!

Add a Comment

Login with GitHub to post a comment