search

kirby-deprecated-plugins / kirby-seo

51 stars 11 forks source link

Output html tags or plain text #32

Open olach opened 7 years ago

olach commented 7 years ago

The logic for how to output the content should be changed.

Right now, the third parameter specifies whether the content should be returned or echoed. So far so good. But this parameter also specifies if the content should include the html tags or not.

To better comply with how Kirby does its things, my proposal would be to have this behavior instead:

seo('title') returns the content as plain text. I.e: title content

seo('title')->html() returns the content wrapped in html. I.e: <title>title content</title>

jenstornell commented 7 years ago

I agree that it looks better and will probably work better as well. However, I'm not sure about the old syntax at all. I hijack seo function so no one can use a function with that name.

A possible Kirby way would be:

<?php echo $page->seoTitle('html'); ?>

or

<title><?php echo $page->seoTitle(); ?></title>

But I'm not sure. While this is a Kirby way, it's a bit more to write.

olach commented 7 years ago

You can have a look at the PR #33 I just made. I solved it this way:

seo('title') returns the content wrapped in html. I.e: <title>title content</title>

seo('title')->value() returns the content as plain text. I.e: title content

olach commented 7 years ago

And as you write, a global seo() function might not be the best way to solve this. Maybe better to assign it to the $page object?

<?= $page->seo('title') ?>
<?= $page->seo('title')->value() ?>
jenstornell commented 7 years ago

Late reply, but I will continue this field when this is solved:

https://github.com/getkirby/panel/issues/943

olach commented 7 years ago

Good. Until then, I just encountered a problem where I need to output the SEO title from another page. Since you use a global seo function which is only for the current page, I cannot use this function. So a better approach would be to assign it to the $page object.

<?= $page->seo('title') ?>
jenstornell commented 7 years ago

Yes, $page->seo('title') looks good. I will probably rewrite most of this plugin with a more Kirby approach and better code structure. It's only been a year, but I've learned so much since I coded this thing.