Inline Checkbox/Radio Fixes & Improvements

[jesseleite]

Can't figure out how to make tests pass, sorry.

[coveralls]

Coverage remained the same at 89.02% when pulling 9d8be51 on JerseyMilker:inline-radio-checkbox-chaining into c4190c4 on adamwathan:master.

[jesseleite]

Just had a thought... From an API standpoint, a radio is a radio, whether stacked or inline. A chainable inline() method could act more like a modifier, rather than specifying inline via your main inlineRadio() method.

{!! BootForm::radio('Hidden', 'visibility', 'hidden')->inline()->check() !!}
{!! BootForm::checkbox('Hidden', 'visibility')->value('hidden')->inline()->check() !!}

Not sure how this would affect the original rendering issue. It might make things easier on that front, but idk.

[coveralls]

Coverage increased (+1.02%) to 90.04% when pulling f905bb8 on JerseyMilker:inline-radio-checkbox-chaining into c4190c4 on adamwathan:master.

[jesseleite]

I just pushed a few commits, which make the inlineRadio() and inlineCheckbox() tests pass. This fixes my original issue with chainable methods appending to label instead of input control.

I also added a few new failing tests for that inline() as a chainable modifier idea I mentioned above.

[coveralls]

Coverage increased (+1.02%) to 90.04% when pulling ab220b7 on JerseyMilker:inline-radio-checkbox-chaining into c4190c4 on adamwathan:master.

[jesseleite]

Just noting that these tests will fail, specifically because these lines in the new inline() chainable modifier depends on this PR on your base form package.

Lastly, the old inlineCheckbox() and inlineRadio() methods are now just pass-throughs, and tell the new inline() modifier to do the heavy lifting (see this commit 0726a87). I think inline as chainable modifier makes more sense API wise, but these passthroughs keep the package backwards compatible. All of the following should now be valid:

{!! BootForm::radio('Hidden', 'visibility')->inline() !!}
{!! BootForm::checkbox('Hidden', 'visibility')->inline() !!}

{!! BootForm::inlineRadio('Hidden', 'visibility') !!}
{!! BootForm::inlineCheckbox('Hidden', 'visibility') !!}

My only concern right now is that I removed this behaviour from inlineCheckbox():

if ($checked) {
    $control->check();
} else {
    $control->uncheck();
}

I assume this is already handled by non-inline checkbox(), but I should probably double check for checkbox()->inline().

[coveralls]

Coverage increased (+1.52%) to 90.53% when pulling 0726a87 on JerseyMilker:inline-radio-checkbox-chaining into c4190c4 on adamwathan:master.

[coveralls]

Coverage increased (+1.52%) to 90.53% when pulling 0664744 on JerseyMilker:inline-radio-checkbox-chaining into c4190c4 on adamwathan:master.

[adamwathan]

Can you update your code to use getAttribute('type') instead of the type method? I had to remove that method from the base form package because of a collision with the InputGroup element in this package. Those shorthand methods are traditionally setters in these packages anyways so it made more sense to me to add a different method for getting attribute values.

[jesseleite]

For sure, I'll take a look at this when I have chance.

[jesseleite]

Done.

[jesseleite]

Hugs for everyone.

[jesseleite]

Hey @adamwathan, I just realized commit a39cc97 requires adamwathan/form 0.7.1 or greater. You have 0.7.* in composer.json.

[adamwathan]

Sounds like an opportunity for a pull request.