Wip/extensibility

[der-On]

This pull request adds functionality to extend sitemap elements either by blocking theire creation or by extending them.

closes #5

[mjauvin]

Can you explain how these events can be used to make a full multi-lang sitemap? You need to be able to inject at a much earlier stage and you'll need to generate the page url for each locale.

[der-On]

Can you explain how these events can be used to make a full multi-lang sitemap? You need to be able to inject at a much earlier stage and you'll need to generate the page url for each locale.

For my use case it worked as i only needed to add xhtml:link nodes and attach a query string to each url, but one might actually need to pass additional data to the item to e.g. reference the original resource it was generated from. I will further investiage and adjust this PR.

[der-On]

@mjauvin I've removed the unessecary arguments and will be trying to extend the translate plugin to allow for localized page urls in the sitemap.

[der-On]

@mjauvin @LukeTowers I've experimented with the translate plugin to add localized links to the sitemap and came up with this code:

// add localized links to static pages in sitemap
Event::listen('winter.sitemap.makeUrlElement', function($definition, $xml, $pageUrl, $lastModified, $item, $url) {
    $baseUrl = rtrim(url('/'), '/');

    if ($item->type === 'all-static-pages' || $item->type === 'static-page') {
        $locales = array_keys(Locale::listAvailable());

        foreach($locales as $locale) {
            $localeUrl = $baseUrl . '/' . $locale . str_replace($baseUrl, '', $pageUrl);
            $linkElement = $xml->createElement('xhtml:link');
            $linkElement->setAttribute('rel', 'alternate');
            $linkElement->setAttribute('hreflang', $locale);
            $linkElement->setAttribute('href', $localeUrl);
            $url->appendChild($linkElement);
        }
    }
});

[mjauvin]

you can't build the url like this, it won't work in all scenarios. please check how I did it in my plugin.

[mjauvin]

What about cms pages with a translatable slug?

ref. https://github.com/mjauvin/wn-mlsitemap-plugin/blob/master/classes/Definition.php#L69

[mjauvin]

Another problem I see is that the URLs needs to be generated for all locales (not just have xhtml:link for each locale)

[der-On]

Another problem I see is that the URLs needs to be generated for all locales (not just have xhtml:link for each locale)

I'm not sure what is the best practise approach here. My quick research told me that using a canonical URL + the alternate URLs for the locales seems to be what google recommends nowdays.

[der-On]

you can't build the url like this, it won't work in all scenarios. please check how I did it in my plugin.

Just checked it, you basically pass localized URLs to the menu item. That could be a way. I would like to also pass a reference (like an ID or code) of the menu item down to the sitemap item, so one has a canonical reference to the original item when handling the events. Maybe merging both could be the way to go. This gives us localized URLs without the need of handling events but while creating menu items and it gives us a reference to the original item when handling the sitemap events.

[mjauvin]

Another problem I see is that the URLs needs to be generated for all locales (not just have xhtml:link for each locale)

I'm not sure what is the best practise approach here. My quick research told me that using a canonical URL + the alternate URLs for the locales seems to be what google recommends nowdays.

According to this document, it is a bit more complex than what you describe:

https://developers.google.com/search/docs/advanced/crawling/localized-versions#sitemap

[der-On]

@mjauvin I'm now passing the item reference to the events, this way the origin of the item can be retrieved within the event handler. I was also able to add localized urls in a sane way in the translate plugin:

$self = $this;

// add localized links to static pages in sitemap
Event::listen('winter.sitemap.makeUrlElement', function($definition, $xml, $pageUrl, $lastModified, $item, $itemReference, $urlElement) use ($self) {
    if ($item->type === 'all-static-pages' || $item->type === 'static-page') {
        $locales = array_keys(Locale::listAvailable());

        foreach($locales as $locale) {
            $linkElement = $xml->createElement('xhtml:link');
            $linkElement->setAttribute('rel', 'alternate');
            $linkElement->setAttribute('hreflang', $locale);
            $linkElement->setAttribute('href', $self->localeUrl($pageUrl, $locale));
            $urlElement->appendChild($linkElement);
        }
    }
});

[mjauvin]

@der-On if your PR allows creating the equivalent code linked below in the translate plugin, I'll approve and merge it:

https://github.com/mjauvin/wn-mlsitemap-plugin/blob/master/classes/Definition.php#L69-L97

[mjauvin]

See #10 for other plugins implementing Multilang functionality.

[LukeTowers]

Replaced by #11