Page MenuHomePhabricator

Minerva skin should use the SkinMustache class
Closed, ResolvedPublic

Description

Minerva currently uses a hybrid of templates and MinervaTemplate. I am interested in collaborating with somebody who shares my interest to make it use the core SkinMustache class.

Vector will soon be using SkinMustache (see T251212) and it's important that the two most widely used skins (serving desktop and mobile traffic) reflect how we want to build skins going forward. This will help inform the template data provided by SkinMustache and a core skin API.

Proposed roadmap

(Feel free to create subtasks!)

  • A new public function MinervaTemplate::getTemplateData is refactored out of the MinervaTemplate::render function - it should take no arguments. The MinervaTemplate::render function should be merged with the execute function (see developer note 1 for how this should look)
  • SkinMinerva class is updated so it extends SkinMustache and calls MinervaTemplate::getTemplateData. MinervaTemplate::execute should have an empty body at this point.
  • Remove getFooterTemplateData https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/631867

Developer notes

[1] Proposed revised MinervaTemplate execute method:

	/**
	 * Start render the page in template
	 */
	public function execute() {
		$templateParser = new TemplateParser( __DIR__ );
		// begin rendering
		echo $templateParser->processTemplate( 'skin', $this->getTemplateData() );
		$this->printTrail();
		?>
		</body>
		</html>
		<?php
	}

[2] Proposed revised transitional SkinMinerva class

	/**
	 * Initialize Minerva Skin
	 */
	public function __construct( $options ) {
		$options['templateDirectory'] = __DIR__;
		$options['name'] = 'minerva';
		parent::__construct( $options );
		$this->skinOptions = MediaWikiServices::getInstance()->getService( 'Minerva.SkinOptions' );
	}

	public function getTemplateData() : array {
		$tpl = $this->prepareQuickTemplate();
		return parent::getTemplateData() + $tpl->getTemplateData();
	}

Event Timeline

Let me know if any clarifications are needed! @Ammarpad has been starting to upstream some changes to SkinMustache as part of T255924 and is very knowledgeable with all things skin related so please add him as a reviewer as well on anything you submit.

ovasileva lowered the priority of this task from High to Medium.
ovasileva added a subscriber: Peter.ovchyn.

Change 631865 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Minerva should use the SkinMustache class

https://gerrit.wikimedia.org/r/631865

Change 631867 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Simplify footer and logo generation

https://gerrit.wikimedia.org/r/631867

Change 631865 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Minerva should use the SkinMustache class

https://gerrit.wikimedia.org/r/631865

Change 631867 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Simplify footer and logo generation

https://gerrit.wikimedia.org/r/631867

Jdlrobson updated the task description. (Show Details)

Remaining work pulled out into T266072 which is clearer on the state. Minerva skin should use the SkinMustache class is resolved.