Sitelet https://web.archive.org/web/20201208192044/https://github.com/octobercms/october/issues/5214
Skip to content

/ october

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.

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

Jump to bottom

Menu items don't support being provided custom properties anymore #5214

Open
ayumi-cloud opened this issue Jul 15, 2020 · 4 comments
Open

Menu items don't support being provided custom properties anymore #5214

ayumi-cloud opened this issue Jul 15, 2020 · 4 comments
Assignees
@bennothommo
Labels
Type: Bug good first issue help wanted
Milestone
v1.1.2

Comments

@ayumi-cloud
Copy link
Contributor

@ayumi-cloud ayumi-cloud commented Jul 15, 2020 •
edited

  • OctoberCMS Build: 467
  • PHP Version: 7.4

We had this code running in one of our plugins for quite a while, see example: https://octobertricks.com/tricks/register-custom-side-navigation-for-your-plugin

We believe it stopped working on v466 update (2 updates ago)

When adding the code line:

<?php

$sideMenuItems = BackendMenu::listSideMenuItems();
print_r($sideMenuItems);

?>

To see what's being passed through we see the following:

Array
(
    [example] => Backend\Classes\SideMenuItem Object
    (
        [code] => example
        [owner] => Acme.Plugin
        [label] => acme.plugin::lang.menu.example.title
        [icon] => fas fa-users-cog
        [iconSvg] =>
        [url] => https://www.example.com/acme/plugin/example
        [counter] =>
        [counterLabel] =>
        [badge] =>
        [order] => 30
        [attributes] => Array
    (
    )
        [permissions] => Array
        (
            [0] => acme.plugin.example
        )
    )
)

However, the Plugin.php file has the following:

'sideMenu' => [
    'example' => [
        'label'       => 'acme.plugin::lang.menu.example.title',
        'icon'        => 'fas fa-users-cog',
        'url'         => Backend::url('acme/plugin/example'),
        'permissions' => ['acme.plugin.example'],
        'group'       => 'something',
        'order'       => 30
    ],

Note the group = something - yet it is not being outputted now?

Because of this we are seeing the following code being run:

foreach ($sideMenuItems as $sideItemCode => $item){
    if(!property_exists($item, 'group'))
        $item->group = 'default';
}

So the menu nav is showing the word default at the top and below the submenu's are not working because the group is not being passed now.

Works fine in a previous version 465.
@LukeTowers LukeTowers added this to the v1.0.468 milestone Jul 15, 2020
@bennothommo
Copy link
Member

@bennothommo bennothommo commented Jul 16, 2020

@ayumi-cloud It appears that the October Trick in question was relying on a "hack" so to speak, in that previously we were allowing arbitrary configuration options for the navigation items. @Klaasie's work on #4929 has changed that as now the navigation configuration is converted into true classes that only take the official configuration options, and not just converted to standard objects, as per this method.

@LukeTowers I see two options at this juncture - we can either allow group to be defined for sub-items as the Settings navigation has set that precedent, or we could investigate a way to allow arbitrary configuration options to be added into the MainMenuItem and SideMenuItem classes.
@bennothommo
Copy link
Member

@bennothommo bennothommo commented Jul 16, 2020

My opinion is for the latter, as there's no real harm in including these values if people want to use them for custom navigation partials.
@LukeTowers
Copy link
Member

@LukeTowers LukeTowers commented Jul 17, 2020

@bennothommo I prefer the latter as well. See https://github.com/octobercms/october/pull/4929/files#r455474792 for how we can do that.
@bennothommo bennothommo modified the milestones: v1.0.468, v1.0.469 Jul 29, 2020
@LukeTowers LukeTowers modified the milestones: v1.1.0, v1.1.1 Sep 4, 2020
@github-actions
Copy link

@github-actions github-actions bot commented Nov 4, 2020

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue.
If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.
@LukeTowers LukeTowers changed the title `BackendMenu::listSideMenuItems();` not passing `group` property Menu items don't support being provided custom properties anymore Nov 20, 2020
@LukeTowers LukeTowers modified the milestones: v1.1.1, v1.1.2 Nov 20, 2020
@ayumi-cloud ayumi-cloud mentioned this issue Dec 7, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bennothommo bennothommo

Labels
Type: Bug good first issue help wanted
Projects
None yet
Milestone
v1.1.2
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
@LukeTowers @bennothommo @ayumi-cloud
You can’t perform that action at this time.