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

Jump to bottom

[4.1] mod_breadcrumbs - convert microdata to JSON-LD #30727

Merged
merged 9 commits into from Oct 27, 2020
Merged

[4.1] mod_breadcrumbs - convert microdata to JSON-LD #30727

merged 9 commits into from Oct 27, 2020

Conversation

hans2103
Copy link
Contributor

@hans2103 hans2103 commented Sep 22, 2020 •
edited

Pull Request for Issue # .

Summary of Changes

With #25117 in mind and the possible adaptation of Spatie\SchemaOrg in mind I would like to prepare current breadcrumbs by converting the Microdata into JSON-LD structure.
But not only with that possible adaptation in mind... The JSON-LD format is the recommended format.
The HTML code will be cleaner when the data is moved to JSON-LD format
Another benefit of this change is that the current page will always be added to the data. This is not the case in the current situation with Hide Last Item is active.

Testing Instructions

  • Install Joomla with Sample Data Blog.
  • Note that there will be a mod_breadcrumbs on every page. example: http://your-joomla-4-site/index.php/blog/article
  • Inspect element and see the markup of mod_breadcrumbs
  • Apply this patch. No npm run something:something is required
  • Refresh the page and note the changed markup.

Actual result BEFORE applying this Pull Request

<nav role="navigation" aria-label="Breadcrumbs">
    <ol itemscope="" itemtype="https://schema.org/BreadcrumbList" class="mod-breadcrumbs breadcrumb">
        <li class="mod-breadcrumbs__here float-left">You are here: &nbsp;</li>
        <li itemprop="itemListElement" itemscope="" itemtype="https://schema.org/ListItem" class="mod-breadcrumbs__item breadcrumb-item">
            <a itemprop="item" href="/index.php" class="pathway"><span itemprop="name">Home</span></a>
            <meta itemprop="position" content="1">
        </li>
        <li itemprop="itemListElement" itemscope="" itemtype="https://schema.org/ListItem" class="mod-breadcrumbs__item breadcrumb-item">
            <a itemprop="item" href="/index.php/blog" class="pathway"><span itemprop="name">Blog</span></a>
            <meta itemprop="position" content="2">
        </li>
        <li aria-current="page" itemprop="itemListElement" itemscope="" itemtype="https://schema.org/ListItem" class="mod-breadcrumbs__item breadcrumb-item active">
            <span itemprop="name">Article</span>
            <meta itemprop="position" content="3">
        </li>
    </ol>
</nav>

Expected result AFTER applying this Pull Request

<nav role="navigation" aria-label="Breadcrumbs">
    <ol class="mod-breadcrumbs breadcrumb">
        <li class="mod-breadcrumbs__here float-left">You are here: &nbsp;</li>
        <li class="mod-breadcrumbs__item breadcrumb-item"><a href="/index.php" class="pathway"><span>Home</span></a></li>
        <li class="mod-breadcrumbs__item breadcrumb-item"><a href="/index.php/blog" class="pathway"><span>Blog</span></a></li>
        <li class="mod-breadcrumbs__item breadcrumb-item active"><span>Article</span></li>
    </ol>
</nav>

While inspecting the element and scroll to top and inspect element <head>. It should contain the following <script> tag.

    <script type="application/ld+json">
        {
            "@context": "https:\/\/schema.org",
            "@type": "BreadcrumbList",
            "itemListElement": [
                {
                    "@type": "ListItem",
                    "position": 1,
                    "item": {
                        "@id": "http:\/\/joomla4.test\/index.php",
                        "name": "Home"
                    }
                },
                {
                    "@type": "ListItem",
                    "position": 2,
                    "item": {
                        "@id": "http:\/\/joomla4.test\/index.php\/blog",
                        "name": "Blog"
                    }
                },
                {
                    "@type": "ListItem",
                    "position": 3,
                    "item": {
                        "@id": "http:\/\/joomla4.test\/index.php\/blog\/article",
                        "name": "Article"
                    }
                }
            ]
        }    </script>

Documentation Changes Required

@hans2103 hans2103 changed the title ♻️ convert microdata to JSON-LD [4.0] mod_breadcrumbs - convert microdata to JSON-LD Sep 22, 2020
@HLeithner
Copy link
Member

We are in feature freeze for j4 please rebase to j4.1

@hans2103
Copy link
Contributor Author

@HLeithner can you help me how I rebase to 4.1?

@HLeithner
Copy link
Member

You can try to just target the 4.1-dev branch in github (you can edit this PR and select another target branch).

Depending on how long it takes to merge and if 4.1-dev is uptodate it could work.

Else: https://git-scm.com/book/en/v2/Git-Branching-Rebasing

But it's maybe easier to create a new branch of the 4.1 branch and copy paste.

@hans2103 hans2103 changed the base branch from 4.0-dev to 4.1-dev September 22, 2020 09:16
@hans2103
Copy link
Contributor Author

changed base.... already checked that mod_breadcrumbs/tmpl/default.php J4 = J4.1

@richard67
Copy link
Member

Bad news: Because the 4.1-dev branch is not up to date with latest changes in the 4.0-dev branch, this PR now shows all changes in 4.0-dev since the 4.1-dev was updated to 4.0-dev last time, and not only the changes from this PR.

Maybe that will be fixed by just waiting for the next update of the 4.1-dev branch from 4.0-dev, but I'm not sure about that.

@HLeithner
Copy link
Member

@zero-24 or @wilsonge can you please update the branch, I'm unable at the moment to do this.

@HLeithner
Copy link
Member

@hans2103 keep it as it is for the moment.

@ceford
Copy link
Contributor

ceford commented Sep 22, 2020

Using Patchtester I get this:

The file marked for modification does not exist: administrator/components/com_media/resources/scripts/components/toolbar/toolbar.vue

Is this to do with the base change?

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30727.

@richard67
Copy link
Member

@ceford Yes, you should wait until the 4.1-dev branch is up to date, and then you can test this PR only on a 4.1 and not on a 4.0 installation.

@wilsonge wilsonge changed the base branch from 4.1-dev to 4.0-dev September 22, 2020 14:29
@wilsonge wilsonge changed the base branch from 4.0-dev to 4.1-dev September 22, 2020 14:29
@wilsonge
Copy link
Contributor

Should be good now - pushed it up into 4.1-dev (needed to swap the branches around to make github realise)

@richard67
Copy link
Member

Drone seems to have a problem. I think when adding a change (e.g. those requested by review above) it will be fixed.

@hans2103 hans2103 changed the title [4.0] mod_breadcrumbs - convert microdata to JSON-LD [4.1] mod_breadcrumbs - convert microdata to JSON-LD Sep 23, 2020
@joomlacorner
Copy link
Contributor

I have tested this item ✅ successfully on 4710faf

Worked as Expected result.

'''

  1. You are here:   
    2. 	<li class="mod-breadcrumbs__item breadcrumb-item"><a href="/index.php" class="pathway"><span>Home</span></a></li><li class="mod-breadcrumbs__item breadcrumb-item"><a href="/index.php/blog" class="pathway"><span>Blog</span></a></li><li class="mod-breadcrumbs__item breadcrumb-item active"><span>Welcome to your blog</span></li>	</ol>
	<script type="application/ld+json">
{"@context":"https:\/\/schema.org","@type":"BreadcrumbList","itemListElement":[{"@type":"ListItem","position":1,"item":{"@id":"https:\/\/j4.akarawuth.com\/index.php","name":"Home"}},{"@type":"ListItem","position":2,"item":{"@id":"https:\/\/j4.akarawuth.com\/index.php\/blog","name":"Blog"}},{"@type":"ListItem","position":3,"item":{"@id":"https:\/\/j4.akarawuth.com\/index.php\/blog\/3-welcome-to-your-blog","name":"Welcome to your blog"}}]}	</script>
'''
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30727.

hans2103 and others added 2 commits October 1, 2020 21:08
@hans2103 @SharkyKZ
Update modules/mod_breadcrumbs/tmpl/default.php
f01c729 
Co-authored-by: SharkyKZ <sharkykz@gmail.com>
@hans2103 @SharkyKZ
Update modules/mod_breadcrumbs/tmpl/default.php
be55d71 
Co-authored-by: SharkyKZ <sharkykz@gmail.com>
@adj9
Copy link

adj9 commented Oct 24, 2020

I have tested this item ✅ successfully on fdf12e3

done :)!

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30727.

@hans2103
Copy link
Contributor Author

hans2103 commented Oct 25, 2020 •
edited

@Gostn hve you checked the 'head' element?

@hans2103
Copy link
Contributor Author

@Gostn I see...
with commit 2fbe114 I have moved the json block to the <head> of the document. Suggested by @SharkyKZ

Just adjusted the test instructions. Can you test again?

@richard67
Copy link
Member

RTC

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30727.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 25, 2020
@richard67 richard67 added this to the Joomla 4.1 milestone Oct 25, 2020
@rdeutz rdeutz merged commit 59319ff into joomla:4.1-dev Oct 27, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 27, 2020
@hans2103 hans2103 deleted the feature/mod_breadcrumbs branch October 27, 2020 15:09
@crystalenka crystalenka mentioned this pull request Jan 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SharkyKZ SharkyKZ SharkyKZ left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Joomla 4.1.0
Development

Successfully merging this pull request may close these issues.

None yet