Jdlrobson has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/381886 )
Change subject: WIP: Render book in one master template
......................................................................
WIP: Render book in one master template
This replaces all the templates with one single template.
I'd argue this improves the readability of what is happening here
while also exposing some problems with the current approach.
It also makes it easy for us to write unit tests.
TODO:
* the getters for template function should be refactored.
The call to $headingCounter->incrementAndGetTopLevel() adds additional
complexity that hurts readability and passing $metadata by reference
makes it difficult to change the order of rendering.
* It feels like getContributorHtml could be powered via template data
* Some of the getters probably can be moved into the renderBook method
Change-Id: I85b3cd938e3dbd0e5528027d67941ec4ed2c29be
---
M includes/BookRenderer.php
A templates/book.mustache
D templates/images.mustache
D templates/license.mustache
D templates/toc.mustache
5 files changed, 58 insertions(+), 55 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Collection
refs/changes/86/381886/1
diff --git a/includes/BookRenderer.php b/includes/BookRenderer.php
index 0bc4bfc..c2ccc20 100644
--- a/includes/BookRenderer.php
+++ b/includes/BookRenderer.php
@@ -96,12 +96,15 @@
}
}
- $final = $this->renderCoverAndToc( $collection, $metadata )
- . $final
- . $this->renderContributors( $metadata,
$headingCounter->incrementAndGetTopLevel() )
- . $this->renderImageInfos( $metadata,
$headingCounter->incrementAndGetTopLevel() )
- . $this->renderLicense( $metadata,
$headingCounter->incrementAndGetTopLevel() );
- return $final;
+ $data = $this->getCoverAndTocData( $collection, $metadata ) +
+ $this->getImageInfosData( $metadata ) +
+ $this->getLicenseData( $metadata ) + [
+ 'bookhtml' => $final,
+ 'imagesSectionNumber' =>
$headingCounter->incrementAndGetTopLevel(),
+ 'licenseSectionNumber' =>
$headingCounter->incrementAndGetTopLevel(),
+ 'contributorshtml' =>
$this->getContributorHtml( $metadata,
$headingCounter->incrementAndGetTopLevel() ),
+ ];
+ return $this->templateParser->processTemplate( 'book', $data );
}
/**
@@ -114,9 +117,9 @@
* - level: heading level or -2 for chapter, -1 for article
* - anchor: id of the document node which the outline item refers
to
* - number: a hierarchical section number (something like "1.2.3")
- * @return string HTML to prepend to the book.
+ * @return object template data to render book
*/
- private function renderCoverAndToc( $collection, &$metadata ) {
+ private function getCoverAndTocData( $collection, &$metadata ) {
$hasChapters = (bool)array_filter( $collection['items'],
function ( $item ) {
return $item['type'] === 'chapter';
} );
@@ -194,12 +197,12 @@
}
$metadata['outline'] = $outline;
- return $this->templateParser->processTemplate( 'toc',
$this->fixTemplateData( [
+ return $this->fixTemplateData( [
'title' => $collection['title'],
'subtitle' => $collection['subtitle'],
'toctitle' => wfMessage( 'coll-toc-title' )->text(),
'tocitems' => $this->getNestedOutline( $outline ),
- ] ) );
+ ] );
}
/**
@@ -208,7 +211,7 @@
* @param string $sectionNumber The section number for the contributors
section, if any.
* @return string HTML to append to the book.
*/
- private function renderContributors( $metadata, $sectionNumber = null )
{
+ private function getContributorHtml( $metadata, $sectionNumber = null )
{
$list = array_map( function ( $name ) {
return Html::element( 'li', [], $name );
}, array_keys( $metadata['contributors'] ) );
@@ -228,7 +231,7 @@
* @param string $sectionNumber The section number for the images
section, if any.
* @return string HTML to append to the book.
*/
- private function renderImageInfos( $metadata, $sectionNumber = null ) {
+ private function getImageInfosData( $metadata, $sectionNumber = null ) {
if ( !$metadata['images'] ) {
return '';
}
@@ -242,11 +245,10 @@
foreach ( $metadata['images'] as $image ) {
$images[] = array_merge( $image, $messages );
}
- return $this->templateParser->processTemplate( 'images', [
- 'sectionNumber' => $sectionNumber,
+ return [
'images' => $images,
- 'headingMsg' => wfMessage( 'coll-images-title' )->text()
- ] );
+ 'imagesHeadingMsg' => wfMessage( 'coll-images-title'
)->text()
+ ];
}
/**
@@ -255,15 +257,14 @@
* @param string $sectionNumber The section number for the images
section, if any.
* @return string HTML to append to the book.
*/
- private function renderLicense( $metadata, $sectionNumber = null ) {
+ private function getLicenseData( $metadata, $sectionNumber = null ) {
if ( !$metadata['license'] ) {
return '';
}
- return $this->templateParser->processTemplate( 'license', [
- 'sectionNumber' => $sectionNumber,
+ return [
'license' => $metadata['license'],
- 'headingMsg' => wfMessage( 'coll-license-title'
)->text()
- ] );
+ 'licenseHeadingMsg' => wfMessage( 'coll-license-title'
)->text()
+ ];
}
/**
diff --git a/templates/book.mustache b/templates/book.mustache
new file mode 100644
index 0000000..fb66633
--- /dev/null
+++ b/templates/book.mustache
@@ -0,0 +1,36 @@
+{{#title?}}
+<div class="cover-page">
+ <header>
+ <h1>{{title}}</h1>
+
{{#subtitle?}}<h2>{{subtitle}}</h2>{{/subtitle?}}
+ </header>
+</div>
+{{/title?}}
+
+<nav class="mw-book-toc">
+ <h1>{{toctitle}}</h1>
+ <ul>
+ {{#tocitems}}
+ {{>tocitem}}
+ {{/tocitems}}
+ </ul>
+</nav>
+{{{bookhtml}}}
+{{{contributorshtml}}}
+<h1 id="mw-book-images"
data-mw-sectionnumber={{imagesSectionNumber}}>{{imagesHeadingMsg}}</h1>
+<ul>
+ {{#images}}
+ <li>
+ <b>{{title}}</b>
+ <i>{{sourceMsg}}</i>: {{url}}
+ <i>{{licenseMsg}}</i>: {{credits}}
+ <i>{{artistMsg}}</i>: {{{artist}}} {{! artist may
contain safe HTML }}
+ </li>
+ {{/images}}
+</ul>
+<h1 id="mw-book-license"
data-mw-sectionnumber={{licenseSectionNumber}}>{{licenseHeadingMsg}}</h1>
+<div>
+ {{#license}}
+ <a href="{{url}}">{{text}}</a>
+ {{/license}}
+</div>
diff --git a/templates/images.mustache b/templates/images.mustache
deleted file mode 100644
index 3c49fdd..0000000
--- a/templates/images.mustache
+++ /dev/null
@@ -1,11 +0,0 @@
-<h1 id="mw-book-images"
data-mw-sectionnumber={{sectionNumber}}>{{headingMsg}}</h1>
-<ul>
- {{#images}}
- <li>
- <b>{{title}}</b>
- <i>{{sourceMsg}}</i>: {{url}}
- <i>{{licenseMsg}}</i>: {{credits}}
- <i>{{artistMsg}}</i>: {{{artist}}} {{! artist may
contain safe HTML }}
- </li>
- {{/images}}
-</ul>
diff --git a/templates/license.mustache b/templates/license.mustache
deleted file mode 100644
index bc97ac0..0000000
--- a/templates/license.mustache
+++ /dev/null
@@ -1,6 +0,0 @@
-<h1 id="mw-book-license"
data-mw-sectionnumber={{sectionNumber}}>{{headingMsg}}</h1>
-<div>
- {{#license}}
- <a href="{{url}}">{{text}}</a>
- {{/license}}
-</div>
diff --git a/templates/toc.mustache b/templates/toc.mustache
deleted file mode 100644
index a989bbe..0000000
--- a/templates/toc.mustache
+++ /dev/null
@@ -1,17 +0,0 @@
-{{#title?}}
-<div class="cover-page">
- <header>
- <h1>{{title}}</h1>
- {{#subtitle?}}<h2>{{subtitle}}</h2>{{/subtitle?}}
- </header>
-</div>
-{{/title?}}
-
-<nav class="mw-book-toc">
- <h1>{{toctitle}}</h1>
- <ul>
- {{#tocitems}}
- {{>tocitem}}
- {{/tocitems}}
- </ul>
-</nav>
--
To view, visit https://gerrit.wikimedia.org/r/381886
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I85b3cd938e3dbd0e5528027d67941ec4ed2c29be
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Collection
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
