menu: add Favorites section#10033
Conversation
Greelan
force-pushed
the
favorites-refactor
branch
from
35e4431 to
c50a78a
Compare
|
|
||
| if (!empty($username)) { | ||
| try { | ||
| $cfg = Config::getInstance(); |
There was a problem hiding this comment.
We should try to avoid direct config access, I noticed the dashboard had a similar issue, so I just refactored that in 3c3f2c4
When we build the dashboard, the User model didn't exist yet, but is backwards compatible.
There was a problem hiding this comment.
OK, I've refactored to mirror
|
|
||
| // per-user favorites - generating requires getItems() call | ||
| $favorites = new \OPNsense\Core\Favorites($_SESSION['Username'] ?? ''); | ||
| $this->view->menuFavorites = $favorites->buildFavoritesEntries($this->view->menuSystem); |
There was a problem hiding this comment.
We should be able to inject entries in a similar way as Interfaces are being injected, but it might be better to refactor some code first there as we also have the issue of menu items being conditionally being registered for isc-dhcp.
|
@Greelan don't mind my comments too much, I'm reviewing your PR a bit and leaving some pointers of things we should look at, no direct action is required. |
No problem, I've made some conforming changes to address the first comment, so hopefully that helps |
|
@Greelan it will take some time to ingest this as I'm mostly working on this in my free time and it does need some changes in our code to ease the proces as well, but I do like your idea, the code just needs some work on both ends (the feature and hooks to inject this) |
|
Absolutely fine, I was already conscious that this is lower priority given it is not on the roadmap, and you and others have plenty of other things to do. My commits and comments aren't intended to be pushy xD |
|
@AdSchellevis no doubt this was your intent in part, but I assume that I could use the new |
|
@Greelan I'm afraid we can't do this in the frontend as we need to modify the menu as well, let me think on this for a while, it's still on the radar, just busy :) |
|
All good, my thinking was just around rendering of the favorites entries and the star icons |
…ing parts out of the MenuSystem class In most cases we use static menu registartions, but there are exceptions which depend on interfaces for example. While looking at #10033, a longer standing wish came up again, which is the reason to add this support right now. It also helps in removing some legacy components for good via plugins. To register new menu items, the following pattern may be used: * In your model, derive a Menu class from MenuContainer * implement a method collect() which should add new menu items via the appendItem() {bound to appendItem in MenuSystem} Always try to minimize the amount of code inside these plugins as this code will be executed on each page load.
…ing parts out of the MenuSystem class In most cases we use static menu registartions, but there are exceptions which depend on interfaces for example. While looking at #10033, a longer standing wish came up again, which is the reason to add this support right now. It also helps in removing some legacy components for good via plugins. To register new menu items, the following pattern may be used: * In your model, derive a Menu class from MenuContainer * implement a method collect() which should add new menu items via the appendItem() {bound to appendItem in MenuSystem} Always try to minimize the amount of code inside these plugins as this code will be executed on each page load.
Greelan
force-pushed
the
favorites-refactor
branch
from
6580299 to
055dff6
Compare
…ing parts out of the MenuSystem class In most cases we use static menu registartions, but there are exceptions which depend on interfaces for example. While looking at #10033, a longer standing wish came up again, which is the reason to add this support right now. It also helps in removing some legacy components for good via plugins. To register new menu items, the following pattern may be used: * In your model, derive a Menu class from MenuContainer * implement a method collect() which should add new menu items via the appendItem() {bound to appendItem in MenuSystem} Always try to minimize the amount of code inside these plugins as this code will be executed on each page load.
…ing parts out of the MenuSystem class (#10113) * MVC: add support for pluggable dynamic menu items and move some existing parts out of the MenuSystem class In most cases we use static menu registartions, but there are exceptions which depend on interfaces for example. While looking at #10033, a longer standing wish came up again, which is the reason to add this support right now. It also helps in removing some legacy components for good via plugins. To register new menu items, the following pattern may be used: * In your model, derive a Menu class from MenuContainer * implement a method collect() which should add new menu items via the appendItem() {bound to appendItem in MenuSystem} Always try to minimize the amount of code inside these plugins as this code will be executed on each page load. * menusystem: simplify class generation for links and add some support to be used for favorites. This commit offers a couple of things, first of all it adds a unique class name on every traversable menu item so we can easily use javascript to pass a click event, for example: $(".menu_ref_7e46272fe380827861cbaf5b484c43c9")[0].click() We need this in order to link an item, but not use its actual uri in our href later, as this would confuse the "active" item selected. Next, we offer the option to inject an additional link class ( e.g. $this->appendItem(..,..,['linkclass' => 'my_link_class']); ), as this offers a way to register on the "favorite" buttons. Finally we cleanup the menu code a bit so the volt and legacy template contain less logic.
… and JS wrapper for breadcrumb labelling Requires full testing
2 participants
Implements a Favorites section:
Some sample screenshots attached.
Supersedes #10028
Depends on 1c2405e