-
Notifications
You must be signed in to change notification settings - Fork 823
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
FIX Add extraEmptyValues to TreedropdownField #11044
FIX Add extraEmptyValues to TreedropdownField #11044
Conversation
3518887
to
0c6600b
Compare
0c6600b
to
3ec745f
Compare
/** | ||
* @return array | ||
*/ | ||
public function getSchemaValidation() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you, please, add unit test for this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Note - the parent card on this is a SPIKE so don't merge this just yet, need to agree with team first that we're happy with the approach before we merge anything
3ec745f
to
95e3827
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall seems okay to me - though as a bugfix that affects CMS 4 and doesn't add any new API, this should target 4.13, shouldn't it?
Closed in favor of #11068 |
Issue silverstripe/silverstripe-admin#1616
Test setup provided here silverstripe/silverstripe-admin#1616 (comment)