Deprecate wagtail.contrib.modeladmin.menus.SubMenu in favour of wagtail.admin.menu.Menu

The Menu class was not originally designed to accept menu items at constructor time (instead requiring them to be passed via hooks); ModelAdmin's SubMenu class patched this functionality in, and the documentation for extending admin views piggybacked on this. Add this functionality to the base Menu class so that we don't have this unnecessary dependency on ModelAdmin.

CHANGELOG.txt

@@ -47,6 +47,7 @@ Changelog
  * Updated `django-filter` version to support 23 (Yuekui)
  * Use `.iterator()` in a few more places in the admin, to make it more stable on sites with many pages (Andy Babic)
  * Migrate some simple React component files to TypeScript (LB (Ben) Johnston)
+ * Deprecate the usage and documentation of the `wagtail.contrib.modeladmin.menus.SubMenu` class, provide a warning if used directing developers to use `wagtail.admin.menu.Menu` instead (Matt Westcott)
  * Fix: Typo in `ResumeWorkflowActionFormatter` message (Stefan Hammer)
  * Fix: Throw a meaningful error when saving an image to an unrecognised image format (Christian Franke)
  * Fix: Remove extra padding for headers with breadcrumbs on mobile viewport (Steven Steinwand)

docs/extending/admin_views.md

@@ -187,15 +187,14 @@ The calendar will now be visible at the URL `/admin/calendar/month/`.
 
 ![A single calender month](../_static/images/adminviews_calendarmonth.png)
 
-Finally we can alter our `wagtail_hooks.py` to include a group of custom menu items. This is similar to adding a single item but involves importing two more classes, `SubMenu` and `SubmenuMenuItem`.
+Finally we can alter our `wagtail_hooks.py` to include a group of custom menu items. This is similar to adding a single item but involves importing two more classes, `Menu` and `SubmenuMenuItem`.
 
 ```{code-block} python
-:emphasize-lines: 3-4, 21-26
+:emphasize-lines: 3,20-25
 
 from django.urls import path, reverse
 
-from wagtail.admin.menu import MenuItem, SubmenuMenuItem
-from wagtail.contrib.modeladmin.menus import SubMenu
+from wagtail.admin.menu import Menu, MenuItem, SubmenuMenuItem
 from wagtail import hooks
 
 
@@ -212,12 +211,12 @@ def register_calendar_url():
 
 @hooks.register('register_admin_menu_item')
 def register_calendar_menu_item():
-    menu_items = [
+    submenu = Menu(items=[
         MenuItem('Calendar', reverse('calendar'), icon_name='date'),
         MenuItem('Current month', reverse('calendar-month'), icon_name='date'),
-    ]
+    ])
 
-    return SubmenuMenuItem('Calendar', SubMenu(menu_items), classnames='icon icon-date')
+    return SubmenuMenuItem('Calendar', submenu, classnames='icon icon-date')
 ```
 
 The 'Calendar' item will now appear as a group of menu items.

docs/releases/4.0.md

@@ -54,6 +54,7 @@ When using a queryset to render a list of images, you can now use the `prefetch_
  * Updated `django-filter` version to support 23 (Yuekui)
  * Use `.iterator()` in a few more places in the admin, to make it more stable on sites with many pages (Andy Babic)
  * Migrate some simple React component files to TypeScript (LB (Ben) Johnston)
+ * Deprecate the usage and documentation of the `wagtail.contrib.modeladmin.menus.SubMenu` class, provide a warning if used directing developers to use `wagtail.admin.menu.Menu` instead (Matt Westcott)
 
 ### Bug fixes
 
@@ -136,3 +137,7 @@ The `get_snippet_edit_handler` function in `wagtail.snippets.views.snippets` has
 The `explorer_breadcrumb` template tag is not documented, however if used it will need to be renamed to `breadcrumbs` and the `url_name` is now a required arg.
 
 The `move_breadcrumb` template tag is no longer used and has been removed.
+
+### `wagtail.contrib.modeladmin.menus.SubMenu` is deprecated
+
+The `wagtail.contrib.modeladmin.menus.SubMenu` class should no longer be used for constructing submenus of the admin sidebar menu. Instead, import `wagtail.admin.menu.Menu` and pass the list of menu items as the `items` keyword argument.

wagtail/admin/menu.py

@@ -1,4 +1,6 @@
+from django.core.exceptions import ImproperlyConfigured
 from django.forms import Media, MediaDefiningClass
+from django.utils.functional import cached_property
 
 from wagtail import hooks
 from wagtail.admin.ui.sidebar import LinkMenuItem as LinkMenuItemComponent
@@ -40,22 +42,34 @@ class MenuItem(metaclass=MediaDefiningClass):
 
 
 class Menu:
-    def __init__(self, register_hook_name, construct_hook_name=None):
+    def __init__(self, register_hook_name=None, construct_hook_name=None, items=None):
+        if register_hook_name is not None and not isinstance(register_hook_name, str):
+            raise ImproperlyConfigured(
+                "Expected a string or None as register_hook_name, got %r. "
+                "Did you mean to pass an `items` keyword argument instead?"
+                % register_hook_name
+            )
+
         self.register_hook_name = register_hook_name
         self.construct_hook_name = construct_hook_name
-        # _registered_menu_items will be populated on first access to the
-        # registered_menu_items property. We can't populate it in __init__ because
-        # we can't rely on all hooks modules to have been imported at the point that
-        # we create the admin_menu and settings_menu instances
-        self._registered_menu_items = None
+        self.initial_menu_items = items
 
-    @property
+    @cached_property
     def registered_menu_items(self):
-        if self._registered_menu_items is None:
-            self._registered_menu_items = [
-                fn() for fn in hooks.get_hooks(self.register_hook_name)
-            ]
-        return self._registered_menu_items
+        # Construct the list of menu items from the set passed to the constructor along with any
+        # registered through hooks. We can't do this in __init__ because we can't rely on all hooks
+        # modules to have been imported at the point that we create the admin_menu and
+        # settings_menu instances
+        if self.initial_menu_items:
+            items = self.initial_menu_items.copy()
+        else:
+            items = []
+
+        if self.register_hook_name:
+            for fn in hooks.get_hooks(self.register_hook_name):
+                items.append(fn())
+
+        return items
 
     def menu_items_for_request(self, request):
         items = [item for item in self.registered_menu_items if item.is_shown(request)]

wagtail/contrib/modeladmin/menus.py

@@ -1,4 +1,7 @@
+from warnings import warn
+
 from wagtail.admin.menu import Menu, MenuItem, SubmenuMenuItem
+from wagtail.utils.deprecation import RemovedInWagtail50Warning
 
 
 class ModelAdminMenuItem(MenuItem):
@@ -52,24 +55,12 @@ class GroupMenuItem(SubmenuMenuItem):
             order=order,
         )
 
-    def is_shown(self, request):
-        """
-        If there aren't any visible items in the submenu, don't bother to show
-        this menu item
-        """
-        for menuitem in self.menu._registered_menu_items:
-            if menuitem.is_shown(request):
-                return True
-        return False
-
-
-class SubMenu(Menu):
-    """
-    A sub-class of wagtail's Menu, used by AppModelAdmin. We just want to
-    override __init__, so that we can specify the items to include on
-    initialisation
-    """
 
-    def __init__(self, menuitem_list):
-        self._registered_menu_items = menuitem_list
-        self.construct_hook_name = None
+def SubMenu(items):
+    warn(
+        "wagtail.contrib.modeladmin.menus.SubMenu is deprecated. Use wagtail.admin.menus.Menu and "
+        "pass the list of menu items as the 'items' keyword argument instead",
+        category=RemovedInWagtail50Warning,
+        stacklevel=2,
+    )
+    return Menu(items=items)

wagtail/contrib/modeladmin/options.py

@@ -12,6 +12,7 @@ from django.utils.safestring import mark_safe
 from wagtail import hooks
 from wagtail.admin.admin_url_finder import register_admin_url_finder
 from wagtail.admin.checks import check_panels_in_model
+from wagtail.admin.menu import Menu
 from wagtail.admin.panels import ObjectList, extract_panel_definitions_from_model_class
 from wagtail.coreutils import accepts_kwarg
 from wagtail.models import Page, TranslatableMixin
@@ -27,7 +28,7 @@ from .helpers import (
     PagePermissionHelper,
     PermissionHelper,
 )
-from .menus import GroupMenuItem, ModelAdminMenuItem, SubMenu
+from .menus import GroupMenuItem, ModelAdminMenuItem
 from .mixins import ThumbnailMixin  # NOQA
 from .views import (
     ChooseParentView,
@@ -591,7 +592,7 @@ class ModelAdmin(WagtailRegisterable):
         """
         Utilised by Wagtail's 'register_menu_item' hook to create a menu item
         to access the listing view, or can be called by ModelAdminGroup
-        to create a SubMenu
+        to create a submenu
         """
         return ModelAdminMenuItem(self, order or self.get_menu_order())
 
@@ -692,7 +693,7 @@ class ModelAdmin(WagtailRegisterable):
 class ModelAdminGroup(WagtailRegisterable):
     """
     Acts as a container for grouping together mutltiple PageModelAdmin and
-    SnippetModelAdmin instances. Creates a menu item with a SubMenu for
+    SnippetModelAdmin instances. Creates a menu item with a submenu for
     accessing the listing pages of those instances
     """
 
@@ -728,11 +729,11 @@ class ModelAdminGroup(WagtailRegisterable):
     def get_menu_item(self):
         """
         Utilised by Wagtail's 'register_menu_item' hook to create a menu
-        for this group with a SubMenu linking to listing pages for any
+        for this group with a submenu linking to listing pages for any
         associated ModelAdmin instances
         """
         if self.modeladmin_instances:
-            submenu = SubMenu(self.get_submenu_items())
+            submenu = Menu(items=self.get_submenu_items())
             return GroupMenuItem(self, self.get_menu_order(), submenu)
 
     def get_submenu_items(self):