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

Centralize pagination/canonical meta URL generation in Document #3077

Merged
merged 5 commits into from
Dec 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions locale/core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
5 changes: 4 additions & 1 deletion src/Forum/Content/Discussion.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 3 additions & 0 deletions src/Forum/Content/Index.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
68 changes: 67 additions & 1 deletion src/Frontend/Document.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 <head>.
*
Expand Down Expand Up @@ -243,8 +261,17 @@ protected function makeHead(): string
return '<link rel="stylesheet" href="'.e($url).'">';
}, $this->css);

if ($this->page) {
if ($this->page > 1) {
$head[] = '<link rel="prev" href="'.e(self::setPageParam($this->canonicalUrl, $this->page - 1)).'">';
}
if ($this->hasNextPage) {
$head[] = '<link rel="next" href="'.e(self::setPageParam($this->canonicalUrl, $this->page + 1)).'">';
}
}

if ($this->canonicalUrl) {
$head[] = '<link rel="canonical" href="'.e($this->canonicalUrl).'">';
$head[] = '<link rel="canonical" href="'.e(self::setPageParam($this->canonicalUrl, $this->page)).'">';
}

$head = array_merge($head, $this->makePreloads());
Expand Down Expand Up @@ -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;
}
}
}
21 changes: 20 additions & 1 deletion src/Frontend/Driver/BasicTitleDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}