Page MenuHomePhabricator

Componentize MenuLink and Footers
Closed, ResolvedPublic5 Estimated Story Points

Description

Background

Our menu code is complicated, and as part of our Q1 technical goal (T311647 as part of a larger effort to simplify skin code T263213) we will be simplifying this. This work is an important first step towards achieving the goal.

We've started the process of building components in core. This task will add menus as a core skin component.

Developer notes

  • Add a new component called SkinComponentMenuLink (or something similar e.g. SkinComponentPortletLink)
  • For now, the component should be a data array with a single html property. Use the existing Skin::makeLink to make it.
  • Update the Skin::makeLink function to use the new SkinComponentMenuLink component
$link = new SkinComponentMenuLink( ... );
return $link->getTemplateData()['html']

Proposed patch generates footer data using the new menu components and removes the data generation methods from the Skin and SkinTemplate classes into the Footer component class which is added to the SkinComponentRegistry and thereby automatically included in Skin::getTemplateData.

The new menu components replace the menu link generation in the Skin class. Notably Skin::MakeLink and Skin::MakeListItem leverage the new MenuLink and MenuListItem components respectively.

SkinTemplate::getFooterIcons was moved into the Skin class to pre-empt polymorphic calls when invoking Skin methods (for data creation) from within the Footer component and to pass unit tests.

Note that this data will eventually be replaced by data generation delivered by the core menu api T315014 (see attached POC patch). Many of the methods moved/created for footer data generation in this ticket/patch are temporary, will be removed, and greatly simplified with a call to the MenuBuilder service once it's ready like so:

class SkinComponentFooter implements SkinComponent {

	public function getTemplateData(): array {
		return $this->getFooterData();
	}

	/**
	 * Returns the footer menu data.
	 *
	 * @return array
	 */
	private function getFooterData(): array {
		$services = MediaWikiServices::getInstance();
		$footerMenuDirector = $services->getService( 'Menu.Director' );
		return $footerMenuDirector->getMenuData();
	}
}

QA/Sign off steps

'html' => $this->makeLink( $this->key, $this->item, $this->options )
  • Ensure that there are no visual regressions or changes in functionality in the footer or any of the menus in all skins (modern Vector, legacy Vector, web-maintained skins).

Event Timeline

I suggested to Clare yesterday that this would be a good task to look at after T301722

Jdlrobson renamed this task from Componentize MenuLink to Componentize MenuLink and Footers.May 9 2022, 6:14 PM

Change 808913 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[mediawiki/core@master] POC/WIP Componentize menulink + footer

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

Jdlrobson triaged this task as Medium priority.Jul 18 2022, 6:53 PM

After talking to @cjming we have decided to pull this into the sprint board towards our technical OKR.

cjming set the point value for this task to 5.
cjming removed cjming as the assignee of this task.Sep 2 2022, 6:53 PM
cjming moved this task from Doing to Code Review on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.
cjming subscribed.
Jdlrobson lowered the priority of this task from Medium to Low.Sep 6 2022, 5:28 PM

Change 831372 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[mediawiki/core@master] Move SkinTemplate::getFooterIcons() to Skin class

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

Change 831372 merged by jenkins-bot:

[mediawiki/core@master] Move SkinTemplate::getFooterIcons() to Skin class

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

cjming moved this task from Doing to Code Review on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.
cjming moved this task from Doing to Code Review on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.

Change 808913 merged by jenkins-bot:

[mediawiki/core@master] Componentize menulink + footer

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

Jdlrobson claimed this task.
Jdlrobson added a subscriber: Edtadros.

@cjming @Mabualruz looks like Pixel is complaining about some changes to the footer here: https://pixel.wmcloud.org/reports/mobile/index.html
I can replicate this when I load the Mobile site with mobile-frontend-terms-url page deleted.

This shouldn't impact production but will impact 3rd parties so we should fix this. I'll look into a patch.

Change 836316 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/MobileFrontend@master] Handle case where Terms of use link is null

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

Change 836316 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Handle case where Terms of use link is null

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

Jdlrobson updated the task description. (Show Details)