ralphjsmit / laravel-seo

A package to handle the SEO in any Laravel application, big or small.
https://ralphjsmit.com/laravel-seo-package
MIT License
631 stars 51 forks source link

getDynamicSEOData(): tags and section don't generate anything #62

Closed thehands79 closed 8 months ago

thehands79 commented 8 months ago

php 8.2.15 laravel/framework 10.43.0 ralphjsmit/laravel-seo 1.4.3

public function getDynamicSEOData(): SEOData
    {
        Log::debug($this->category->name); // content in logs
        Log::debug($this->tags->pluck('name')->toArray()); // content in logs

        return new SEOData(
            title: $this->title,
            description: Str::words($this->description, 30, ''),
            author: $this->user->name,
            section: $this->category->name, // no content in page
            tags: $this->tags->pluck('name')->toArray(), // no content in page
            schema: SchemaCollection::initialize()->addArticle(),
        );
    }
ralphjsmit commented 8 months ago

Hi @thehands79! Do you echo the Eloquent model in your Blade as well? Like {!! seo($model !!}?

thehands79 commented 8 months ago

Hi, yes that's right here's the code:

    @php
    $seo = seo()->for($advert);
    $pattern = '><!--[if BLOCK]><![endif]-->';
    $replacement = '>';
    $seo = str_replace($pattern, $replacement, $seo);
    $pattern = '&amp;';
    $replacement = '&';
    $seo = str_replace($pattern, $replacement, $seo);
    @endphp
    @push('seo')
    {!! $seo !!}
    @endpush

(I noticed that ampersands in URLs are being encoded too, hence the additional hacky code.)

ralphjsmit commented 8 months ago

Hi, thank you! I checked and the tags and section properties are only outputted when $SEOData->type === 'article. The default type is "website", but you can set it to article or other types for specific pages. The tags and section are only relevant for the OpenGraph article output and therefore not included. You can view the source code here.

In order to fix it, you can update the type to article:

        return new SEOData(
            type: 'article',
            // ..
        );
thehands79 commented 8 months ago

HI, thanks for the reply. It would be better if the README was explicit about which properties are available for which types (or did I miss that?). You could add a <table> with props as rows and types as columns.

ralphjsmit commented 8 months ago

Thanks for the idea! The docs already do say it, but it's not super-explicit:

Scherm­afbeelding 2024-02-08 om 15 09 47

I'm now a bit too busy to create a (Markdown) table, but if you have time, then you can definitely PR it 👍