wagtail-nest / wagtail-modeladmin

Add any model in your project to the Wagtail admin. Formerly wagtail.contrib.modeladmin.
Other
21 stars 8 forks source link

Refactor ButtonHelper component of modeladmin to offer drop-down/sub-menu structuring of actions #7

Open ababic opened 8 years ago

ababic commented 8 years ago

The new more/dropdown UI element used in the Explorer to hide less common actions is really smart, and would certainly be a pattern worth reusing. A logical first step would be look at what's there, and what can be abstracted out for re-use, then to refactor the ButtonHelper component to make use of it.

At the same time, it would probably make sense to refine the way buttons in ButtonHelper are defined (it could be more DRY than it is) and rendered in the various modeladmin templates.

ababic commented 8 years ago

After having time to think this through, I have a proposal for what I'd like to achieve here:

Currently, adding a new button for items in the IndexView (or elsewhere) requires the ButtonHelper class to be sub-classed, and methods adding / overriding in that class. One method needs to define the button (it's text, where it links to etc), and another method does some permission analysis as it bulks buttons together.

I would like to make this process easier by...

  1. Allowing additional button methods to be defined on the ModelAdmin class itself - much like ModelAdmin allows you to define methods that can be named in list_display and accessed at the appropriate time. The ButtonHelper can simply look for a 'definition method' on the ModelAdmin class if it doesn't have a method of it's own.
  2. Add attributes to the ModelAdmin class allowing 'button sets' to be defined, using a simple iterable, e.g:
index_view_buttons = ('edit', 'inspect', 'delete', 'copy')

The appropriate iterable can then be passed to the ButtonHelper's get_buttons_for_obj method, which can then find definitions for each of those buttons, and return an appropriate 'structure' for rendering. I use the term 'structure', because I also hope to support more complex iterables for these 'button series' attributes, to utilise the 'drop-down/sub-menu' design pattern, like so:

index_view_buttons = (
    'edit',
    'view',
    'add_child',
    ('more', ('move', 'copy', 'delete', 'unpublish', 'revisions'),
)

Which would reproduce the menu shown in Explorer.

What I have yet to decide on, is how best to bring permissions into the equation... so that the right buttons only show for the right users. I suppose the simplest solution there would be for each 'button description' method to include a permissions_required iterable within the dictionary they return, which could then be passed off to PermissionHelper to figure out whether the button should be included in the final structure.

@gasman Interested to hear your thoughts on this :)

ababic commented 7 years ago

There's another piece of work being discussed in wagtail/wagtail-modeladmin#10 which looks like it will benefit from a few of the features discussed above, but the support for multi-level/dropdown definitions probably isn't as important as the rest of the stuff.

For now, I think we just need to focus on the following, and work on bringing in the dropdowns later:

  1. Add an index_view_buttons attribute that let's you define a flat list of the buttons you want to show in IndexView for each row, e.g.
index_view_buttons = ('edit', 'inspect', 'delete', 'copy')
  1. Add an inspect_view_buttons attribute that let's you define a flat list of the buttons you want to show at the bottom of the InspectView page, e.g.
inspect_view_buttons = ('edit', 'delete', 'copy')
  1. Support the definition of 'button definition methods' on the ModelAdmin class instead of just on the ButtonHelper, so that you don't have to subclass ButtonHelper just to define a new button or override what an existing one does.

  2. (The difficult bit) Figure out how to make ButtonHelper respect the new attributes / button definition method locations while preserving backwards compatibility.

ababic commented 7 years ago

I think I've figured out how to implement these changes in a backwards compatible way:

We leave the existing ButtonHelper classes largely as they are, but make them raise a deprecation warning on init(). That way, anyone subclassing them will be fine for now.

We create a new GenericButtonHelper class that has the new behaviour on it, and update the ModelAdmin class to use that by default. But, anyone using the button_helper_class attribute to use a custom class (based on one of the existing classes) won't have to worry, as that will be respected.

Then it's just a case of making sure the views support both the old and new style classes when they call on them to do button-related things.

ababic commented 7 years ago

Quick update: I've made a huge amount of progress with this (including getting the dropdown buttons to render), but it ended up requiring changes in a few more places than I was expecting, and so I'm thinking it'll now be best to split the changes up over a few different PRs to make things easier to review (with unit tests added along the way as required)

In case I die or something, here's where things are at: https://github.com/wagtail/wagtail/compare/master...rkhleics:new-button-helper