shweshi / OpenGraph

A Laravel package to fetch Open Graph data of a website.
https://opengraph.shashi.dev
MIT License
154 stars 29 forks source link

include Title tag too? #78

Open jayenne opened 2 years ago

jayenne commented 2 years ago

Although not technically a meta tag, the title tag would be a handy addition as it is an arbitraty informative label too.

With a tiny addition between the $metadata decleration the $tags as $tag loop to create a$metadata['title'] key val pair, one could add the given title too.

        if($allMeta){
            $title = $doc->getElementsByTagName('title');
            $metadata['title'] = $title->length > 0 ? $title->item(0)->textContent : null ;
        }

By putting it here, should there be a metata tag called title too, it would override this value for the preffered one.

shweshi commented 2 years ago

@jayenne so, if metatag title is present it should return the metatag title else it should check for the title tag and return?

jayenne commented 2 years ago

Yes, (IMHO*), I've submitted a PR for this too.

*I think if a site goes to the trouble of including an meta title in addition to the title tag then I think it would be the meta title they'd rather be used. agree?

shweshi commented 2 years ago

@jayenne feature wise I agree this would be a valuable addition. Although since this package is specific to fetch meta tags. And ideally if the title tag is not present it shouldn't return. People might be using this if tests cases too.. to check if title is present, title is not present.

I looked into the PR I would suggest it would be better if we let the consumer know what is happening under the hood, that title is taken if meta title is not there. Also I would suggest if we let consumer decide whether they want title tag or not if meta title is not present.

Let me know your opinion.