Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Integrating with GridState_Data fixes #51 #53

Closed
wants to merge 1 commit into
from

Conversation

2 participants
Contributor

dhensby commented Jan 31, 2014

Basically, if you request a property of the state that isn't defined, it'll just make it's default value a new GridState_Data which evaluates to truey.

We need to access the property as if it's a function passing a default value of false every time.

Integrating with GridState_Data fixes #51
Basically, if you request a property of the state that isn't defined, it'll just make it's default value a new `GridState_Data` which evaluates to truey.

We need to access the property as if it's a function passing a default value of false every time.
Owner

UndefinedOffset commented Jan 31, 2014

Looks like this is causing a crash, may want to have a look at https://travis-ci.org/UndefinedOffset/SortableGridField/jobs/17979754. Appreciate the fix though ;)

Contributor

dhensby commented Jan 31, 2014

That's because travis isn't using the latest version of SS. GridState_Data::sortableToggle() doesn't exist, so this is expected.

See https://github.com/silverstripe/silverstripe-framework/blob/master/forms/gridfield/GridState.php?source=c#L127 - there's a lot more stuff going on there than in 3.1 including __call() method and another accessor.

Basically, silverstripe/silverstripe-framework#2522 prevents anyone setting falsey values on a GridState_Data object - I'm going to submit a failing test and try and get some discussion around that.

Owner

UndefinedOffset commented Jan 31, 2014

Hmm I wonder if the trick to work around this is to simply store something else in the state like on/off,, pain in the a** but at least it maintains support for all versions.

Contributor

dhensby commented Jan 31, 2014

Sure, something like that would be good. Storing a nested array value like:

$state->sortableToggle = array('value' => false);

But there's a chance that wouldn't work either.... 'on' / 'off' is probably safest, but making that horrible kind of a change because non-release core is broken doesn't seem right to me

Owner

UndefinedOffset commented Jan 31, 2014

Ya I agree, for now I'll leave the compatibility for sortablegridfield at 3.0 to 3.1, I'll merge this into the experimental branch later today.

Owner

UndefinedOffset commented Jan 31, 2014

Merged in the experimental branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment