Buttons about ng-bootstrap HOT 13 CLOSED

Just to set a baseline, here are check-box buttons in vanilla ng2: http://plnkr.co/edit/pey0BwIo8dkKbViA02R0?p=preview

Whatever we do it should be simpler / shorter to write as compared to this baseline.

from ng-bootstrap.

Moving discussion over from #42 there were 2 proposal in plunkers:

• http://plnkr.co/edit/ZcNiSJR6rFfMAvXX9Q9X?p=preview from @wesleycho
• http://plnkr.co/edit/5V4ZP8qThnM0xFyIIXFt?p=preview from @Foxandxss

IMO both are going in a good directions, we just need to push this idea further. My biggest open question for now is the relation of those directives to the ngModel / ngControl - that is - do we want to plug into those to provide things like validation etc.

from ng-bootstrap.

So I would like to go back to the topic of buttons and after experimenting a bit I'm not sure we want to have a directive for check-boxes. I'm saying this since the vanilla version looks like so:

<div class="btn-group" data-toggle="buttons">
    <label class="btn btn-primary" [class.active]="foo">
         <input type="checkbox" [(ngModel)]="foo"> Checkbox 2
     </label>
</div>

while with a directive it would be:

<div class="btn-group" data-toggle="buttons">
    <label class="btn btn-primary">
         <input type="checkbox" [(ngModel)]="foo" ngbButtonCheckbox> Checkbox 2
     </label>
</div>

so I'm not sure the directive pays for itself here.... Thoughts?

from ng-bootstrap.

I'm sort of in agreement - I feel like it's too simple.

from ng-bootstrap.

agreed. we should put something on our site though - or someone will say something. maybe even just the template above.

from ng-bootstrap.

Yes, agree with @icfantv - if we decide to skip a directive for a given piece of functionality because we think it is "not worth it" we should still show this on a demo page to say: "hey, look, this is actually easy with vanilla ng2, no need for a custom directives here"

from ng-bootstrap.

And get endless issues because they don't manage to make the checkboxs to work.

from ng-bootstrap.

I've been experimenting with radio buttons and for now I came up with this:

<div class="btn-group" data-toggle="buttons" [(ngModel)]="model" ngb-radio-group>
    <label class="btn btn-primary">
        <input type="radio" name="rad2" value="one">One
    </label>
    <label class="btn btn-primary">
        <input type="radio" name="rad2" value="two">Two
    </label>
</div>

Here is the plunker:
http://plnkr.co/edit/hHNhl8lBgZrMdOTY8jDj?p=preview

There are three directives for the group, label and input
Let me know what you think

from ng-bootstrap.

I like the direction! The only other thing to explore would be how to use non-string values. I'm not sure if <input type="radio" name="rad2" [value]="2"> would do the trick as browser will, most probably, convert it to string anyway. But then we could have support for ngValue.

from ng-bootstrap.

I like this too - that way it's not opinionated on the template, and just implements the high level logic

from ng-bootstrap.

@maxokorokov since the initial direction looks good, could you please turn your code into a PR? This way it will be much easier to review / iterate on it. I started to review the code but is kind of hard in plunker. Here are few remarks for things I've bump into:

• make selectors less specific so directives are more flexible: div.btn-group[ngb-radio-group][ngModel] => [ngb-radio-group][ngModel]
• Since you are marking some injectables as @Optional() you need to account for this with defensive checks
• In your plunker try to delete one character (effectively making non of the radios selected). This should mark a given control as invalid, but it does not

There are more comments coming but it will be easier to express / tackle those when we've got a PR. And hey, thnx for all the work done so far on this!

from ng-bootstrap.

@pkozlowski-opensource opened pull request #216

The character issue you mention is due to the 'required' validator that is invalid only when the value is empty/not set, so for me it works as expected. The rest should be addressed.

from ng-bootstrap.

Radio support just landed via 44adfd7 and we've decided that checkboxes are not needed so this one is fixed :-)

from ng-bootstrap.