breadcrumbs icon indicating copy to clipboard operation
breadcrumbs copied to clipboard

http://schema.org/BreadcrumbList to "https"

Open piep14 opened this issue 3 years ago • 1 comments

Hello,

In the render() method, we find http and not https. Could you modify or can we override the method and how?

Thanks

public function render()
    {
        if (empty($this->breadcrumbs)) {
            return '';
        }

        $cssClasses = implode(' ', $this->breadcrumbsCssClasses);

        return '<'. $this->listElement . ' itemscope itemtype="http://schema.org/BreadcrumbList"'
                .' class="' . $cssClasses .'">'
                . $this->renderCrumbs()
                . '</'. $this->listElement .'>';
    }

piep14 avatar Aug 28 '20 07:08 piep14

Hi @piep14, sure, this update can be included - however I'm a little pressed for time so I would need you to just do a bit of research and check whether this is backwards compatible, so I know which version number to bump. Intuitively, I see no reason for this to break BC, but would appreciate a second set of eyes.

As for overriding - sure you can do that, just extend the class and override the method, something like this:

<?php

namespace App\Breadcrumbs;

use Creitive\Breadcrumbs as BaseBreadcrumbs;

class Breadcrumbs extends BaseBreadcrumbs
{
    /**
     * @inheritDoc
     */
    public function render()
    {
        if (empty($this->breadcrumbs)) {
            return '';
        }

        $cssClasses = implode(' ', $this->breadcrumbsCssClasses);

        return '<'. $this->listElement . ' itemscope itemtype="https://schema.org/BreadcrumbList"'
                .' class="' . $cssClasses .'">'
                . $this->renderCrumbs()
                . '</'. $this->listElement .'>';
    }
}

If you're using this package directly, you can then just use this new class instead of Creitive\Breadcrumbs\Breadcrumbs.

Otherwise, if you're using one of the Laravel integration packages, you'll also want to override the service provider - in fact the Laravel integration package contains nothing except a very tiny service provider which registers this class as a singleton, and adds an alias. So if you need this override, you can just remove the Laravel integration package and install this one directly, and write your own service provider, e.g.

<?php

namespace App\Breadcrumbs;

use App\Breadcrumbs\Breadcrumbs;
use Illuminate\Support\ServiceProvider;

class BreadcrumbsServiceProvider extends ServiceProvider
{
    /**
     * Register the service provider.
     *
     * @return void
     */
    public function register()
    {
        $this->app->singleton('breadcrumbs', function ($app) {
            return new Breadcrumbs();
        });

        $this->app->alias('breadcrumbs', 'App\Breadcrumbs\Breadcrumbs');
    }
}

And just put that into your config/app.php.

However, I personally much prefer using third-party packages that include good defaults, without having to override stuff, so it's best if we update this package, and then everyone can benefit from the improvement.

If you can just let me know what you think about backwards compatibility, I can make this change soon - you are of course free to submit a PR but it's just a one-character addition so it's probably easier if I just do it myself.

Thanks!

levacic avatar Aug 31 '20 22:08 levacic