? ? Pending

User tests: Successful: Unsuccessful:

avatar Bakual
Bakual
22 Sep 2020

Summary of Changes

The error.php referenced the chrome style "xhtml" which no longer exists in J4. I've changed it to "html5" which is also used in the "index.php" for the same position "banner". So that one should be a no-brainer.

I've also renamed the "default" chrome layout to "card". The latter was already referenced in the template file in several positions. "default" was only used as chrome style in the sidebars.
Since the name "default" is misleading (there default chrome is defined by the jdoc call, not by the layout name), I've changed it to "card" which is also a good description of what the chrome does.

Testing Instructions

Test various module positions and check the appearance of the modules in Cassiopeia.

Actual result BEFORE applying this Pull Request

Modules in eg position "top-b" render without any chrome. In sidebar, the "default" chrome is used
image

Expected result AFTER applying this Pull Request

All modules are rendered with the expected chrome. "top-b" uses "card", the sidebar uses also "card" and looks exactly the same as before this PR.
image

Documentation Changes Required

None

Topic to discuss

I noticed that eg the position "top-a" uses the style "cardGrey" whole "top-b" uses "card".
The same also applies to bottom positions.
The only difference between the two chromes are the added module class "card-grey" which result in a slightly grey background of the module.
Imho, we don't need the "cardGrey" chrome. The exact same look can be achieved by simply adding "card-grey" as module class suffix in the module options. See my screenshot above where the module "Top-b Card with Suffix card-grey" looks exactly the same as the "Top-a CardGrey" module.
I'd suggest to just drop the "cardGrey" chrome and change the four positions where it is used to "card". If accepted I would add that to this PR.

avatar Bakual Bakual - open - 22 Sep 2020
avatar Bakual Bakual - change - 22 Sep 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 22 Sep 2020
Category Front End Templates (site)
avatar richard67
richard67 - comment - 22 Sep 2020

Is this an alternative to PR #30716 ? To me they seem to excluce each other.

avatar Scrabble96
Scrabble96 - comment - 22 Sep 2020

Is this an alternative to PR #30716 ? To me they seem to excluce each other.

No, because the main point of PR #30716 is to move the mod-id into the surrounding container so that it is displayed whether the title (header) is displayed or not.

avatar richard67
richard67 - comment - 22 Sep 2020

So I understand right that both PR's would work well together?

avatar Scrabble96
Scrabble96 - comment - 22 Sep 2020

So I understand right that both PR's would work well together?

Yes, but if so, then only one chrome is required - the 'default' one (renamed as on this PR as 'card') and drop the 'cardGrey' one.

avatar richard67
richard67 - comment - 22 Sep 2020

@Scrabble96 Depending on which PR will be merged first, the other one then might have merge conflicts after that. If that will be yours and I am available, I can help with solving them, if necessary.

avatar Scrabble96
Scrabble96 - comment - 22 Sep 2020

@Scrabble96 Depending on which PR will be merged first, the other one then might have merge conflicts after that. If that will be yours and I am available, I can help with solving them, if necessary.

I believe that if my PR (#30716) was merged first, there shouldn't be any conflicts. Can we ask for this PR to be on hold until #30716 is merged?

avatar richard67
richard67 - comment - 22 Sep 2020

Can we ask for this PR to be on hold until #30716 is merged?

Not really, but I promise to help with the conflicts if it is merged first.

avatar Scrabble96
Scrabble96 - comment - 22 Sep 2020

Can we ask for this PR to be on hold until #30716 is merged?

Not really, but I promise to help with the conflicts if it is merged first.

Thank you. Mind you, if this PR is merged first, would it be easier for me to start again (!) with another PR and apply my modified default.php chrome code to the newly renamed 'card.php' chrome as this PR (#30729) doesn't involve any changes to the chrome code.

avatar Bakual
Bakual - comment - 22 Sep 2020

Actually, I think Git should be able to handle the changes. But for the other PR it's a simple rename of the file as I didn't touch the contents. So that would be easy to be solved.
If the other PR is merged first, I can just update my branch and it will be fine as well.

And yes, both PRs are valid and do different things. They don't exclude eachother.

avatar richard67
richard67 - comment - 22 Sep 2020

Imho, we don't need the "cardGrey" chrome. The exact same look can be achieved by simply adding "card-grey" as module class suffix in the module options. See my screenshot above where the module "Top-b Card with Suffix card-grey" looks exactly the same as the "Top-a CardGrey" module.
I'd suggest to just drop the "cardGrey" chrome and change the four positions where it is used to "card". If accepted I would add that to this PR.

Makes sense to me.

avatar richard67
richard67 - comment - 22 Sep 2020

But you should not call it "suffix" anymore. In J4 it has changed to a class, see also the changed field label "Module Class" instead of "Module Class Suffix" in backend of J4.

avatar richard67 richard67 - test_item - 22 Sep 2020 - Tested successfully
avatar richard67
richard67 - comment - 22 Sep 2020

I have tested this item successfully on 30a6599

Works as described.

In addition tested that adding module class "card-grey" in the parameters of a module which uses module style "Card" has the same result as using module style "CardGrey" and no module class.


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

avatar jwaisner jwaisner - test_item - 22 Sep 2020 - Tested successfully
avatar jwaisner
jwaisner - comment - 22 Sep 2020

I have tested this item successfully on 30a6599


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

avatar jwaisner jwaisner - change - 22 Sep 2020
Status Pending Ready to Commit
avatar jwaisner
jwaisner - comment - 22 Sep 2020

RTC


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

avatar richard67 richard67 - change - 22 Sep 2020
Labels Added: ? ?
avatar Bakual
Bakual - comment - 22 Sep 2020

But you should not call it "suffix" anymore. In J4 it has changed to a class, see also the changed field label "Module Class" instead of "Module Class Suffix" in backend of J4.

Ah sure, I remember that discussion. The parameter however in code is still moduleclass_sfx 😄

As this PR is already RTC now, I'm going to do the removal of cardGrey in a separate one.

avatar infograf768 infograf768 - change - 23 Sep 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-09-23 05:50:12
Closed_By infograf768
Labels
avatar infograf768 infograf768 - close - 23 Sep 2020
avatar infograf768 infograf768 - merge - 23 Sep 2020
avatar infograf768
infograf768 - comment - 23 Sep 2020

Tks.

avatar infograf768
infograf768 - comment - 23 Sep 2020

@Bakual Please do not forget

I'm going to do the removal of cardGrey in a separate one.

avatar Bakual
Bakual - comment - 23 Sep 2020

@Bakual Please do not forget

I'm going to do the removal of cardGrey in a separate one.

Already done yesterday: #30734 😄
Thanks for merging!

avatar Scrabble96
Scrabble96 - comment - 23 Sep 2020

But you should not call it "suffix" anymore. In J4 it has changed to a class, see also the changed field label "Module Class" instead of "Module Class Suffix" in backend of J4.

Ah sure, I remember that discussion. The parameter however in code is still moduleclass_sfx 😄

I agree with @Bakual, after all, the definition of suffix in this case is to append something, which is what happens when you type a class name into the 'Module Class' field in the Advanced tab.

avatar richard67
richard67 - comment - 23 Sep 2020

I agree with @Bakual, after all, the definition of suffix in this case is to append something, which is what happens when you type a class name into the 'Module Class' field in the Advanced tab.

You are wrong!!! The code has been changes so no suffix is appended, and only the variable name kept the old, now misleading name.

Check the code: In J3, where it is a suffix, the value is directly appended to to the class without a space, and when using a space at the beginning of a suffix, it became a class vaklue and not just a suffix appended to a class value in the HTML markup.

In J4, there is a space added between the Module Class value and the class value before, so you can't use that value anymore to append something to the class, and don't need a space anymore at the beginning of the value to make it be a class.

That's why in backend of J4 the field label has been changed to "Module Class", and that is what it does!!!

avatar richard67
richard67 - comment - 23 Sep 2020

@Scrabble96 See my previous comment.

avatar Scrabble96
Scrabble96 - comment - 23 Sep 2020

In J4, there is a space added between the Module Class value and the class value before, so you can't use that value anymore to append something to the class, and don't need a space anymore at the beginning of the value to make it be a class.

That's why in backend of J4 the field label has been changed to "Module Class", and that is what it does!!!

Sorry again. I have not yet had time to read through all the merged PRs for Cassiopeia and, as there are no tooltips in the module setup, I was not aware of this change, although I can see the space in the mod chrome. I have been adding a space before the class name as in J3 - unknowingly creating two spaces - but Joomla/browsers remove the extra space before rendering it so on inspecting the code in the browser there is apparently no difference.

avatar richard67
richard67 - comment - 23 Sep 2020

No problem. I just wanted to explain the change so it is clear. Hopefully our documentation for J4 will handle that.

Add a Comment

Login with GitHub to post a comment