verbb / cp-nav

Control Panel Nav is a Craft CMS plugin to help manage your Control Panel navigation.
MIT License
130 stars 11 forks source link

Missing subnav items for plugins with custom labels #43

Closed deling closed 6 years ago

deling commented 6 years ago

Description

CpNavService::modifyCpNav() makes the assumption that the handle of a navigation item is the same as its url in the cpnav_navigation table. This is true most of the time, but some plugins, like sprout-lists, allow the user to set a custom label for navigation items, which is causing subnav items to be missed and possibly other unpredictable behavior. cp-nav probably shouldn't be using the display label to look things up, which is supposed to be presentational. The url is more stable.

Steps to reproduce

  1. Install sprout-lists
  2. Change display name of sprout-lists under its settings
  3. Confirm subnav items exist under Sprout Lists in the control panel
  4. Install cp-nav
  5. Subnav items are now missing under Sprout Lists

It does not happen when Sprout Lists is installed second because cp-nav picks up its default url and label.

Additional info

A fix might be to try to use the url first, not the label, to generate the handle and then fall back to the label instead of the other way around. Possible refactoring of CpNavService::regenerateNav() could be

foreach ($currentNav as $value) {
                if (isset($value['url'])) {
                  $handle = str_replace(UrlHelper::url() . '/', '', $value['url']);
                } else {
                  $handle = StringHelper::toKebabCase($value['label']);
                }
                if (!isset($generatedNav[$handle]) && !CpNav::$plugin->navigationService->getByHandle($layoutId, $handle)) {

                    if (isset($value['icon'])) {
                        $icon = $value['icon'];
                    } else {
                        $icon = null;
                    }

                    $model = $this->_prepareNavModel([
                        'layoutId' => $layoutId,
                        'handle'   => $handle,
                        'label'    => $value['label'],
                        'order'    => $order,
                        'icon'     => $icon,
                        'url'      => $handle,
                    ]);
engram-design commented 6 years ago

Totally agree, and your code is exactly what we need. Thanks, fixed in 2.0.5.