Problem/Motivation

User wants to see group menus as part of the site navigation.

Steps to reproduce

Proposed resolution

Allow group menus to have a core menu item as parent. To facilitate this, add a menu_link_reference field item and widget.

Remaining tasks

User interface changes

API changes

The menu name is no longer hardwired but needs to be called with $group_content_menu->getMenuName() and also to render a group menu tree , the storage handler has a loadMenuTree method.

Data model changes

Comments

Charlie ChX Negyesi created an issue. See original summary.

ghost of drupal past’s picture

StatusFileSize
new62.11 KB
ghost of drupal past’s picture

StatusFileSize
new61.95 KB

Status: Needs review » Needs work

The last submitted patch, 3: 3317493_3.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

ghost of drupal past’s picture

Status: Needs work » Needs review
StatusFileSize
new62.29 KB

Status: Needs review » Needs work

The last submitted patch, 5: 3317493_5.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

ghost of drupal past’s picture

Status: Needs work » Needs review
StatusFileSize
new62.3 KB

Status: Needs review » Needs work

The last submitted patch, 7: 3317493_7.patch, failed testing. View results

ghost of drupal past’s picture

Status: Needs work » Needs review
StatusFileSize
new66.07 KB
ghost of drupal past’s picture

StatusFileSize
new62.34 KB
heddn’s picture

  1. +++ b/modules/menu_link_reference/menu_link_reference.info.yml
    @@ -0,0 +1,6 @@
    +package: Custom
    

    Group

  2. +++ b/modules/menu_link_reference/menu_link_reference.info.yml
    @@ -0,0 +1,6 @@
    +core: 8.x
    

    delete

  3. +++ b/modules/menu_link_reference/menu_link_reference.info.yml
    @@ -0,0 +1,6 @@
    +core_version_requirement: ^8 || ^9
    

    ^9 || ^10

    Please!

  4. +++ b/modules/menu_link_reference/src/Plugin/Field/FieldWidget/MenuLinkReference.php
    @@ -0,0 +1,144 @@
    +  const NONE = '_none';
    

    visibility, public or private.

  5. +++ b/modules/menu_link_reference/src/Plugin/Field/FieldWidget/MenuLinkReference.php
    @@ -0,0 +1,144 @@
    +      '#title' => t('Menu name'),
    

    $this-t()

  6. +++ b/modules/menu_link_reference/src/Plugin/Field/FieldWidget/MenuLinkReference.php
    @@ -0,0 +1,144 @@
    +      '#empty_option' => '- Please select -',
    

    $this-t()

  7. +++ b/modules/menu_link_reference/src/Plugin/Field/FieldWidget/MenuLinkReference.php
    @@ -0,0 +1,144 @@
    +      '#title' => t('Menu link'),
    

    $this-t()

  8. +++ b/modules/menu_link_reference/src/Plugin/Field/FieldWidget/MenuLinkReference.php
    @@ -0,0 +1,144 @@
    +    return array_map(fn (EntityInterface $e) => $e->label(), $menus);
    

    return array_map(static fn (EntityInterface $e) => $e->label(), $menus);

  9. +++ b/src/GroupContentMenuStorage.php
    @@ -0,0 +1,103 @@
    +  public function updateLinks(array $tree, array $update, $do): void {
    

    A little more details on $do variable would help. from its name I would assume its a boolean, but then it isn't type hinted. Hmm, actually, the param arguments are all a little out of sync.

  10. +++ b/tests/src/FunctionalJavascript/GroupContentSubmenuTest.php
    @@ -35,11 +34,7 @@ class GroupContentSubmenuTest extends WebDriverTestBase  {
    @@ -47,20 +42,22 @@ class GroupContentSubmenuTest extends WebDriverTestBase  {
    

    Class test name should rename.

  11. +++ b/tests/src/FunctionalJavascript/GroupContentSubmenuTest.php
    @@ -47,20 +42,22 @@ class GroupContentSubmenuTest extends WebDriverTestBase  {
    +    $this->groupContenMmenuType = $this->randomMachineName();
    

    Variable naming a bit off.

ghost of drupal past’s picture

StatusFileSize
new62.64 KB
new5.1 KB

Thanks for the review, this is done. I haven't renamed the test , it is testing the submenu functionality after all.

heddn’s picture

Status: Needs review » Reviewed & tested by the community

  • chx committed 1fe9ade on 8.x-1.x
    Issue #3317493 by Charlie ChX Negyesi: Integrate into core menus
    

ghost of drupal past’s picture

Status: Reviewed & tested by the community » Fixed

Thanks!

heddn’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.