Radio buttons don't check if bound to subdocument #668

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@giacmir

giacmir commented Oct 24, 2012

As in the object I noticed that if a radio button set is bound to a subdocument using a dot notation (e.g. person.name) it doesn't checks.

This happens because the $name variable passed is changed before the value check and so it always fails.

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Oct 25, 2012

Member

Thanks for the fix! We accept pull requests against the dev branch, and will merge it once it has an accompanying test case that proves the issue. You can refer to our contributor guide for more information: http://lithify.me/docs/manual/appendices/contributing.wiki

Member

nateabele commented Oct 25, 2012

Thanks for the fix! We accept pull requests against the dev branch, and will merge it once it has an accompanying test case that proves the issue. You can refer to our contributor guide for more information: http://lithify.me/docs/manual/appendices/contributing.wiki

@ericcholis

This comment has been minimized.

Show comment Hide comment
@ericcholis

ericcholis Feb 6, 2013

Contributor

Since this has been floating for a little while, I'd be happy to re-submit against the latest dev and write tests.

Any thoughts on where to place the tests? I figured I it would be best to just extend:

lithium\tests\cases\template\helper\FormTest->testRadioGeneration();
lithium\tests\cases\template\helper\FormTest->testCustomRadio();
lithium\tests\cases\template\helper\FormTest->testCustomValueRadio();

to account for dot-notation names. Thought I would check first.

Contributor

ericcholis commented Feb 6, 2013

Since this has been floating for a little while, I'd be happy to re-submit against the latest dev and write tests.

Any thoughts on where to place the tests? I figured I it would be best to just extend:

lithium\tests\cases\template\helper\FormTest->testRadioGeneration();
lithium\tests\cases\template\helper\FormTest->testCustomRadio();
lithium\tests\cases\template\helper\FormTest->testCustomValueRadio();

to account for dot-notation names. Thought I would check first.

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Feb 6, 2013

Member

@ericcholis That would be most appreciated, thanks. Extending those existing tests sounds fine.

Member

nateabele commented Feb 6, 2013

@ericcholis That would be most appreciated, thanks. Extending those existing tests sounds fine.

@ericcholis

This comment has been minimized.

Show comment Hide comment
@ericcholis

ericcholis Feb 6, 2013

Contributor

All set, opened a new issue #815

Contributor

ericcholis commented Feb 6, 2013

All set, opened a new issue #815

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Feb 6, 2013

Member

Closing in favor of #815. Thanks @ericcholis.

Member

nateabele commented Feb 6, 2013

Closing in favor of #815. Thanks @ericcholis.

@nateabele nateabele closed this Feb 6, 2013

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