[4.0] Cleanup cassiopeia chromes and rename "default" to "card" by Bakual · Pull Request #30729 · joomla/joomla-cms

Bakual · 2020-09-22T10:43:17Z

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

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.

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.

…at all.

chmst · 2020-09-22T11:14:25Z

see also https://issues.joomla.org/tracker/joomla-cms/30716

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

richard67 · 2020-09-22T11:15:41Z

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

Scrabble96 · 2020-09-22T11:40:24Z

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.

richard67 · 2020-09-22T11:41:46Z

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

Scrabble96 · 2020-09-22T11:42:50Z

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.

richard67 · 2020-09-22T11:46:23Z

@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.

Scrabble96 · 2020-09-22T12:18:02Z

@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?

richard67 · 2020-09-22T12:20:07Z

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.

Scrabble96 · 2020-09-22T12:34:13Z

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.

Bakual · 2020-09-22T13:43:55Z

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.

richard67 · 2020-09-22T19:18:08Z

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.

richard67 · 2020-09-22T19:22:28Z

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.

richard67 · 2020-09-22T20:26:12Z

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.}

jwaisner · 2020-09-22T20:34:47Z

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.}

jwaisner · 2020-09-22T20:35:01Z

RTC

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

Bakual · 2020-09-22T20:47:45Z

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.

infograf768 · 2020-09-23T05:50:20Z

Tks.

infograf768 · 2020-09-23T05:51:25Z

@Bakual Please do not forget

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

Bakual · 2020-09-23T06:16:44Z

@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!

Scrabble96 · 2020-09-23T06:52:50Z

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.

richard67 · 2020-09-23T06:57:15Z

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!!!

richard67 · 2020-09-23T06:57:47Z

@Scrabble96 See my previous comment.

Scrabble96 · 2020-09-23T08:56:35Z

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.

richard67 · 2020-09-23T08:59:41Z

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

* Joomla/4.0-dev: (844 commits) [4.0] Template layout select (joomla#30772) [4.0][CLI] com_finder use console command (joomla#30768) [4.0] Modifying com_actionlogs string (joomla#30758) [4.0] Fancy selectbox fix (joomla#30739) [4.0] Add missing Table Caption (joomla#30763) [4.0] Wrap all buttons in btn-group to improve styling (joomla#30761) [4.0] Cassiopeia missing string (joomla#30765) Improve batch text (joomla#28447) Fix icons not displaying (joomla#30749) Remove the chrome "cardGrey". The same effect can be achieved by using the module class "card-grey" with the "card" chrome (joomla#30734) Remove obsolete html code (joomla#30737) [4.0] Check out improvements related to nullable columns (joomla#30747) Removing card.scss overrides as we don't use those classes Use renderField() method to render fields (joomla#30738) [4.0] spelling (joomla#30742) [4.0] Remove use of ReflectionParameter::getClass() (joomla#30581) [4.0] Fix saving images (joomla#30730) [4.0] Cleanup cassiopeia chromes and rename "default" to "card" (joomla#30729) Fix hiddenLabel and add hiddenLegend form attribute (joomla#29710) [4.0] Fix Debug plugin to display query parameters (joomla#30717) ...

…la#30729) * Renaming the "default" chrome to "card" as it isn't a default chrome at all. * There is no "xhtml" chrome anymore

Thomas Hunziker added 2 commits September 22, 2020 10:27

Renaming the "default" chrome to "card" as it isn't a default chrome …

c9fd806

…at all.

There is no "xhtml" chrome anymore

30a6599

joomla-cms-bot added the PR-4.0-dev label Sep 22, 2020

joomla-cms-bot removed the PR-4.0-dev label Sep 22, 2020

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 22, 2020

richard67 added the PR-4.0-dev label Sep 22, 2020

Bakual mentioned this pull request Sep 22, 2020

[4.0] Remove "cardGrey" chrome #30734

Merged

infograf768 merged commit 7a91649 into joomla:4.0-dev Sep 23, 2020

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 23, 2020

infograf768 added this to the Joomla 4.0 milestone Sep 23, 2020

Bakual deleted the 4_CleanupCassiopeiaChromes branch September 23, 2020 06:16

richard67 mentioned this pull request Sep 23, 2020

[4.0] Cassiopeia - update card mod chrome #30716

Closed

Bakual mentioned this pull request Sep 25, 2020

[4.0] top-b and bottom-b missing chromes so do not display title when set to 'Show' #26095

Closed

Uh oh!

Conversation

Bakual commented Sep 22, 2020

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

Topic to discuss

Uh oh!

chmst commented Sep 22, 2020

Uh oh!

richard67 commented Sep 22, 2020

Uh oh!

Scrabble96 commented Sep 22, 2020

Uh oh!

richard67 commented Sep 22, 2020

Uh oh!

Scrabble96 commented Sep 22, 2020

Uh oh!

richard67 commented Sep 22, 2020

Uh oh!

Scrabble96 commented Sep 22, 2020

Uh oh!

richard67 commented Sep 22, 2020

Uh oh!

Scrabble96 commented Sep 22, 2020

Uh oh!

Bakual commented Sep 22, 2020

Uh oh!

richard67 commented Sep 22, 2020

Uh oh!

richard67 commented Sep 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richard67 commented Sep 22, 2020

Uh oh!

jwaisner commented Sep 22, 2020

Uh oh!

jwaisner commented Sep 22, 2020

Uh oh!

Bakual commented Sep 22, 2020

Uh oh!

infograf768 commented Sep 23, 2020

Uh oh!

infograf768 commented Sep 23, 2020

Uh oh!

Bakual commented Sep 23, 2020

Uh oh!

Scrabble96 commented Sep 23, 2020

Uh oh!

richard67 commented Sep 23, 2020

Uh oh!

richard67 commented Sep 23, 2020

Uh oh!

Scrabble96 commented Sep 23, 2020

Uh oh!

richard67 commented Sep 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

richard67 commented Sep 22, 2020 •

edited

Loading