dotkernel / frontend

DotKernel Frontend Application. Based on Mezzio microframework using Laminas components.
https://v5.dotkernel.net
MIT License
23 stars 5 forks source link

add canonical #462

Closed arhimede closed 3 months ago

arhimede commented 3 months ago

in the header of the page, add canonical tag

and see what happens when there is called an bad route

arhimede commented 3 months ago

issue connected to https://github.com/laminas/getlaminas.org/issues/191

bidi47 commented 3 months ago

tested with this right after the <title> tag <link rel="canonical" href="{{ url() }}" />

a bad route gives the error Attempting to use matched result when routing failed; aborting this confirms today's DK TSC meeting

the bigger issue is that using url() is not a good solution for these two pages https://v5.dotkernel.net/ and https://v5.dotkernel.net/page/home they are the exact same page, but url() will include the unique url for each, so we need manual mapping

see https://github.com/dotkernel/frontend/compare/issue-462?expand=1 for my solution the pages that don't have a manual canonical url are fine (the fallback is url()) and this also provides devs a way to handle custom canonical urls for more complex page setups

alexmerlin commented 3 months ago

What I meant during today's TSC meeting:

Extend your approach by allowing developers to specify the correct routeName when generating the canonical URL

<link rel="canonical" href="{{ url(routeName ?? null) }}" />

so they can set routeName where they want to overwrite the default behaviour

public function homeAction(): ResponseInterface
{
    return new HtmlResponse(
        $this->template->render('page::home', [
            'routeName' => 'home',
        ])
    );
}

home comes from the route name specified here.

If routeName is not provided when parsing a page, then the code would work as before.

NOTE

My approach does not fix the error reported on non-existing pages.

bidi47 commented 3 months ago

@alexmerlin, i agree. your solution is much cleaner than mine

the remaining issue is that twig won't accept any php code to check url() before it fails so it looks like Bogdan's solution can't be implemented here unless i'm mistaken?

alexmerlin commented 3 months ago

No, we cannot use his solution.

I wasted today a couple of hours trying to find a solution. My best shot was creating the below Twig extenstion, which would first check if the current request matches any route.

<?php

declare(strict_types=1);

namespace Frontend\App\Twig;

use Mezzio\Router\RouterInterface;
use Psr\Http\Message\ServerRequestInterface;
use Twig\Extension\AbstractExtension;
use Twig\TwigFunction;

class ExistingRouteExtension extends AbstractExtension
{
    public function __construct(
        private ServerRequestInterface $request,
        private RouterInterface $router
    ) {
    }

    public function getFunctions(): array
    {
        return [
            new TwigFunction('existingRoute', [$this, 'existingRoute']),
        ];
    }

    public function existingRoute(): bool
    {
        return $this->router->match($this->request)->isSuccess();
    }
}

and then we would test by using:

<link rel="canonical" href="{{ existingRoute() and url(routeName ?? null) }}" />

if existingRoute returns false, url would not get called so it would not throw an exception.

But, unfortunately it did not work because the routes are not (yet?) loaded when we call/register the new extension.

If we could somehow load the routes, this could be our best solution to this problem,


Another solution I thought about was overwriting Mezzio's UrlHelper with our own (which would wrap their __invoke method in a try-catch).

Yet again, this would not be a viable solution because their UrlHelper is marked as final - so even if this worked, this wouldn't be a long term solution.

alexmerlin commented 3 months ago

Though, I have another idea:

In src/App/templates/layout/default.html.twig, wrap the meta tag in a canonical block:

{% block canonical %}
<link rel="canonical" href="{{ url(routeName ?? null) and url() }}" />
{% endblock %}

By default all templates would parse it with it's default content, as seen above.


Then, the 404 template in src/App/templates/error/404.html.twig would parse an empty canonical block:

{% block canonical %}{% endblock %}

This way, the canonical meta tag would be parsed only for existing pages.