-
Notifications
You must be signed in to change notification settings - Fork 821
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
GridField and GridState_component bug #6886
Comments
Hey @t3hn0 - which version of SilverStripe are you running? Looks like 3.x, just want to confirm. Quick investigation shows that you can create a GridField with a I think the point of this issue is that these two scenarios produce a different config - one with a // Config (Base) contains a grid state component
$gridField = GridField::create('Grid');
// Config (Base) does not contain a grid state component
$gridField = GridField::create('Grid')
->setConfig(GridFieldConfig_Base::create()); My impression of the purpose of the code below (from the GridField constructor) is to add a $this->setConfig($config);
$this->config->addComponent(new GridState_Component());
$this->state = new GridState($this); If this is causing trouble, then perhaps the bottom two lines could be moved into |
Hey @robbieaverill it was version 3.5. It's not only Suggested update should be ok. |
Hey @t3hn0 - cool. Would you be willing to submit a patch for this? |
I agree with @t3hn0. The original behaviour (assign state on constructor) is flawed, and as this issue demonstrates, isn't reliable. I would also agree with moving to setConfig(), and having the constructor call this. Maybe have this code check to see if one such config already exists, and add it only conditionally. |
@robbieaverill ok this is my first try, so please don't kill me if I got it wrong :) |
…e set config with GridField::setConfig Fixes silverstripe#6886
…e set config with GridField::setConfig Fixes silverstripe#6886
Hi!
I'm not sure if it's intentional but if you construct GridField through injector like in example bellow:
then GridfieldState_Component is not set on GridField config. GridState_Component is added to current GridFieldConfig in GridField constructor, but if we redefine GridFieldConfig with GridField::setConfig, GridState_Component is never set. That can cause JavaScript error while using GridFieldSortableRows and probably others.
Shouldn't you merge these lines
and put it all in GridField::setConfig method?
The text was updated successfully, but these errors were encountered: