Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Cassiopeia: update cardGrey and default chromes to add mod ids to all modules, plus new noTitle chrome #30680

Closed
wants to merge 11 commits into from
Closed

[4.0] Cassiopeia: update cardGrey and default chromes to add mod ids to all modules, plus new noTitle chrome #30680

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7df08cb
Update cardGrey.php
Scrabble96 Sep 18, 2020
f93e868
Update default.php
Scrabble96 Sep 18, 2020
c11e5a6
Update cardGrey.php
Scrabble96 Sep 18, 2020
ca7ddfe
Create noTitle.php
Scrabble96 Sep 18, 2020
90c1042
Update templates/cassiopeia/html/layouts/chromes/default.php
Scrabble96 Sep 19, 2020
e0fa90f
Update templates/cassiopeia/html/layouts/chromes/noTitle.php
Scrabble96 Sep 19, 2020
a12ceb8
Update templates/cassiopeia/html/layouts/chromes/cardGrey.php
Scrabble96 Sep 19, 2020
d67c35b
Update cardGrey.php
Scrabble96 Sep 19, 2020
55a24ce
Merge pull request #2 from joomla/4.0-dev
Scrabble96 Sep 19, 2020
5fa8919
Update default.php
Scrabble96 Sep 19, 2020
a297f7f
Update noTitle.php
Scrabble96 Sep 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 14 additions & 26 deletions templates/cassiopeia/html/layouts/chromes/cardGrey.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,34 +21,22 @@
}

$moduleTag = $params->get('module_tag', 'div');
$moduleAttribs = [];
$modulePos = $module->position;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$modulePos = $module->position;
$modulePos = $module->position;

PHPCS: Here we use spaces to indent to alignt the "=" if several assignments in consecutive lines.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one here, too, please.

$moduleAttribs['class'] = $module->position . ' card card-grey ' . htmlspecialchars($params->get('moduleclass_sfx'), ENT_QUOTES, 'UTF-8');
$modId = 'mod-' . $module->id;
Scrabble96 marked this conversation as resolved.
Show resolved Hide resolved
$headerTag = htmlspecialchars($params->get('header_tag', 'h4'), ENT_QUOTES, 'UTF-8');
$headerClass = htmlspecialchars($params->get('header_class', ''), ENT_QUOTES, 'UTF-8');
$headerAttribs = [];
$headerAttribs['class'] = $headerClass;

if ($module->showtitle) :
$moduleAttribs['aria-labelledby'] = 'mod-' . $module->id;
$headerAttribs['id'] = 'mod-' . $module->id;

if ($headerClass !== 'card-title') :
$headerAttribs['class'] .= 'card-header ' . $headerClass;
endif;
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 if ($module->showtitle && $headerClass === 'card-title') : ?>
$header = '<div class="card-header'. $headerClass .'"><h3>' . $module->title . '</h3/></div>';
if ($module->content) : ?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is not needed. It's already performed on line 18.

<?php if ($module->showtitle) : ?>
<div id="<?php echo $modId; ?>" class="<?php echo $moduleAttribs['class'] ?>">
<?php echo $header; ?>
<?php endif; ?>
<?php echo $module->content; ?>
</div>
</<?php echo $moduleTag; ?>>
<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; ?>
45 changes: 17 additions & 28 deletions templates/cassiopeia/html/layouts/chromes/default.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,34 +21,23 @@
}

$moduleTag = $params->get('module_tag', 'div');
$moduleAttribs = [];
$modulePos = $module->position;
Scrabble96 marked this conversation as resolved.
Show resolved Hide resolved
$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 = htmlspecialchars($params->get('header_class', ''), ENT_QUOTES, 'UTF-8');
$headerAttribs = [];
$headerAttribs['class'] = $headerClass;

if ($module->showtitle) :
$moduleAttribs['aria-labelledby'] = 'mod-' . $module->id;
$headerAttribs['id'] = 'mod-' . $module->id;

if ($headerClass !== 'card-title') :
$headerAttribs['class'] .= 'card-header ' . $headerClass;
endif;
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 if ($module->showtitle && $headerClass === 'card-title') : ?>
$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');

$header = '<div class="card-header'. $headerClass .'"><h3>' . $module->title . '</h3/></div>';
if ($module->content) : ?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is not needed. It's already performed on line 18.

<?php if ($module->showtitle) : ?>
<div id="<?php echo $modId; ?>" class="<?php echo $moduleAttribs['class'] ?>">
<?php echo $header; ?>
<?php endif; ?>
<?php echo $module->content; ?>
</div>
</<?php echo $moduleTag; ?>>
<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; ?>
26 changes: 26 additions & 0 deletions templates/cassiopeia/html/layouts/chromes/noTitle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?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;
Scrabble96 marked this conversation as resolved.
Show resolved Hide resolved

if ($module->content) : ?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this check so it's the same as in other layouts?

<div id="<?php echo $modId; ?>" class="<?php echo $modulePos; ?> <?php echo htmlspecialchars($params->get('moduleclass_sfx')); ?>" aria-labelledby="<?php echo $module->title; ?>">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use $moduleTag instead of div.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like use of $moduleTag is missing in other layouts as well.

Copy link
Contributor Author

@Scrabble96 Scrabble96 Sep 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use $moduleTag instead of div.

Aren't they the same thing? Edit: Forget that! I see what it is and will try to update it (I've never used anything other than the default 'div')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use $moduleTag instead of div.

I think the code needs to change like this by adding $moduleTag and $headerTag but removing references to h3 and h4 as I'm not sure what they're there for. It is possible to have h1 or h2 as a header tag.

https://github.com/Scrabble96/joomla-cms/blob/d67c35bd85bb30d82a27dda43c0b4b7de149874b/templates/cassiopeia/html/layouts/chromes/cardGrey.php#L27-L40

(Sigh) I can't see how to paste the copied lines in here like others have done, hence the link.

<div>
<?php echo $module->content; ?>
</div>
</<?php echo $moduleTag; ?>>
<?php endif; ?>