[#30678] - [4.0] Feature request/fix: Cassiopeia template - move mod-id from header to whole module
- Closed
- 18 Sep 2020
- Medium
- Build: staging
- # 30678
What needs to be fixed
Currently, the cardGrey and default chromes add the module Id to the header only with a corresponding "aria-labelledby" followed by the same mod Id, e.g. aria-labelledby="mod-16"
Why this should be fixed
If the module header is not shown, then neither is the id and the module outer now contains a different label. e.g. aria-label="main menu" being the module name.
If there was an id for all modules (I accept that it won't work where the module in index.php says "style=none") then these could be used for hyperlink anchors, e.g. /mypage#mod-16
How would you fix it
- Move the mod Id to a level up, i.e. the one that contains the header and content.
- Show the module name as the aria-label either only when header is not shown (v1 below) or always (v2 below)
I have created two versions of the default.php chrome with these changes illustrated:
1.1 Applies the mod-id to the module regardless of whether the header (title) is displayed
1.2 Applies the aria-label only if the title is not displayed.
This is 46 lines of code (the current default.php is 54 lines)
Here is the rendered code viewed with inspector ('mod-class' is the Module class suffix and head-class is the Header class suffix):
2.1 as 1.1 above
2.2 Applies the aria-label regardless of whether the header is displayed.
This is only 39 lines of code.
This is how it looks in inspector:
Here is the code if you would like to test them:
Version 1 (updated following comment from @hans2103 :
<?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');
$modulePos = $module->position;
$moduleAttribs['class'] = $module->position . ' card ' . htmlspecialchars($params->get('moduleclass_sfx'), ENT_QUOTES, 'UTF-8');
$modId = 'mod-' . $module->id;
$headerTag = htmlspecialchars($params->get('header_tag', 'h4'), ENT_QUOTES, 'UTF-8');
$headerClass = htmlspecialchars($params->get('header_class', ''), ENT_QUOTES, 'UTF-8');
$moduleAttribs['aria-label'] = $module->title;
$header = '<div class="card-header'. $headerClass .'"><h3>' . $module->title . '</h3/></div>';
if ($module->content) : ?>
<?php if ($module->showtitle) : ?>
<div id="<?php echo $modId; ?>" class="<?php echo $moduleAttribs['class'] ?>">
<?php echo $header; ?>
<div class="card-body"><?php echo $module->content; ?></div>
</div>
<?php else : ?>
<div id="<?php echo $modId; ?>" class="<?php echo $moduleAttribs['class'] ?>" aria-labelledby="<?php echo $module->title; ?>">
<div class="card-body"><?php echo $module->content; ?></div>
</div>
<?php endif; ?>
<?php endif; ?>
and this is Version 2 - update: this doesn't work due to aria-label being used if title/header on or off
<?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');
$modulePos = $module->position;
$moduleAttribs['class'] = $module->position . ' card ' . htmlspecialchars($params->get('moduleclass_sfx'), ENT_QUOTES, 'UTF-8');
$modId = 'mod-' . $module->id;
$headerTag = htmlspecialchars($params->get('header_tag', 'h4'), ENT_QUOTES, 'UTF-8');
$headerClass = htmlspecialchars($params->get('header_class', ''), ENT_QUOTES, 'UTF-8');
$moduleAttribs['aria-label'] = $module->title;
$header = '<div class="card-header'. $headerClass .'"><h3>' . $module->title . '</h3/></div>';
if ($module->content) : ?>
<div id="<?php echo $modId; ?>" class="<?php echo $moduleAttribs['class'] ?>" aria-label="<?php echo $module->title; ?>">
<?php if ($module->showtitle) : ?>
<?php echo $header; ?>
<?php endif; ?>
<div class="card-body"><?php echo $module->content; ?></div>
</div>
<?php endif; ?>
I have also created a version called 'noTitle.php' to use in place of 'none':
(updated following comment from @hans2103) :
<?php
/**
* @package Joomla.Site
* @subpackage Templates.crocosmia
*
* @copyright Copyright (C) 2005 - 2019 Open Source Matters, Inc. All rights reserved.
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/
defined('_JEXEC') or die;
$module = $displayData['module'];
$params = $displayData['params'];
$attribs = $displayData['attribs'];
$modulePos = $module->position;
$moduleTag = $params->get('module_tag', 'div');
$modId = 'mod-' . $module->id;
if ($module->content) : ?>
<div id="<?php echo $modId; ?>" class="<?php echo $modulePos; ?> <?php echo htmlspecialchars($params->get('moduleclass_sfx')); ?>" aria-labelleby="<?php echo $module->title; ?>">
<div>
<?php echo $module->content; ?>
</div>
</<?php echo $moduleTag; ?>>
<?php endif; ?>
Additional context
Version: Joomla 4 Beta5 Nightly 18/09/2020
joomla-cms-bot
- | Labels |
Added:
?
|
||
@Scrabble96 thank you for testing and presenting solutions. Can you make pull-requests with your code?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30678.
Thank you. If I can remember how to do it (and I have a new PC with nothing Github-related installed) I'll try to do so later, but I'll also have a go at the cardGrey version, too.
@Scrabble96 could you please check the HTML when you set Show Title = NO
When Show Title is set to no the attribute aria-label is used instead of aria-labelledby
@Scrabble96 could you please check the HTML when you set Show Title = NO
When Show Title is set to no the attributearia-labelis used instead ofaria-labelledby
To be honest, I didn't really know what the difference is. I've just looked it up and I can change that easily enough in v1 and noTitle.php. It won't work in v2 as they share the outer div. Thank you.
This is how version 1 html looks updated as above. Is it necessary to include the aria-labelledby code in the outer div if the module title is displayed? If so, I can add it.
OK. I have made all the changes in my fork and saved them to a new branch and I thought only those changed files would be in that branch. Trouble is, when I start a PR, all my old commits are included as well. I'm using the browser version of Github and can't for the life of me work out how to remove the old commits. Help, please!
@Scrabble96 When you make a PR you always start with a code comparison, e.g. by following the "compare" link when you have your branch selected on GitHub. But this by default compares with the staging branch, which is for 3.9.x. The comparison somewhere shows at the left hand side the branch with which is compared (target branch or base branch, e.g. "staging"), and on the right hand site your branch in your repository.
Before clicking the "Create Pull Request" (or so) button, you have to change the base branch from "staging" to "4.0-dev". Have you done that?
Yes, done all that:
And the branch is one I created today, but as well as the file changes I've made today, it's listing commits and PRs - some merged, some not - going back to 2019 when I started - even those PRs when I updated the repository earlier!
Well, a new branch always contains the commit hisotry of the branch based on which it was created. So it remains to be checked if the comparison on GitHub shows only the desired changes or not. If not, then the new branch is based on the wrong branch.
Is your branch already on GitHub? If so, what's the name? Maybe I can check what's wrong?
Yes, I work in Github as I haven't installed the github for desktop app. It's as shown in my screenshot.
I'm now installing the desktop app. There's lots of instructions on how to do remove commits locally and then uploading to Github.
The branch as such is ok and also your attempt how to start with the pull request. But is seems you had once done some work on your 4.0-dev branch, which also is the base branch of your PR, and so you see more changes than expected.
See this link for a comparison between your 4.0-dev branch and the CMS's 4.0-dev branch: 4.0-dev...Scrabble96:4.0-dev.
You should exactly see those additional commits which were the unexpected ones when you checked the comparison for your branch.
A local git client can definitely help to solve that.
Another way, more easy maybe, is just to delete your Joomla CMS fork on GitHub and then fork the repository again, but this would mean you lose your history and possible other ongoing work. Some people do that when in trouble because that's the most easy way.
Well, after fumbling around I did as @richard67 suggested, deleted the repository and started again. This time the PR only has the commits that should be there.
@Scrabble96 Well, as I see you made a PR first in your onw repository and merged it into your 4.-dev branch. Maybe that was for testing, I don't know. But it will cause you the same problem again when you in future make again a PR based on your 4.0-dev branch: It will contain the changes from the merged commit already and so show the changes from this plus the new PR when comparing to the 4.0-dev branch of the CMS. So either next time you delete your fork again, or someone teaches you a bit git so you can properly reset your 4.0-dev branch to the 4.0-dev of the CMS. I have the knowledge to do that, but not really the time now.
@Scrabble96 But the new PR is ok now, my previous comment refers to problems you will have when creating another PR in future based on your 4.0-dev branch in which you have merged your PR no. 1 in your repo. Question: Does PR #30680 solve this issue here? If yes, you can close the issue because we do that when we have a PR which claims to solve it. Thanks in advance.
@Scrabble96 Well, as I see you made a PR first in your onw repository and merged it into your 4.-dev branch. Maybe that was for testing, I don't know. But it will cause you the same problem again when you in future make again a PR based on your 4.0-dev branch: It will contain the changes from the merged commit already and so show the changes from this plus the new PR when comparing to the 4.0-dev branch of the CMS. So either next time you delete your fork again, or someone teaches you a bit git so you can properly reset your 4.0-dev branch to the 4.0-dev of the CMS. I have the knowledge to do that, but not really the time now.
Oh dear. I'll get there in the end. I thought I had to merge it into my fork before I could merge it into here. Once the new PR is closed I'll start again. Thank you very much for your help.
I will close this issue now.
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-09-18 17:19:51 |
| Closed_By | ⇒ | Scrabble96 |
@Scrabble96 I can feel with you. Sometimes I look back my first PR's from 2014 to remember how I started, and that was a struggle, too.
@Scrabble96 I started like you. After 3 or xx tries and with help of some experts I finally can do it. So thank you and don't give up. :)
@Scrabble96 thank you for testing and presenting solutions. Can you make pull-requests with your code?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30678.