Skip to content
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

Changed behaviour of abstracted menus in 4.0 not documented #497

Open
dkirkham opened this issue Aug 16, 2024 · 0 comments
Open

Changed behaviour of abstracted menus in 4.0 not documented #497

dkirkham opened this issue Aug 16, 2024 · 0 comments

Comments

@dkirkham
Copy link
Contributor

In #472, I ended up removing the wagtailmenus.views.MenuTabbedInterfaceMixin class, which included code like this:

class MenuTabbedInterfaceMixin:
def get_edit_handler(self):
if hasattr(self.model, 'edit_handler'):
edit_handler = self.model.edit_handler
elif hasattr(self.model, 'panels'):
edit_handler = ObjectList(self.model.panels)
else:
edit_handler = TabbedInterface([
ObjectList(self.model.content_panels, heading=_("Content")),
ObjectList(self.model.settings_panels, heading=_("Settings"),
classname="settings"),
])
return edit_handler.bind_to_model(self.model)

In the replacement SnippetViewSets, eg. MainMenuAdmin, I directly coded the edit_handler as follows:

edit_handler = TabbedInterface([
ObjectList(panels.main_menu_content_panels, heading=_("Content")),
ObjectList(panels.menu_settings_panels, heading=_("Settings"),
classname="settings"),
])

If a developer has subclassed AbstractMainMenu or AbstractFlatMenu, and modified properties panels, content_panels or settings_panel, as described in the documentation, then these changes won't flow through to the admin pages.

A developer can achieve the same outcome by modifying instead the edit_handler in the subclassed admin views, and to me this is better aligned to SnippetViewSet behaviour and clearer than relying on some different magic within the admin view class.

This wasn't an intentional change on my part, it was missed in the ModelAdmin to Snippet changeover, and the tests didn't pick up change in behaviour.

A side effect of this change, as I see it, is that the properties content_panels and settings_panel are no longer required in the AbstractMainMenu and AbstractFlatMenu classes, which means that we can solve #464.

It is probably worth some discussion to determine if this changed behaviour in 4.0 should be documented, or the change reverted. I'd especially like to hear from any developers who have subclassed AbstractMainMenu or AbstractFlatMenu.

Depending on the agreed course of action, I'm happy to prepare PRs accordingly. Of course, I'm also happy if others want to have a go at the PRs.

It would be good to also fix #488 so that any new documentation is more readily visible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant