From dcc9868129d4144a602761a8150594b4881f7370 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> Date: Fri, 3 Dec 2021 13:31:50 -0500 Subject: [PATCH] Centralize pagination/canonical meta URL generation in Document (#3077) * Centralize pagination/canonical meta URL generation in Document * Apply fixes from StyleCI [ci skip] [skip ci] * Use translations for title template * Apply fixes from StyleCI [ci skip] [skip ci] * Dont add translator to title driver interface It's an implementation detail, and can be made available on specific implementations as needed. Co-authored-by: Alexander Skvortsov --- locale/core.yml | 5 ++ src/Forum/Content/Discussion.php | 5 +- src/Forum/Content/Index.php | 3 ++ src/Frontend/Document.php | 68 +++++++++++++++++++++++- src/Frontend/Driver/BasicTitleDriver.php | 21 +++++++- 5 files changed, 99 insertions(+), 3 deletions(-) diff --git a/locale/core.yml b/locale/core.yml index c7bbf75c60..7c4812140c 100644 --- a/locale/core.yml +++ b/locale/core.yml @@ -628,6 +628,11 @@ core: submit_button: => core.ref.save_changes title: => core.ref.reset_your_password + # Translations in this namespace are used to format page meta titles. + meta_titles: + with_page_title: "{pageNumber, plural, =1 {{pageTitle} - {forumName}} other {{pageTitle}: Page # - {forumName}}}" + without_page_title: "{pageNumber, plural, =1 {{forumName}} other {Page # - {forumName}}}" + # Translations in this namespace are used in messages output by the API. api: invalid_username_message: "The username may only contain letters, numbers, and dashes." diff --git a/src/Forum/Content/Discussion.php b/src/Forum/Content/Discussion.php index 936e0e3289..9add89c2c5 100644 --- a/src/Forum/Content/Discussion.php +++ b/src/Forum/Content/Discussion.php @@ -97,10 +97,13 @@ public function __invoke(Document $document, Request $request) $hasNextPage = $page < 1 + intval($apiDocument->data->attributes->commentCount / 20); $document->title = $apiDocument->data->attributes->title; - $document->canonicalUrl = $url(['page' => $page]); $document->content = $this->view->make('flarum.forum::frontend.content.discussion', compact('apiDocument', 'page', 'hasPrevPage', 'hasNextPage', 'getResource', 'posts', 'url')); $document->payload['apiDocument'] = $apiDocument; + $document->canonicalUrl = $url([]); + $document->page = $page; + $document->hasNextPage = $hasNextPage; + return $document; } diff --git a/src/Forum/Content/Index.php b/src/Forum/Content/Index.php index ca4fe0e650..82ac0622e1 100644 --- a/src/Forum/Content/Index.php +++ b/src/Forum/Content/Index.php @@ -88,7 +88,10 @@ public function __invoke(Document $document, Request $request) $document->title = $this->translator->trans('core.forum.index.meta_title_text'); $document->content = $this->view->make('flarum.forum::frontend.content.index', compact('apiDocument', 'page')); $document->payload['apiDocument'] = $apiDocument; + $document->canonicalUrl = $this->url->to('forum')->base().($defaultRoute === '/all' ? '' : $request->getUri()->getPath()); + $document->page = $page; + $document->hasNextPage = isset($apiDocument->links->next); return $document; } diff --git a/src/Frontend/Document.php b/src/Frontend/Document.php index 037e4a61e9..b839fca195 100644 --- a/src/Frontend/Document.php +++ b/src/Frontend/Document.php @@ -95,6 +95,24 @@ class Document implements Renderable */ public $canonicalUrl; + /** + * Which page of content are we on? + * + * This is used to build prev/next meta links for SEO. + * + * @var null|int + */ + public $page; + + /** + * Is there a next page? + * + * This is used with $page to build next meta links for SEO. + * + * @var null|bool + */ + public $hasNextPage; + /** * An array of strings to append to the page's . * @@ -243,8 +261,17 @@ protected function makeHead(): string return ''; }, $this->css); + if ($this->page) { + if ($this->page > 1) { + $head[] = ''; + } + if ($this->hasNextPage) { + $head[] = ''; + } + } + if ($this->canonicalUrl) { - $head[] = ''; + $head[] = ''; } $head = array_merge($head, $this->makePreloads()); @@ -289,4 +316,43 @@ public function setForumApiDocument(array $forumApiDocument) { $this->forumApiDocument = $forumApiDocument; } + + public static function setPageParam(string $url, ?int $page) + { + if (! $page || $page === 1) { + return self::setQueryParam($url, 'page', null); + } + + return self::setQueryParam($url, 'page', (string) $page); + } + + /** + * Set or override a query param on a string URL to a particular value. + */ + protected static function setQueryParam(string $url, string $key, ?string $value) + { + if (filter_var($url, FILTER_VALIDATE_URL)) { + $urlParts = parse_url($url); + if (isset($urlParts['query'])) { + parse_str($urlParts['query'], $urlQueryArgs); + + if ($value === null) { + unset($urlQueryArgs[$key]); + } else { + $urlQueryArgs[$key] = $value; + } + + $urlParts['query'] = http_build_query($urlQueryArgs); + $newUrl = $urlParts['scheme'].'://'.$urlParts['host'].$urlParts['path'].'?'.$urlParts['query']; + } elseif ($value !== null) { + $newUrl = $url.'?'.http_build_query([$key => $value]); + } else { + return $url; + } + + return $newUrl; + } else { + return $url; + } + } } diff --git a/src/Frontend/Driver/BasicTitleDriver.php b/src/Frontend/Driver/BasicTitleDriver.php index 71cd8ec408..b872721492 100644 --- a/src/Frontend/Driver/BasicTitleDriver.php +++ b/src/Frontend/Driver/BasicTitleDriver.php @@ -12,13 +12,32 @@ use Flarum\Frontend\Document; use Illuminate\Support\Arr; use Psr\Http\Message\ServerRequestInterface; +use Symfony\Contracts\Translation\TranslatorInterface; class BasicTitleDriver implements TitleDriverInterface { + /** + * @var TranslatorInterface + */ + protected $translator; + + public function __construct(TranslatorInterface $translator) + { + $this->translator = $translator; + } + public function makeTitle(Document $document, ServerRequestInterface $request, array $forumApiDocument): string { $onHomePage = rtrim($request->getUri()->getPath(), '/') === ''; - return ($document->title && ! $onHomePage ? $document->title.' - ' : '').Arr::get($forumApiDocument, 'data.attributes.title'); + $params = [ + 'pageTitle' => $document->title ?? '', + 'forumName' => Arr::get($forumApiDocument, 'data.attributes.title'), + 'pageNumber' => $document->page ?? 1, + ]; + + return $onHomePage || ! $document->title + ? $this->translator->trans('core.views.meta_titles.without_page_title', $params) + : $this->translator->trans('core.views.meta_titles.with_page_title', $params); } }