3.1 CMS actions enhancements

432 views
Skip to first unread message

Sam Minnée

unread,
Nov 12, 2012, 5:30:44 PM11/12/12
to SS Dev, Paul Clarke
I'm not sure where this work is being tracked, but I know that Paul has done a redesign of the page actions panel for the 3.1 CMS:
https://skitch.com/clarkepaul/849t1/actions-panel-and-status

This is a huge improvement and I'm very supportive of it, but I think we need to be careful about how we extend the actions API to support this.

For example, the workflow module modifies the actions available fairly extensively.

Can we please build the enhanced API in such a way that modules don't need to fork in order to support both 3.0 and 3.1?

A few thoughts:

 - The different button locations (disabled/enabled, 'more actions menu' vs main list, etc) could be added some generic meta-data such as classes, that 3.0 can simply ignore.
 - If the meta-data is stored in different fields, it should be straightforward to introspect, and fill out that meta-data only if 3.1 is installed.

----
Sam Minnée
Chief Executive Officer
SilverStripe Limited

Mobile: 021 311 441
Skype: sam.minnee

Sam Minnée

unread,
Nov 12, 2012, 8:01:09 PM11/12/12
to silverst...@googlegroups.com, Paul Clarke
OK, I'm going to answer my own question :-)

The code is already implemented at these URLs (thanks Naomi)

https://github.com/silverstripe-droptables/silverstripe-cms/tree/side-by-side
https://github.com/silverstripe-droptables/sapphire/tree/side-by-side

SiteTree::getCMSActions() used to include a flat list of FormActions.  Now it has two fields:

 * A CompositeField for popular actions - this keeps the buttons visually joined together, it looks something like: [ Save | Publish ]

 * A TabSet MoreOptionsTabSet, with a Tab, MoreOptions.  Each tab creates a new drop-up menu.

Modules that push buttons onto the end of getCMSActions() will show them to the right of the "More options" list.

Users 

---

Mateusz, I have a few thoughts:

 * The use-case for adding more than 1 drop-up menu seems a little contrived, but I suppose it might be useful.  If we are going to keep it in there, I'd suggest that we make it look a little nicer than it does right now, and I think that the field name MoreOptionsTabSet.MoreOptions should be changed.  Perhaps ActionMenus.MoreOptions?

 * It would be nicer if the MoreOptions menu was always on the end of the list of fields.  You could do this by having MoreOptions added outside of getCMSActions() - for example by combining the results of getCMSActions() and getCMSMenuActions() in the form generation.  That, however, would lock down the API.  We'd need to decide if locking down the API at that level was appropriate.

Marcus Nyeholt

unread,
Nov 12, 2012, 11:12:06 PM11/12/12
to silverst...@googlegroups.com, Paul Clarke
What is the preferred pattern for providing implementations for actions returned by getCMSActions? Given a FormAction needs a method defined on the controller owning the form (or the form itself), this implies a coupling of a model class to a controller class (ignoring the obvious coupling in getCMSFields returning interface elements :D)

Would it make sense to allow a FormAction to have a callback attached to it that would allow model objects to specify a method (or closure) that can receive an array of $data? Not necessarily the whole form or request... the controller or form object is still responsible for handling the action, but it will then call the callback when needed. 

Cheers,

Marcus



--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To view this discussion on the web visit https://groups.google.com/d/msg/silverstripe-dev/-/8-e2tcMJPA4J.

To post to this group, send email to silverst...@googlegroups.com.
To unsubscribe from this group, send email to silverstripe-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/silverstripe-dev?hl=en.

drzax

unread,
Nov 13, 2012, 5:22:50 PM11/13/12
to silverst...@googlegroups.com, Paul Clarke
These changes should also flow through to the actions panel on the edit form for DataObjects managed via ModelAdmin. As far as I can tell it's currently not possible to change the actions there at all which is crazy.

Cheers,
Simon

Marcus Nyeholt

unread,
Nov 13, 2012, 5:47:00 PM11/13/12
to silverst...@googlegroups.com, Paul Clarke
You can, but you need to override the ItemEditForm from your ModelAdmin subclass to do so. Don't have code handy right now, but I think it involves something like 

$grid->getComponent('GridFieldDetailForm')->setItemEditFormCallback(function ($form) {
    $form->Actions()->push();
});


--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To view this discussion on the web visit https://groups.google.com/d/msg/silverstripe-dev/-/4KAc1awcgaAJ.

Ingo Schommer

unread,
Nov 14, 2012, 6:50:38 PM11/14/12
to silverst...@googlegroups.com, Paul Clarke, na...@silverstripe.com, Mateusz Uzdowski
Ticket reference: http://open.silverstripe.org/ticket/7368

@Naomi,Mateusz: Please try to keep larger work items like these centralized,
by ensuring you work against a ticket on open.silverstripe.org.
Either by searching (found this one through a simple search for "more options"),
or by creating one, and ensuring any feature branches and commits reference that ticket.

I've discussed the solution with Mateusz over Skype,
and think as elegant as possible with the current form system.
"Tabs" isn't an ideal metaphor, but that's the only API which allows
us to deal with nested fields efficiently.
And it should be backwards compatible to usage of $actions->removeByName(), which is great.
In terms of the workflow module making use of the new flyout,
it could have conditional logic which checks FieldList->hasTabSet() or FieldList->fieldByName('MoreOptions').

Mateusz Uzdowski

unread,
Nov 14, 2012, 7:54:14 PM11/14/12
to silverst...@googlegroups.com, Paul Clarke, na...@silverstripe.com, Mateusz Uzdowski
Hey there!

I'm afraid I missied the discussion here - even the more important to have a central place for it, thanks for the reminder Ingo!


Regarding Sam's remarks: 
- rename the TabSet to ActionMenus - yep, great. Was already renamed to Root, which was a bad idea because it was not unique :|
- why TabSet: it might be a valuable scenario to allow flexibility of adding more drop-up menus in there - this is where having a TabSet starts making sense. A "workflow" drop up is an example where it could potentially be used. Is that something that we want to provide?
- regarding the ordering, I would prefer flexibility over forced ordering - after all you can control the order within the FieldList to certain extent, and people are used to the notion of pushing and unshifting into a list. But over to you guys for popular vote.

@Marcus, you can provide a controller action via LeftAndMainExtension, but that's not very explicit coupling and is ugly - especially the bit where you are trying to get a very simple thing - the record ID:

        $className = $this->owner->stat('tree_class');

        // Plumbing to get the ID of the currently edited record.
        $SQL_id = Convert::raw2sql($data['ID']);
        if(substr($SQL_id,0,3) != 'new') {
            $record = DataObject::get_by_id($className, $SQL_id);
            if($record && !$record->canEdit()) return Security::permissionFailure($this);
            if(!$record || !$record->ID) throw new SS_HTTPResponse_Exception("Bad record ID #$SQL_id", 404);
        } else {
            throw new SS_HTTPResponse_Exception("Cannot cleanup a new record.", 404);
        }

It's almost like there should be a possibility of adding a Page_CMS_Controller in Page.php that would get auto-coupled via $this->record into the Model... I'm all for a more structured approach here because now it's not obvious how to do it.

Thanks for awesome suggestions!

Mateusz

Marcus Nyeholt

unread,
Nov 15, 2012, 2:04:35 AM11/15/12
to silverst...@googlegroups.com, Paul Clarke, na...@silverstripe.com, Mateusz Uzdowski
So I've been playing more with the idea of FormAction having a callback; See a diff at https://gist.github.com/4077148

Usage then becomes

$actions->push(FormAction::create('updatetemplateversion', $label, function ($data, Form $form, SS_HTTPRequest $request) {
        // do stuff to $form->getRecord();
return $form->loadDataFrom($form->getRecord())->forAjaxTemplate();
}));

Thoughts? 

Because having workarounds by creating extensions or subclasses of GridFieldDetailForm just to have a custom button in a model admin detail view is very tiresome :)




--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To view this discussion on the web visit https://groups.google.com/d/msg/silverstripe-dev/-/n31P-YEWnn0J.

Nicolaas Thiemen Francken - Sunny Side Up

unread,
Nov 15, 2012, 4:23:33 AM11/15/12
to silverst...@googlegroups.com, Paul Clarke, na...@silverstripe.com, Mateusz Uzdowski
Great idea

What sort of a variable is the callbackt?  Might be good to make that really clear to make it easier to use it. 




Marcus Nyeholt

unread,
Nov 15, 2012, 5:53:15 AM11/15/12
to silverst...@googlegroups.com, Paul Clarke, na...@silverstripe.com, Mateusz Uzdowski
It's a closure; conceivably it could be any is_callable type. 


On Thu, Nov 15, 2012 at 8:23 PM, Nicolaas Thiemen Francken - Sunny Side Up <ma...@sunnysideup.co.nz> wrote:
Great idea

What sort of a variable is the callbackt?  Might be good to make that really clear to make it easier to use it. 

--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.

Hamish Friedlander

unread,
Nov 21, 2012, 2:58:14 PM11/21/12
to silverst...@googlegroups.com
Hi,

This was from another thread, but I've cross posted here for relevancy.

Callbacks seem like a reasonable solution, but I think it'd be nicest to implement with a callbacks-as-object pattern, since that allows us to maintain more context without heaps of arguments to the callback method. For instance,

class MyForm_HandleFoo extends FormActionHandler {
  function handle($data) {
    // $this->form, $this->controller, etc are all set at this point
  }
}

I guess you could allow both types (either a PHP callable type or a class of FormActionHandler)? We don't really have a philosophical position re: python's "only one way to do anything" versus ruby's "heaps of ways, pick the one that's most fluent".

Hamish Friedlander

Sam Minnée

unread,
Nov 21, 2012, 8:02:50 PM11/21/12
to silverst...@googlegroups.com
I guess you could allow both types (either a PHP callable type or a class of FormActionHandler)? We don't really have a philosophical position re: python's "only one way to do anything" versus ruby's "heaps of ways, pick the one that's most fluent".

Callable is an interface that FormActionHandler should implement.  is_callable() returns true on the object, then.

Marcus Nyeholt

unread,
Nov 21, 2012, 9:34:07 PM11/21/12
to silverst...@googlegroups.com
I think in the other thread you mentioned the issue of setting callbacks from the model layer, but my take on things is that there's form controls being produced by model layer code which is why that's an issue. 

Anyway, supporting the two ways of doing things would be acceptable; I just like the neatness of closures in these circumstances :)


On Thu, Nov 22, 2012 at 12:02 PM, Sam Minnée <s...@silverstripe.com> wrote:
I guess you could allow both types (either a PHP callable type or a class of FormActionHandler)? We don't really have a philosophical position re: python's "only one way to do anything" versus ruby's "heaps of ways, pick the one that's most fluent".

Callable is an interface that FormActionHandler should implement.  is_callable() returns true on the object, then.

Hamish Friedlander

unread,
Nov 21, 2012, 9:45:58 PM11/21/12
to silverst...@googlegroups.com
Hi 

On 22 November 2012 15:34, Marcus Nyeholt <nye...@gmail.com> wrote:
I think in the other thread you mentioned the issue of setting callbacks from the model layer, but my take on things is that there's form controls being produced by model layer code which is why that's an issue. 

I don't mind setting callbacks in the model, I just don't like the handling the callback in the model - that's clearly controller code.

On Thu, Nov 22, 2012 at 12:02 PM, Sam Minnée <s...@silverstripe.com> wrote:
I guess you could allow both types (either a PHP callable type or a class of FormActionHandler)? We don't really have a philosophical position re: python's "only one way to do anything" versus ruby's "heaps of ways, pick the one that's most fluent".

Callable is an interface that FormActionHandler should implement.  is_callable() returns true on the object, then.

No reason not to - we can't treat FormActionHandler exactly the same as other callables though. The reason to introduce it is to have a thing to hang more context on - with just a closure there's a lot of context that gets lost (the form you've come from, the form's controller, etc) unless you pass it all through as arguments.

Hamish Friedlander

Marcus Nyeholt

unread,
Nov 21, 2012, 9:58:26 PM11/21/12
to silverst...@googlegroups.com
I don't mind setting callbacks in the model, I just don't like the handling the callback in the model - that's clearly controller code.


Not necessarily - handling the request and calling the callback is controller code, but it depends on the implementation of the callback - or more to the point, whether the callback is expected/allowed to touch on things like the request or form objects. In which case, then having a FormActionHandler that implements callable is probably the most reasonable middle ground. 

Sam Minnée

unread,
Nov 21, 2012, 10:28:07 PM11/21/12
to silverst...@googlegroups.com

----
Sam Minnée
Chief Executive Officer
SilverStripe Limited

Mobile: 021 311 441
Skype: sam.minnee

So the arguments passed currently are handler($data, $form, $request).  I'd expect that we stick with those, otherwise we're arbitrarily changing the method signature of something that's basically been in there since 2.0.  The controller can be accessed with $form->getController().

It seems fine to provide additional context to the FormActionHandler if such context exists; I'm just not sure what that context is.
Reply all
Reply to author
Forward
0 new messages