Skip to content
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 default checkbox input #23

Merged
merged 2 commits into from Mar 28, 2015
Merged

Fix default checkbox input #23

merged 2 commits into from Mar 28, 2015

Conversation

jadb
Copy link
Member

@jadb jadb commented Mar 28, 2015

@schalla pointed out that the tests for the default checkbox
input were asserting a wrong markup according to GetBootstrap.

Since I am not sure about all the other ways checkboxes can
be displayed, I left the rest (inline ones and ones used in
horinzontal forms untouched). Will revisit if they are reported
broken.


if (!$this->_align) {
$this->templates([
'checkboxContainer' => '<div class="checkbox">{{content}}</div>',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this template juggling is getting ugly and will be difficult to maintain in future. We should have two sets of templates and just select the one required depending on whether user wants horizontal form or not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100% - am already thinking of a way to refactor this in a cleaner way.

@schalla pointed out that the tests for the default checkbox
input were asserting a wrong markup according to [GetBootstrap].

Since I am not sure about all the other ways checkboxes can
be displayed, I left the rest (inline ones and ones used in
horinzontal forms untouched). Will revisit if they are reported
broken.
@jadb jadb force-pushed the fix-default-checkbox-input branch from 55153da to 3c97f05 Compare March 28, 2015 14:44
jadb added a commit that referenced this pull request Mar 28, 2015
@jadb jadb merged commit 4a86345 into master Mar 28, 2015
@jadb jadb deleted the fix-default-checkbox-input branch March 28, 2015 15:59
@jadb
Copy link
Member Author

jadb commented Mar 28, 2015

Merging all recent PRs and then tagging a new patch release for 0.2 before I start work on a 0.3 (dev) to try using widgets for better organizing everything (and hopefully add a couple new features along the way). Might break the API but we're not on 1.0 yet - so any comments welcomed once I create the branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants