[4.2] mod_breadcrumbs. Fix several issues with ld+json structured data#38285
Merged
roland-d merged 7 commits intojoomla:4.2-devfrom Jul 29, 2022
GHSVS-de:mod_breadcrumbs
Merged
[4.2] mod_breadcrumbs. Fix several issues with ld+json structured data#38285roland-d merged 7 commits intojoomla:4.2-devfrom GHSVS-de:mod_breadcrumbs
roland-d merged 7 commits intojoomla:4.2-devfrom
GHSVS-de:mod_breadcrumbs
Conversation
- Add a helper method getHome() that can be used without redundant code elsewhere. - Get the home item ($homeCrumb) in mod_breadcrumbs.php if showHome is disabled. - default.php: - - Use an independant counter for positions. - - Always set an itemListElement for startpage.
GHSVS-de
commented
Jul 16, 2022
richard67
reviewed
Jul 16, 2022
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
|
I have tested this item ✅ successfully on 8ac90ae This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38285. |
1 similar comment
Contributor
|
I have tested this item ✅ successfully on 8ac90ae This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38285. |
Member
|
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38285. |
Contributor
|
Thank you |
brianteeman
pushed a commit
to brianteeman/joomla-cms
that referenced
this pull request
Aug 2, 2022
joomla#38285) * Fix position counter and get home item always. - Add a helper method getHome() that can be used without redundant code elsewhere. - Get the home item ($homeCrumb) in mod_breadcrumbs.php if showHome is disabled. - default.php: - - Use an independant counter for positions. - - Always set an itemListElement for startpage. * Code style mod_breadcrumbs.php * Code style BreadcrumbsHelper.php * codestyle default.php * Typo headline versus Heading * Update modules/mod_breadcrumbs/tmpl/default.php Co-authored-by: Richard Fath <richard67@users.noreply.github.com> Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
rdeutz
pushed a commit
to joomla-projects/joomla-cms
that referenced
this pull request
Aug 8, 2022
joomla#38285) * Fix position counter and get home item always. - Add a helper method getHome() that can be used without redundant code elsewhere. - Get the home item ($homeCrumb) in mod_breadcrumbs.php if showHome is disabled. - default.php: - - Use an independant counter for positions. - - Always set an itemListElement for startpage. * Code style mod_breadcrumbs.php * Code style BreadcrumbsHelper.php * codestyle default.php * Typo headline versus Heading * Update modules/mod_breadcrumbs/tmpl/default.php Co-authored-by: Richard Fath <richard67@users.noreply.github.com> Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request for Issue #38229
Fixes also: Position is wrongly set if the menu pathway for example contains Heading menu items.
Summary of Changes
getHome()that can be used without redundant code elsewhere to get the home item.$app->getLanguage()if needed.$homeCrumb) inmod_breadcrumbs.phpifshowHomeis disabled.default.php:showHomeis disabled.<SCRIPT ld.json>if BraedCrumbList is populated.Testing Instructions
mod_breadcrumbsmodule. On all pages. Play around with the settings in the following.The visible breadcrumb hasn't changed with this pr, only the
ld+jsonpart.Either:
In frontend click through your menus and check the source code of displayed codes.
Search for
ld+jsonto find a<script>tag like<script type="application/ld+json">{"@context":"https:\/\/schema.org","@type":"BreadcrumbList","itemListElement":[...]}</script>Or:
You can also use Browser Addons like "JSON-LD Tester" for chromium browsers (Chrome, Vivaldi ...) which uses Google testing tools.
Both:
positiongo from 1 to x in +1 steps.position: 1.Actual result BEFORE applying this Pull Request
"itemListElement":[]on startpage ifshowHomeis disabled in mod_breadcrumbs.Expected result AFTER applying this Pull Request
No errors in structured data when testing.
Additional
A legend is going around that the last element must not have a
@id(= forbidden) if it's the current page, which is wrong (I've tested that on several pages that use other tools for ld+json). Because Idon't want endless discussions here I've left the old behavior in mod_breadcrumbs (no
@idfor last element if it's current page).