@pmiazga any updates on this? I assume you are thinking about this while implementing T216152 - if so would it make sense for us to agree a timebox on it and move this into the sprint?

After talking to Piotr today, it sounds like he's doing this as part of T216152 so I'm pulling into the sprint so we make sure to capture those findings.

Proposal:

Use the same structure to build all menus in the system. We have pretty good Menus implementation and a Hook for registering new Menu elements. Instead of rewriting the system we could tweak a bit existing implementation, and write a reliable system for building menus. There are two reasons for that choice:

• BlueSpiceMultiUpload and GrowthExperiments already use MobileMenu hook to create MenuEntry objects
• we don't have enough time to refactor existing Menu system, the existing system allows us to play with it a little and make it much easier to extend.

To represent the menu elements, we need two objects:

• MenuElement -> it's already there, represents a single menu item that can contain one, or more components (links, most of the menu entries have only one component, but entries like User item in MainMenu has two (UserPage link and logout link)
• Group (previously called MenuBuilder). The Group is a collection of MenuEntries. Menus can have only one Group (a single list of menu elements), other menus like MainMenu have three/four sets of options, divided with a separator. Instead of creating a thing called "MenuSeparator", we can pass an array of Group objects. Each Group of elements should be rendered as one "chunk", and then we can have a space between different groups, so it looks like a separator.

The best way to build menus is to use Builder pattern.

• each Menu can have one Director, a class that knows how to build a menu.
• a Strategy, that based on the user, user preferences site configuration picks up the proper Builder class, for example, if the user opted into AMC, use AdvancedMainMenuBuilder builder instead of DefaultMainMenuBuilder
• The Director (with proper Builder) should be registered as Service via ServiceWirings file.
• when building menu elements, Director should call MobileMenu hook to allow extensions modify t

Menu specification (add as required):

It should be possible for an extension to register a menu item in a certain menu without any knowledge of how the menu is rendered.

It's already possible via MobileMenu hook. Some extensions are already using it.

A menu can be enabled in a certain mode or disabled in a certain mode e.g. AMC

Instead of having a complicated tree of if statements, we should aim for a Builder pattern. The Director class should know how to Build menus. Then,
we need a strategy (most probably somewhere in ServiceWiring) that provides proper Bilder object.

A menu can have completely different contents in a given mode e.g. contributions might be in left menu on stable but in a new user menu that shows on hover in AMC mode

Builder pattern allows us to do such things

Editors can update URIs of menu items e.g. On Commons, Special:Random can be replaced with Special:Random/file (see T188697)

Once we have Builders, we can implement a special Builder that reads a WikiPage and returns menu entries definitions for a menu. Each menu has a concrete Director implementation that knows how to build menu object,
and then multiple Builders that provide a concrete implementation for a specific type of menu

A menu itself be made of multiple menus e.g. left hamburger navigation has 4 menus right now in stable

To simplify this step we can reuse the existing structure. A Menu is a collection of Groups, each Group is a collection fo MenuEntries, each MenuEntry is a collection of components. The only necessary change is to modify the Group::insert() to accept MenuElements and Group objects. This allows us to implement submenus.

A menu uses a mustache template for rendering so that it can also be used on the client.

It's already here, the only problem is that the Menu building and rendering are located in different places and it makes difficult to replace templates. We need to refactor the code, so MenuBuilding and rendering logic is in one place

Menus should work without JS

It's already implemented. All menus have templates. The old MenuBuilder (Group now) has getEntries() method that returns the menu in raw format (an array representation). This array is used by the JS (to render it on the client side), and in MinervaTemplate to render it server side.

A menu can be converted to a JSON of template properties so that it can be rendered by JavaScript via Mustache

It's already implemented.

It should be possible for an extension to register a menu item in a certain menu without any knowledge of how the menu is rendered.

This is still the case. @pmiazga's change maintained the hooks that were run for the "sections" of the main menu:

• https://github.com/wikimedia/mediawiki-skins-MinervaNeue/commit/1f4582cc09773ea0bf5260beedceb70002c8aab4#diff-d7e345a3188f682da968087270bc0621R65
• https://github.com/wikimedia/mediawiki-skins-MinervaNeue/commit/1f4582cc09773ea0bf5260beedceb70002c8aab4#diff-0b42c8874a096155a4bf5b3c74259966R91
• https://github.com/wikimedia/mediawiki-skins-MinervaNeue/commit/1f4582cc09773ea0bf5260beedceb70002c8aab4#diff-0b42c8874a096155a4bf5b3c74259966R115
• https://github.com/wikimedia/mediawiki-skins-MinervaNeue/commit/1f4582cc09773ea0bf5260beedceb70002c8aab4#diff-0b42c8874a096155a4bf5b3c74259966R148

A menu can have completely different contents in a given mode e.g. contributions might be in left menu on stable but in a new user menu that shows on hover in AMC mode

@pmiazga's change achieves this for the main menu and it's easy to see how the pattern might be reused for other menus, e.g. https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/502641/

A menu itself be made of multiple menus e.g. left hamburger navigation has 4 menus right now in stable
A menu uses a [Mustache] template for rendering so that it can also be used on the client.
Menus should work without JS

IIRC these were properties of the pre-existing system in Minerva, which @pmiazga reused a lot of.

A menu can be converted to a JSON of template properties so that it can be rendered by JavaScript via Mustache

I removed this AC in T214715#5134345 because it's a restatement of the following AC:

A menu uses a mustache template for rendering so that it can also be used on the client.
Menus should work without JS

Editors can update URIs of menu items e.g. On Commons, Special:Random can be replaced with Special:Random/file (see T188697)

This is certainly possible given @pmiazga's refactoring, e.g. you may create a specialised builder, CommonsBuilder extends DefaultBuilder, which overrides the ::getDiscoveryTools method.

Relatedly, I note that @pmiazga is currently making changes to how the community portal customisable link based on feedback in T216152#5123528.

Editors can update URIs of menu items e.g. On Commons, Special:Random can be replaced with Special:Random/file (see T188697)

@phuedx - this part is bit unclear, it's possible to do by the same way as we're doing the Community Portal link. It's currently possible in at least three ways:

• define new Builder that extends the Default one (as you stated in the comment above)
• override the Definitions::insertRandomItem() to use the message cache. Instead of hardcoding Special:Random, we can load the title from Mediawiki:Random-url
• create a new builder that implements MediaWiki\Minerva\Menu\Main\IBuilder. The CustomBuilder::getGroups() can read a wiki page and parse it in similar manner as Skin:buildSidebar() does and return an array of Menu\Group elements.

By "unclear" I mean, do we want to do it right now? Thanks to new system and clear separation between Skin class, Director and concrete builder classes it's much easier to modify a single menu element (or group of elements).

In T214715#5134590, @pmiazga wrote:

Editors can update URIs of menu items e.g. On Commons, Special:Random can be replaced with Special:Random/file (see T188697)

• override the Definitions::insertRandomItem() to use the message cache. Instead of hardcoding Special:Random, we can load the title from Mediawiki:Random-url

I actually think that this is the best solution for that task (lest we continue toing and froing about parsing MediaWiki:Sidebar).

👍