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

Scrabble96 · 2020-09-18T17:01:35Z

Pull Request for Issue #30678 .

Summary of Changes

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"

If the module header (title) is not shown, then neither is the id and the module outer now contains a different label. e.g. aria-label="module name".

If there was an id for all modules (it won't work where the module position in index.php says "style=none") then these ids could be used for hyperlink anchors, e.g. /mypage#mod-16

Testing Instructions

Create a new new mod chrome with this code
Assign that chrome to a style for a module position in the Cassiopeia template's index.php
Create a module and assign it to that module position
Check the html with the browser inspector with header set to both Show and Hide

There is also a NEW chrome, called noTitle.php which can be applied where it is not usual to have a title showing, e.g. the main menu or search module. This can be used as "style=noTitle" in place of "style=none" which will also allow for custom css styling if a module class is added. At the moment, module class is ignored if the style is 'none'.

Actual result BEFORE applying this Pull Request

cardGrey.php and default.php chromes only applied the mod-id to the header (title) so if the header is not shown, then neither is the mod-id.

Expected result AFTER applying this Pull Request

By using the two changed files and using the noTitle chrome instead of none, all modules will have their ids applied, which is useful for link anchors and all modules can have module class suffix applied. On the front end, there is no change to a visitor's view of the site.

Documentation Changes Required

Comments to say that:

module ids can be used as anchors.
all modules can be assigned a css class if style=none is changed to style=noTitle

This is for modules that generally do not have their header/title displayed, e.g. main menu or search. If used in the module position in the template index.php as "style=noTitle" instead of "style=none" then this will allow a mod id and module class suffix to be added to the modules.

richard67 · 2020-09-19T09:32:05Z

templates/cassiopeia/html/layouts/chromes/cardGrey.php

@@ -21,34 +21,22 @@
 }

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


Suggested change

$modulePos = $module->position;

$modulePos = $module->position;

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

This one here, too, please.

templates/cassiopeia/html/layouts/chromes/cardGrey.php

templates/cassiopeia/html/layouts/chromes/default.php

templates/cassiopeia/html/layouts/chromes/noTitle.php

SharkyKZ · 2020-09-19T14:50:48Z

templates/cassiopeia/html/layouts/chromes/cardGrey.php

-	<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) : ?>


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

SharkyKZ · 2020-09-19T14:51:24Z

templates/cassiopeia/html/layouts/chromes/default.php

+$headerClass = htmlspecialchars($params->get('header_class', ''), ENT_QUOTES, 'UTF-8');
+
+$header = '<div class="card-header'. $headerClass .'"><h3>' . $module->title . '</h3/></div>';
+if ($module->content) : ?>


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

SharkyKZ · 2020-09-19T14:53:20Z

templates/cassiopeia/html/layouts/chromes/noTitle.php

+$moduleTag   = $params->get('module_tag', 'div');
+$modId 		 = 'mod-' . $module->id;
+
+if ($module->content) : ?>


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

SharkyKZ · 2020-09-19T14:56:41Z

templates/cassiopeia/html/layouts/chromes/noTitle.php

+$modId 		 = 'mod-' . $module->id;
+
+if ($module->content) : ?>
+	<div id="<?php echo $modId; ?>" class="<?php echo $modulePos; ?> <?php echo htmlspecialchars($params->get('moduleclass_sfx')); ?>"  aria-labelledby="<?php echo $module->title; ?>">


Use $moduleTag instead of div.

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

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')

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.

Co-authored-by: Richard Fath <[email protected]>

chmst · 2020-09-19T15:38:53Z

@Scrabble96 I have labelled your PR with two labels and we will take it with priority in the Template working group.

As I see that you are new to github and the whole process, may I explain?

Reviewers can see violations of the coding standard, for example spaces where the rules require a tab, then the reviewer can edit your code on github and make a suggestion.
If you see a suggestion of a reviewer, the simplest way is, to accept it.
If you accept, your code is changed and you find can click ont "Resolve conversation". Then the small dialogue is closed.
Now the code is changed in github but not yet in your local repo.
So you have to pull these changes into your own repo.

Reviewers also can write comments if something is not clear as the comments of @SharkyKZ. This means that you should check your code - and in general you will accept the comment and change your code on your own repo. You then can commit your changes and push them.
If this is done, the recommendation or question of the reviewer is resolved and you close the dialogue (resolve conversation).
Or, if you think that the reviewer is wrong, of course you can answer and add your arguments.

I hope this helps - starting in github is a challenge. You can ping me, if you want, with a mention: write @chmst and I get an e-mail.

4.0 dev

Scrabble96 · 2020-09-19T15:57:24Z

@Scrabble96 I have labelled your PR with two labels and we will take it with priority in the Template working group.

Reviewers also can write comments if something is not clear as the comments of @SharkyKZ. This means that you should check your code - and in general you will accept the comment and change your code on your own repo. You then can commit your changes and push them.

I made a change to cardGrey.php in this branch and committed the changes, but they don't seem to be showing up here. How do I add it to this PR?

I see it says 'Pending - Build is running'. Can that be stopped? If the change to cardGrey.php is ok I need to change the other two files as well.

I hope this helps - starting in github is a challenge. You can ping me, if you want, with a mention: write @chmst and I get an e-mail.

Many thanks. I may well ping you at some point.

chmst · 2020-09-19T16:17:01Z

I made a change to cardGrey.php in this branch and committed the changes, but they don't seem to be showing up here. How do I add it to this PR?

You must then push your commit into the repo. Commit is only locally - it is useful, as you can commit every step of development and add a comment what has been done. Then you push it and it is transferred.

I see it says 'Pending - Build is running'. Can that be stopped? If the change to cardGrey.php is ok I need to change the other two files as well.

These automatic tests cannot be stopped, they need their time (Joomla is huge).

Scrabble96 · 2020-09-19T17:20:49Z

You must then push your commit into the repo. Commit is only locally - it is useful, as you can commit every step of development and add a comment what has been done. Then you push it and it is transferred.

Push? I am using github online, not locally. I can't work out GitHub desktop.

Changed div to $moduleTag Removed h3 and h4

Changed opening div to $moduleTag

Scrabble96 · 2020-09-21T10:17:42Z

Sorry, but I can't make head nor tail of how to push new commits into this PR. I have installed git (git bash?) for Windows 10 and Github desktop. I notice it says at the top that I want to merge 11 commits. But I only made 3 changes, so I don't know what the other 8 are.

I might be easier if I delete this PR and just start again with the correct code.

Any other suggestions? Humble thanks

richard67 · 2020-09-21T10:21:55Z

I might be easier if I delete this PR and just start again with the correct code.

If that is easier for you then do so.

But make sure your 4.0-dev branch is up to date with the 4.0-dev branch of the CMS and does not contain any other changes. Then when creating the new branch for the new PR, make sure it is created based on the 4.0-dev branch. This can be done in GitHub desktop by first selecting the 4.0-dev branch as current branch, then creating a new branch, and then select the current 4.0-dev branch and not staging in the choice for the base branch of that new branch.

Scrabble96 · 2020-09-21T10:25:16Z

I've just found git bash on my desktop, which I see is different from git

But all the tutorials I can find only show how to commit to my branch on my own repo only but not how to push them to this PR.

Scrabble96 · 2020-09-21T10:42:52Z

Closing this PR to start again.

Scrabble96 added 4 commits September 18, 2020 17:39

Update cardGrey.php

7df08cb

Update default.php

f93e868

Update cardGrey.php

c11e5a6

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

richard67 mentioned this pull request Sep 18, 2020

[4.0] Feature request/fix: Cassiopeia template - move mod-id from header to whole module #30678

Closed

richard67 reviewed Sep 19, 2020

View reviewed changes

templates/cassiopeia/html/layouts/chromes/cardGrey.php Outdated Show resolved Hide resolved

richard67 reviewed Sep 19, 2020

View reviewed changes

templates/cassiopeia/html/layouts/chromes/default.php Outdated Show resolved Hide resolved

richard67 reviewed Sep 19, 2020

View reviewed changes

templates/cassiopeia/html/layouts/chromes/noTitle.php Outdated Show resolved Hide resolved

SharkyKZ reviewed Sep 19, 2020

View reviewed changes

chmst added a11y Accessibility Frontend Template labels Sep 19, 2020

Scrabble96 and others added 3 commits September 19, 2020 16:13

Update templates/cassiopeia/html/layouts/chromes/default.php

90c1042

Co-authored-by: Richard Fath <[email protected]>

Update templates/cassiopeia/html/layouts/chromes/noTitle.php

e0fa90f

Co-authored-by: Richard Fath <[email protected]>

Update templates/cassiopeia/html/layouts/chromes/cardGrey.php

a12ceb8

Co-authored-by: Richard Fath <[email protected]>

Scrabble96 added 2 commits September 19, 2020 16:41

Update cardGrey.php

d67c35b

Merge pull request #2 from joomla/4.0-dev

55a24ce

4.0 dev

Scrabble96 added 2 commits September 19, 2020 18:30

Update default.php

5fa8919

Changed div to $moduleTag Removed h3 and h4

Update noTitle.php

a297f7f

Changed opening div to $moduleTag

Scrabble96 closed this Sep 21, 2020

richard67 mentioned this pull request Nov 28, 2020

[4.0] Cassopeia template and blog sample data fixes and improvements #31520

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Scrabble96 commented Sep 18, 2020

richard67 Sep 19, 2020

richard67 Sep 19, 2020

SharkyKZ Sep 19, 2020

SharkyKZ Sep 19, 2020

SharkyKZ Sep 19, 2020

SharkyKZ Sep 19, 2020

SharkyKZ Sep 19, 2020

Scrabble96 Sep 19, 2020 •

edited

Loading

Scrabble96 Sep 19, 2020

chmst commented Sep 19, 2020

Scrabble96 commented Sep 19, 2020

chmst commented Sep 19, 2020

Scrabble96 commented Sep 19, 2020 •

edited

Loading

Scrabble96 commented Sep 21, 2020

richard67 commented Sep 21, 2020

Scrabble96 commented Sep 21, 2020 •

edited

Loading

Scrabble96 commented Sep 21, 2020

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

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

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

Conversation

Scrabble96 commented Sep 18, 2020

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Scrabble96 Sep 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chmst commented Sep 19, 2020

Scrabble96 commented Sep 19, 2020

chmst commented Sep 19, 2020

Scrabble96 commented Sep 19, 2020 • edited Loading

Scrabble96 commented Sep 21, 2020

richard67 commented Sep 21, 2020

Scrabble96 commented Sep 21, 2020 • edited Loading

Scrabble96 commented Sep 21, 2020

Scrabble96 Sep 19, 2020 •

edited

Loading

Scrabble96 commented Sep 19, 2020 •

edited

Loading

Scrabble96 commented Sep 21, 2020 •

edited

Loading