[4.0] Cassiopeia - update card mod chrome

[Scrabble96]

Pull Request for Issue #30678 (second attempt) .

Summary of Changes

Updated html > layouts > chromes > card.php (renamed from 'default', see #30729) to display mod-id as the containing element's (address, div, section, etc) #id instead of just in the title so that the id displays whether the title is shown or not.

Testing Instructions

1. Create a new module if none already exists and in the Advanced tab select Cassiopeia's 'card' chrome from the list in 'Module Style' if not already applied.
2. On the front end, use the browser's inspector to inspect this module with title shown and then again where title is not shown
   Result: See section "Actual result BEFORE applying this Pull Request" below.
3. Apply the patch of this PR
4. Repeat steps 1. and 2.
   See section "Actual result AFTER applying this Pull Request" below.

Actual result BEFORE applying this Pull Request

module #id only appears if title is shown

Expected result AFTER applying this Pull Request

1. module #id always applied to Cassiopeia modules using Cassiopeia 'card' chrome
2. id can now be used for anchors, etc if 1. above applies.

No visible difference on front end for website visitors

Documentation Changes Required

Note to highlight change

[richard67]

@Scrabble96 You should change your editor's settings so it shows spaces and tabs, so you can see what kind of white space is used.

[Bakual]

Honestly, I don't think we need a new chrome "noTitle". Hiding a title is possible with each module chrome already. There is an option for just that.

[Scrabble96]

Honestly, I don't think we need a new chrome "noTitle". Hiding a title is possible with each module chrome already. There is an option for just that.

In that case, I don't see the reason for having any of the modules as "style=none" because:
a. those modules won't have an id added.
b. you can't add a module class suffix (or if you do, it's ignored). This could be confusing (as it was for me, initially) for someone who does not have the user access level to edit the source code but wants to add a known css class to a module.

Perhaps all the modules currently given "style=none" in the index.php should be "style=default" which would avoid the need for the new 'noTitle' chrome.

[Bakual]

In that case, I don't see the reason for having any of the modules as "style=none" because:
a. those modules won't have an id added.
b. you can't add a module class suffix (or if you do, it's ignored). This could be confusing (as it was for me, initially) for someone who does not have the user access level to edit the source code but wants to add a known css class to a module.

That's exactly the purpose of "none". To have only the module output without any chrome around it.
If you need the id or suffix, you can easily change the module style (= chrome) to another one of your choice in the module options.
The chrome specified in the index.php is just the default chrome to be used. It can be overriden in the module options.
And yes, "default" is a bad and misleading choice as a chrome name.

[Scrabble96]

That's exactly the purpose of "none". To have only the module output without any chrome around it.
If you need the id or suffix, you can easily change the module style (= chrome) to another one of your choice in the module options.
Yes, of course. I haven't used that way as I've always other methods, so I forgot. Sorry.
The chrome specified in the index.php is just the default chrome to be used. It can be overriden in the module options.
And yes, "default" is a bad and misleading choice as a chrome name.

Perhaps 'default' should be renamed as 'unstyled'.

[Bakual]

Perhaps 'default' should be renamed as 'unstyled'.

Nah, unstyled is wrong as well, as there certainly are styles applied. Actually "default" applies a "card" styling. And I don't even see the difference to "cardGrey".
But I leave that to others who have more experience with design.

[Scrabble96]

Nah, unstyled is wrong as well, as there certainly are styles applied. Actually "default" applies a "card" styling. And I don't even see the difference to "cardGrey".

"cardGrey" has a light grey background in the class "card-header". "card-header" is assigned to the default.php as well, but doesn't seem to be applied. I can't work out why. I can't see any other differences.

But I leave that to others who have more experience with design.

[Scrabble96]

Ok. What's wrong with my code this time?
Edit: I've lost the aria-label and aria-labelledby
I'll put them back.

[richard67]

Ok. What's wrong with my code this time?

@Scrabble96 You can check that yourself. At the bottom of the PR where the tests are shown, use the "Details" link at the right side or a failed test. In this case drone failed. The "Details" link brings you to a page where on the left hand side is a list of test steps, failed ones in red. In your case now it is "PHPCS", which means PHP code style. Click this and you get on the right hand side the details on this page: https://ci.joomla.org/joomla/joomla-cms/35590/1/8. You will see that the problem is that you use spaces instead of tabs to indent certain lines.

So please follow my advise from above, set up your editor so you can see tabs and spaces and so can distinguish them. And in addition switch off any automatic indent or formatting in your editor (or IDE) settings, or adjust them in a way so that they meet the code style standards documented e.g. here for PHP: https://developer.joomla.org/coding-standards/php-code.html.

[Scrabble96]

So please follow my advise from above, set up your editor so you can see tabs and spaces and so can distinguish them. And in addition switch off any automatic indent or formatting in your editor (or IDE) settings, or adjust them in a way so that they meet the code style standards documented e.g. here for PHP: https://developer.joomla.org/coding-standards/php-code.html.

Edit: if I use tabs and specify the number of spaces that the tab uses, in which case, is it 4? On looking at the original code, it looks like it. Thank you.

[richard67]

Can you please clarify whether I have to use tabs instead of spaces?
If I use four spaces instead of a tab, for instance, is that ok or should I use tabs and specify the number of spaces that the tab uses, in which case, is it 4? On looking at the original code, it looks like it. Thank you.

@Scrabble96 If the code style says you have to use tabs then you have to use tabs. 1 tab means one level of indent. 4 spaces are not the same as 1 tab, it only can be made that it looks the same by editor settings. The drone PHPCS log for which I have pasted the link https://ci.joomla.org/joomla/joomla-cms/35590/1/8 and of which I have explained you how to find it clearly tells the line numbers where tabs have to be used.

[richard67]

PHPCS is fine now. Thanks @Scrabble96 .

[Scrabble96]

PHPCS is fine now. Thanks @Scrabble96 .

Thank you. But I'm confused as to why cardGrey has passed checks but default has not. The ONLY difference in the code is line 25 - the class 'card-grey' does not appear in default.php

[richard67]

Certainly you can't have the same module in the same position multiple times.

@Llewellynvdm That's not true. Theoretically one can show the same menu module for the same menu multiple times on the same position. That could make sense e.g. if the menu has several levels of submenus and in the first menu you show e.g. the top level, the second one the next deeper level and so on. I have no idea if this is practically relevant, but it is possible. Sorry, forget it, my mistake. You said to use the module ID, not the menu ID, and so they are different modules and so have different IDs. Silly me.

[Scrabble96]

Shall I change and submit the new code? If so, perhaps the #id should be on the module container, regardless of whether the title is displayed or not. In which case, how should I best handle the ‘aria-label’ and ‘aria-labelledby’ aspects? Do we need both?

[HLeithner]

A random unpredictable ID defeats the purpose of an id really in terms of targeting it with anything front-end, having a predictable ID seems important. So can't the position and the module ID be used together as the ID? or what am I missing? Certainly you can't have the same module in the same position multiple times.

I see no way how to do this. ymmv

[Bakual]

If you have the same template position multiple times, or manually load a module multiple times, then you get duplicate IDs.
So you just can't set the ID in the chrome. You need to use class.

[Scrabble96]

If you have the same template position multiple times.

Why would you do that?

You need to use class.

How? By using a pseudo 'before' class?

[Scrabble96]

I have modified the chrome (locally, only) to include the module position in the id and it works. But this is based on the assumption that module positions are only used once. I'll post it here if anyone is interested.

[I do, however, get this message on Firefox: "Layout was forced before the page was fully loaded. If stylesheets are not yet loaded this may cause a flash of unstyled content." but not on Chrome or Edge. ] Edit: I refreshed the .htaccess file and this seems to have cleared the problem.

[Bakual]

See comment from Harald why a position may be present more than once.
So using the ID attribute is not an option. Even when coupled with the position you still get duplicates.
You can only use the class attribute. There you can insert the module ID as a class which can then be used for CSS styling.

[HLeithner]

Funny, I got a link to this document today by a customer: https://www.w3.org/TR/WCAG20-TECHS/H93.html IDs have to be unique.

[Scrabble96]

There you can insert the module ID as a class which can then be used for CSS styling.

But you can't use it for URL anchors which was one of the two reasons I wanted to add an ID to all modules.

[HLeithner]

Technically it should be possible to inject the file and line position of the into the module attributes and generate a unique id per module and position. That would invalidate our module caching if it's loaded twice on the same page. That can be solved in 2 ways.

1. make a placeholder in the chrome/module layout and do a str_replace on render (we do this on full page cache for example)
2. ignore it and accept that the same module maybe rendered twice (I think that's a less complicated variant and shouldn't hurt to much)

The relevant starting point for a PR is in libraries/src/Document/HtmlDocument.php:849

[Bakual]

Probably the simplest approach is to just create your own chrome for your own installation where you need the anchor. There you can make sure that module with that chrome is only showing once per page.

For a module chrome shipped with the CMS we just can't be sure that the ID will be unique.

By the way, a PR which tried to achieve a similar thing with a different approach was #12473

[Scrabble96]

Probably the simplest approach is to just create your own chrome for your own installation where you need the anchor. There you can make sure that module with that chrome is only showing once per page.

For those who would like it, I am happy to share my chrome code which shows both the module position and the module id, but, as @Bakual says, obviously taking into account that any id should be shown only once per page:

<?php
/**
 * @package     Joomla.Site
 * @subpackage  Templates.cassiopeia
 *
 * @copyright   Copyright (C) 2005 - 2020 Open Source Matters, Inc. All rights reserved.
 * @license     GNU General Public License version 2 or later; see LICENSE.txt
 */

defined('_JEXEC') or die;

use Joomla\Utilities\ArrayHelper;

$module  = $displayData['module'];
$params  = $displayData['params'];
$attribs = $displayData['attribs'];

if ($module->content === null || $module->content === '')
{
	return;
}

$moduleTag              = $params->get('module_tag', 'div');
$modId                  = $module->id;
$modPos                 = $module->position;
$moduleAttribs          = [];
$moduleAttribs['id']    = $modPos . '-' . $modId;
$moduleAttribs['class'] = $module->position . ' card ' . htmlspecialchars($params->get('moduleclass_sfx'), ENT_QUOTES, 'UTF-8');
$headerTag              = htmlspecialchars($params->get('header_tag', 'h4'), ENT_QUOTES, 'UTF-8');
$headerClass            = 'card-header ' . htmlspecialchars($params->get('header_class', ''), ENT_QUOTES, 'UTF-8');
$headerAttribs          = [];
$headerAttribs['class'] = $headerClass;

if ($module->showtitle) :
else:
	$moduleAttribs['aria-label'] = $module->title;
endif;

$header = '<' . $headerTag . ' ' . ArrayHelper::toString($headerAttribs) . '>' . $module->title . '</' . $headerTag . '>';
?>
<<?php echo $moduleTag; ?> <?php echo ArrayHelper::toString($moduleAttribs); ?>>
	<?php if ($module->showtitle && $headerClass !== 'card-title') : ?>
			<?php echo $header; ?>
		<?php endif; ?>
	<div class="card-body">
                          		<?php echo $module->content; ?>
                          	</div>
</<?php echo $moduleTag; ?>>

The html looks like this with title:

And like this without a title:

For a module chrome shipped with the CMS we just can't be sure that the ID will be unique.

As pointed out earlier, the current 'card' chrome includes a module ID, but only if the title is shown.

By the way, a PR which tried to achieve a similar thing with a different approach was #12473

I hadn't spotted this as it isn't J4.

EDIT by hleithner: updated the code highlighting

[ChrisHoefliger]

Tested successfully on localhost

[richard67]

@HLeithner @Bakual @infograf768 What shall we do with this one? It has 2 good tests, but these seems to be some unresolved discussion? Shall I set discussion status in the issue tracker?

[ghost]

I have tested this item ✅ successfully on 64bde14

---

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

[HLeithner]

#30716 (comment) Option 1 is the way to go.

[ghost]

@richard67 you got the answer a week ago

[richard67]

@Gostn We don't need trolls here.

[ghost]

We don't need trolls here

true

[Scrabble96]

I have tested this item ✅ successfully on 64bde14
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30716.

@Gostn this PR was effectively put on hold due to the use of IDs in the chrome which would create duplicate IDs if the same module was used twice on the page, which is not allowed in good website page setup. There was a suggestion to add the 'discussion' tag to this PR, but that hasn't happened. I don't have the authority to do that. My original reason for opening this PR was two-fold: that the ID could be used for in-page anchors and that the ID would be shown whether the title was shown or not. Due to the problem with duplicate IDs this PR doesn't seem viable as it stands.

Frustratingly, before HTML5 the name attribute could be used in for anchors in links but it has now been deprecated in favour of the use of IDs, which brings us back to square one. I wonder when they (whoever 'they' are) deprecated its use whether they considered the possibility of duplicate IDs on a page?

[ghost]

@Scrabble96 thanks for your friendly information.

[hans2103]

@Scrabble96 This PR of your seems to have a merge conflict and cannot be merged into Joomla.
Can you rewrite your PR?

I have a suggestion.

joomla-cms/templates/cassiopeia/html/layouts/chromes/card.php

Lines 24 to 25 in 3b82bf1

	$moduleAttribs = [];
	$moduleAttribs['class'] = $module->position . ' card ' . htmlspecialchars($params->get('moduleclass_sfx'), ENT_QUOTES, 'UTF-8');

To make sure a module always has an id, rewrite the two lines above into the three lines below:

$moduleAttribs          = [];
$moduleAttribs['id']    = 'mod-' . $module->id;
$moduleAttribs['class'] = $module->position . ' card ' . htmlspecialchars($params->get('moduleclass_sfx'), ENT_QUOTES, 'UTF-8');

Two prevent duplicate ID's we have to adjust a second part of this file too:

joomla-cms/templates/cassiopeia/html/layouts/chromes/card.php

Lines 36 to 45 in 3b82bf1

	// Only add aria if the moduleTag is not a div
	if ($moduleTag !== 'div')
	{
	if ($module->showtitle) :
	$moduleAttribs['aria-labelledby'] = 'mod-' . $module->id;
	$headerAttribs['id'] = 'mod-' . $module->id;
	else:
	$moduleAttribs['aria-label'] = $module->title;
	endif;
	}

To prevent duplicate ID's we have to adjust the name of the id for the header into:

// Only add aria if the moduleTag is not a div
if ($moduleTag !== 'div')
{
	if ($module->showtitle) :
		$moduleAttribs['aria-labelledby'] = 'mod-title-' . $module->id;
		$headerAttribs['id']              = 'mod-title-' . $module->id;
	else:
		$moduleAttribs['aria-label'] = $module->title;
	endif;
}

With these two changes based on the current implementation of template/cassiopeia/html/layouts/chromes/card.php the PR is good to go I think.
What do you think?

[HLeithner]

As already mentioned if the module is cached (and if I'm not wrong) the id is still duplicated.

[Scrabble96]

@Scrabble96 This PR of your seems to have a merge conflict and cannot be merged into Joomla.
Can you rewrite your PR?

@hans2103 I'll have a look in the next few days but, as @HLeithner says, there is a problem with cached modules so I don't think this is going to work.

[Scrabble96]

I'm going to close this PR due to the cache conflict.