Skip to content

Commit

Permalink
MINOR Setting existing GridState from client during request handling …
Browse files Browse the repository at this point in the history 
…to allow altering it there

MINOR Parameter namespacing for GridState to avoid clashes with multiple GridField instances in same form
MINOR GridField->index() to render the field, unify with gridFieldAlterAction() to support state changes
  • Loading branch information
@chillu
chillu committed Feb 14, 2012
1 parent 4437f9d commit 47ac047
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 19 deletions.
52 changes: 34 additions & 18 deletions forms/gridfield/GridField.php
View file
Expand Up @@ -25,6 +25,7 @@ class GridField extends FormField {
* @var array
*/
public static $allowed_actions = array(
'index',
'gridFieldAlterAction'
);

Expand Down Expand Up @@ -98,8 +99,10 @@ public function __construct($name, $title = null, SS_List $dataList = null, Grid


$this->addExtraClass('ss-gridfield');
$this->requireDefaultCSS();

}

function index($request) {
return $this->gridFieldAlterAction(array(), $this->getForm(), $request);
}

/**
Expand Down Expand Up @@ -356,6 +359,7 @@ public function FieldHolder() {
$this->getAttributes(),
array('value' => false, 'type' => false, 'name' => false)
);
$attrs['data-name'] = $this->Name();
$tableAttrs = array(
'id' => isset($this->id) ? $this->id : null,
'class' => "field CompositeField {$this->extraClass()}",
Expand All @@ -370,6 +374,10 @@ public function FieldHolder() {
);
}

public function getAttributes() {
return array_merge(parent::getAttributes(), array('data-url' => $this->Link()));

This comment has been minimized.

Sign in to view

Copy link
@sminnee

sminnee Feb 14, 2012

Member

Let's add a comment here explaining that data-url is required for no-js operation.
}

/**
* Get the columns of this GridField, they are provided by attached GridField_ColumnProvider
*
Expand Down Expand Up @@ -491,33 +499,38 @@ protected function buildColumnDispatch() {
}
}
}

/**
* This is the action that gets executed when a GridField_AlterAction gets clicked.
*
* @param array $data
* @return string
*/
public function gridFieldAlterAction($data, $form, SS_HTTPRequest $request) {

This comment has been minimized.

Sign in to view

Copy link
@sminnee

sminnee Feb 14, 2012

Member

I suspect that this code has broken the no-JS operation of GridField. There's a bad code-smell here: I think that gridFieldAlterAction() and index() need to have their code separated more aggressively, and both rely on a 3rd method that contains the common code. Suffice it to say that the route back into the gridfield will be quite different for no-JS than for Ajax.

This comment has been minimized.

Sign in to view

Copy link
@chillu

chillu Feb 14, 2012

Author Member

Good point. Yeah, I don't like the preg_match() call their either, relies on a very specific GET signature) - I'll see what I can do.
$id = $data['StateID'];
$stateChange = Session::get($id);

This comment has been minimized.

Sign in to view

Copy link
@sminnee

sminnee Feb 14, 2012

Member

The consequence of this change is that the AJAX-based GridField does away with the session-based state entirely. It irks me to have this much disparity between the no-JS and AJAX-based state management.

This comment has been minimized.

Sign in to view

Copy link
@chillu

chillu Feb 14, 2012

Author Member

The problem here is that there's state set through the client only, in my case ParentID from a separate TreeDropdownField.
I think its a bit extreme to make a custom GridFieldAction in PHP every time the client needs to amend some view data, right?
We could make this a separate "update state" URL handling end point though, which should only pass the desired modifications - and merges with the existing session state.

This comment has been minimized.

Sign in to view

Copy link
@sminnee

sminnee Feb 14, 2012

Member

Yeah - I think that an update state URL that's called by the JS setState() method is the cleanest approach here. It could also be an updateState action if we wanted; it amounts to the same thing.
$gridName = $stateChange['grid'];
$grid = $form->Fields()->dataFieldByName($gridName);

$state = $grid->getState(false);
if(isset($data['GridState'])) $state->setValue($data['GridState']);

$actionName = $stateChange['actionName'];
$args = isset($stateChange['args']) ? $stateChange['args'] : array();
$html = $grid->handleAction($actionName, $args, $data);

if($html) {
return $html;
$html = '';
$data = $request->requestVars();
$fieldData = @$data[$this->Name()];

// Update state from client
$state = $this->getState(false);
if(isset($fieldData['GridState'])) $state->setValue($fieldData['GridState']);

// Try to execute alter action
foreach($data as $k => $v) {
if(preg_match('/^action_gridFieldAlterAction\?StateID=(.*)/', $k, $matches)) {
$id = $matches[1];
$stateChange = Session::get($id);
$actionName = $stateChange['actionName'];
$args = isset($stateChange['args']) ? $stateChange['args'] : array();
$html = $this->handleAction($actionName, $args, $data);
// A field can optionally return its own HTML
if($html) return $html;
}
}

switch($request->getHeader('X-Get-Fragment')) {
case 'CurrentField':
return $grid->FieldHolder();
return $this->FieldHolder();
break;

case 'CurrentForm':
Expand Down Expand Up @@ -565,6 +578,9 @@ function handleRequest(SS_HTTPRequest $request, DataModel $model) {

$this->request = $request;
$this->setModel($model);

$fieldData = $this->request->requestVar($this->Name());
if($fieldData && $fieldData['GridState']) $this->getState(false)->setValue($fieldData['GridState']);

This comment has been minimized.

Sign in to view

Copy link
@sminnee

sminnee Feb 14, 2012

Member

If the javascript setState() call made an immediate request to amend the session-based state, this wouldn't be necessary.
foreach($this->components as $component) {
if(!($component instanceof GridField_URLHandler)) {
Expand Down
2 changes: 1 addition & 1 deletion forms/gridfield/GridState.php
View file
Expand Up @@ -39,7 +39,7 @@ public function __construct($grid, $value = null) {

if ($value) $this->setValue($value);

parent::__construct('GridState');
parent::__construct($grid->Name() . '[GridState]');
}

/**
Expand Down

0 comments on commit 47ac047

Please sign in to comment.