Skip to content

MVC: add support for pluggable dynamic menu items and move some existing parts out of the MenuSystem class #10113

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.

Sign up for GitHub

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

Merged
AdSchellevis merged 2 commits into from
Apr 21, 2026
Merged

MVC: add support for pluggable dynamic menu items and move some existing parts out of the MenuSystem class #10113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4ee4d2
MVC: add support for pluggable dynamic menu items and move some exist…
AdSchellevis Apr 9, 2026
f1ba1bd
menusystem: simplify class generation for links and add some support …
AdSchellevis Apr 16, 2026
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
3 changes: 3 additions & 0 deletions plist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,7 @@
/usr/local/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/UpdateOnlyTextField.php
/usr/local/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/UrlField.php
/usr/local/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/VirtualIPField.php
/usr/local/opnsense/mvc/app/models/OPNsense/Base/Menu/MenuContainer.php
/usr/local/opnsense/mvc/app/models/OPNsense/Base/Menu/MenuInitException.php
/usr/local/opnsense/mvc/app/models/OPNsense/Base/Menu/MenuItem.php
/usr/local/opnsense/mvc/app/models/OPNsense/Base/Menu/MenuSystem.php
Expand Down Expand Up @@ -768,6 +769,7 @@
/usr/local/opnsense/mvc/app/models/OPNsense/Firewall/Filter.xml
/usr/local/opnsense/mvc/app/models/OPNsense/Firewall/Group.php
/usr/local/opnsense/mvc/app/models/OPNsense/Firewall/Group.xml
/usr/local/opnsense/mvc/app/models/OPNsense/Firewall/Menu/Menu.php
/usr/local/opnsense/mvc/app/models/OPNsense/Firewall/Menu/Menu.xml
/usr/local/opnsense/mvc/app/models/OPNsense/Firewall/Migrations/M1_0_0.php
/usr/local/opnsense/mvc/app/models/OPNsense/Firewall/Migrations/MFP1_0_0.php
Expand Down Expand Up @@ -828,6 +830,7 @@
/usr/local/opnsense/mvc/app/models/OPNsense/Interfaces/Lagg.xml
/usr/local/opnsense/mvc/app/models/OPNsense/Interfaces/Loopback.php
/usr/local/opnsense/mvc/app/models/OPNsense/Interfaces/Loopback.xml
/usr/local/opnsense/mvc/app/models/OPNsense/Interfaces/Menu/Menu.php
/usr/local/opnsense/mvc/app/models/OPNsense/Interfaces/Menu/Menu.xml
/usr/local/opnsense/mvc/app/models/OPNsense/Interfaces/Migrations/SET1_0_0.php
/usr/local/opnsense/mvc/app/models/OPNsense/Interfaces/Neighbor.php
Expand Down
49 changes: 49 additions & 0 deletions src/opnsense/mvc/app/models/OPNsense/Base/Menu/MenuContainer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php

/*
* Copyright (C) 2026 Deciso B.V.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* 1. Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
*
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
* INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
* AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
* OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/

namespace OPNsense\Base\Menu;

abstract class MenuContainer
{
private ?MenuSystem $menusystem = null;
Copy link
Copy Markdown
Contributor

@Greelan Greelan Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this is private, how will the favorites code access getItem() to build the breadcrumb labels for the Favorites menu?

Copy link
Copy Markdown
Member Author

@AdSchellevis AdSchellevis Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Greelan see appendItem() examples in Firewall and Interfaces

Copy link
Copy Markdown
Contributor

@Greelan Greelan Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've looked at those. I think the favorites scenario is different, because the labels can't just be read from the config, instead they need to be built from the existing menu tree (eg a favorite pointing to /ui/firewall/filter should show as Firewall: Rules [new]). Perhaps I'm missing something?

Copy link
Copy Markdown
Member Author

@AdSchellevis AdSchellevis Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might need some additional logic to prevent the favorite item needing the same uri as the actual target, but we'll see it when we'll get there. One of the options might be to just store a (md5) hash of the link and reflect the click action for example.

The favorite description itself could be part of the stored content to prevent the need to render the favorites if everything else is already there (since other dynamic content may have been set to favorite as well).

Copy link
Copy Markdown
Contributor

@Greelan Greelan Apr 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree it will need further thinking down the track. I wanted to avoid storing the label in case it became stale (menu item breadcrumbs changed).

Saying that though made me realise that there is a similar issue - though less likely - for the endpoint itself, eg when a MVC migration occurs. It's a bit different since the pruning logic will clean it up, but the consequence is that the menu item will be un-favourited. Not fatal but not ideal from a UX perspective.

Copy link
Copy Markdown
Member Author

@AdSchellevis AdSchellevis Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Greelan this f1ba1bd is likely the missing part for the favorites. The commit message contains some context, with this you can easily reference menu items and should be able to capture clicks on the favorite menu items (and pass them along)

Copy link
Copy Markdown
Contributor

@Greelan Greelan Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I will have a closer look at it when I get a chance.

Copy link
Copy Markdown
Contributor

@Greelan Greelan Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've now spent some time with this and have done some re-work to use the pluggable system, link classes, and JS wrapper. Will need to wait until the next OPNsense release for testing as it is too challenging to patch an existing installation with the needed upstream commits, given they build on other commits as well

Copy link
Copy Markdown
Member

@fichtner fichtner Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI I don't think this will land in 26.1.7 next week but 26.1.8 is very likely.


public function __construct(MenuSystem $menusystem)
{
$this->menusystem = $menusystem;
}

public function appendItem($root, $id, $properties)
{
return $this->menusystem->appendItem($root, $id, $properties);
}

public function collect()
{
return;
}
}
58 changes: 58 additions & 0 deletions src/opnsense/mvc/app/models/OPNsense/Base/Menu/MenuItem.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ class MenuItem
*/
private $CssClass = "";

/**
* Classes to add to the link
* @var string
*/
private $LinkClass = "";


/**
* link to url location
* @var string
Expand Down Expand Up @@ -94,6 +101,12 @@ class MenuItem
*/
private $selected = false;

/**
* Tree depth
* @var int
*/
private $depth = 0;

/**
* class method getters
* @var array
Expand Down Expand Up @@ -141,6 +154,9 @@ public function __construct($id, $parent = null)
$this->id = $id;
$this->visibleName = gettext($id);
$this->parent = $parent;
if ($parent !== null) {
$this->depth = $parent->getDepth() + 1;
}
$prop_exclude_list = ['getXmlPropertySetterName' => true];
if (self::$internalClassMethodAliases === null) {
self::$internalClassMethodAliases = [];
Expand Down Expand Up @@ -169,6 +185,14 @@ public function getId()
return $this->id;
}

/**
* return this nodes depth in the menu
*/
public function getDepth()
{
return $this->depth;
}


/**
* set sort order
Expand Down Expand Up @@ -242,6 +266,40 @@ public function getCssClass()
return $this->CssClass;
}

/**
* setter for default link class
* @param $value
*/
public function setLinkClass($value)
{
$this->LinkClass = $value;
}


/**
* getter for css class set on the actual link
*/
public function getLinkClass()
{
$css = ['list-group-item'];
if (count($this->children) >= 1 && $this->depth < 3) {
if ($this->selected) {
$css[] = 'active-menu-title';
}
} else {
if ($this->depth == 3) {
$css[] = 'menu-level-3-item';
}
if ($this->selected) {
$css[] = 'active';
}
}
if ($this->Url != '') {
$css[] = 'menu_ref_'.md5($this->Url);
}
return implode(' ', $css) . " " . $this->LinkClass;
}

/**
* setter for url field
* @param $value
Expand Down
Loading