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.

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

Support meta_title overrides from page model #58

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

srijan
Copy link

@srijan srijan commented Nov 13, 2023

WIP: Does not support preview yet.

Ref #37.

My use case: in some types of pages, I don't want to append title separator and site title to the page title.

One alternative implementation is to provide a boolean in the pageModel->metadata() function that controls whether to append the site title to the page title.

Let me know if you want me to change the implementation.

@fabianmichael
Copy link
Owner

@srijan Thanks for your contribution, I had this situation as well and have been looking for a better solution for quite some time. Your solution solves the problem quite well, though I am wondering whether we could find something a bit more elegant?

Since Kirby 4 will be released soon, I think a breaking change would be easier to justify at this point if this will land in a future release of the plugin.

@srijan
Copy link
Author

srijan commented Nov 17, 2023

Actually, I tested this with Kirby 4, and looks like it fails with some error, so we can hold off on this implementation for now.

Do you have any ideas on any better ways to implement it?

Note that this is still a hack. Don't merge!
@@ -74,7 +74,7 @@ public function get(
bool $siteFallback = false,
bool $configFallback = false,
mixed $fallback = null
): Field {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srijan Please add a "to do" comment, so I can add the type check again for later.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've replaced the Field namespace in the use section instead. This, I think, makes it incompatible with v3.

@srijan
Copy link
Author

srijan commented Nov 28, 2023

For reference, this is the error returned if I keep the Field return type:

TypeError thrown with message "FabianMichael\Meta\PageMeta::get(): Return value must be of type Kirby\Cms\Field, Kirby\Content\Field returned"

Stacktrace:
#37 TypeError in /var/www/html/site/plugins/meta/src/PageMeta.php:82
#36 FabianMichael\Meta\PageMeta:get in /var/www/html/site/plugins/meta/src/PageMeta.php:427
#35 FabianMichael\Meta\PageMeta:title in /var/www/html/site/plugins/meta/snippets/general.php:1
#34 include in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:425
#33 Kirby\Filesystem\F:loadIsolated in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:364
#32 Kirby\Filesystem\F:Kirby\Filesystem\{closure} in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:372
#31 Kirby\Filesystem\F:load in /var/www/html/vendor/getkirby/cms/src/Toolkit/Tpl.php:36
#30 Kirby\Toolkit\Tpl:load in /var/www/html/vendor/getkirby/cms/src/Template/Snippet.php:172
#29 Kirby\Template\Snippet:factory in /var/www/html/vendor/getkirby/cms/config/components.php:301
#28 Kirby\Cms\Core:{closure} in /var/www/html/vendor/getkirby/cms/src/Cms/App.php:1553
#27 Kirby\Cms\App:snippet in /var/www/html/vendor/getkirby/cms/config/helpers.php:531
#26 snippet in /var/www/html/site/plugins/meta/snippets/meta.php:8
#25 include in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:425
#24 Kirby\Filesystem\F:loadIsolated in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:364
#23 Kirby\Filesystem\F:Kirby\Filesystem\{closure} in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:372
#22 Kirby\Filesystem\F:load in /var/www/html/vendor/getkirby/cms/src/Toolkit/Tpl.php:36
#21 Kirby\Toolkit\Tpl:load in /var/www/html/vendor/getkirby/cms/src/Template/Snippet.php:172
#20 Kirby\Template\Snippet:factory in /var/www/html/vendor/getkirby/cms/config/components.php:301
#19 Kirby\Cms\Core:{closure} in /var/www/html/vendor/getkirby/cms/src/Cms/App.php:1553
#18 Kirby\Cms\App:snippet in /var/www/html/vendor/getkirby/cms/config/helpers.php:531
#17 snippet in /var/www/html/site/snippets/framework.php:7
#16 include in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:425
#15 Kirby\Filesystem\F:loadIsolated in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:364
#14 Kirby\Filesystem\F:Kirby\Filesystem\{closure} in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:372
#13 Kirby\Filesystem\F:load in /var/www/html/vendor/getkirby/cms/src/Toolkit/Tpl.php:36
#12 Kirby\Toolkit\Tpl:load in /var/www/html/vendor/getkirby/cms/src/Template/Snippet.php:248
#11 Kirby\Template\Snippet:render in /var/www/html/vendor/getkirby/cms/src/Template/Snippet.php:132
#10 Kirby\Template\Snippet:end in /var/www/html/vendor/getkirby/cms/config/helpers.php:152
#9 endsnippet in /var/www/html/site/templates/home.php:95
#8 include in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:425
#7 Kirby\Filesystem\F:loadIsolated in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:364
#6 Kirby\Filesystem\F:Kirby\Filesystem\{closure} in /var/www/html/vendor/getkirby/cms/src/Filesystem/F.php:372
#5 Kirby\Filesystem\F:load in /var/www/html/vendor/getkirby/cms/src/Toolkit/Tpl.php:36
#4 Kirby\Toolkit\Tpl:load in /var/www/html/vendor/getkirby/cms/src/Template/Template.php:163
#3 Kirby\Template\Template:render in /var/www/html/vendor/getkirby/cms/src/Cms/Page.php:1017
#2 Kirby\Cms\Page:render in /var/www/html/vendor/getkirby/cms/src/Cms/App.php:777
#1 Kirby\Cms\App:io in /var/www/html/vendor/getkirby/cms/src/Cms/App.php:1191
#0 Kirby\Cms\App:render in /var/www/html/public/index.php:18

@fabianmichael
Copy link
Owner

@srijan I am aware of this issue … the namespace of Field has changed with v4. I want to release a final version of the plugin for 3.x and a 2.0 version for Kirby 4 as soon as possible. But there’s no reason the remove the type check can be added again for the v4-specific version.

@fabianmichael
Copy link
Owner

@srijan Thanks, will look into this soon. Please have a bit of patience. Just released a 1.0.0 as final version for Kirby 3.

@srijan
Copy link
Author

srijan commented Nov 28, 2023

@srijan Thanks, will look into this soon. Please have a bit of patience. Just released a 1.0.0 as final version for Kirby 3.

Please don't feel pressured. Take your time 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants