Semantic-Org / Semantic-UI-Angular

Semantic UI Angular Integrations
MIT License
557 stars 117 forks source link

sm-radio-button design questions #5

Closed m0t0r closed 9 years ago

m0t0r commented 9 years ago

I am planning to create a radio button directive (sm-radio-button) which should probably work in conjunction with sm-radio-group directive which manages a set of sm-radio-button directive. This design is borrowed from angular-material. So, in generic case it should look like:

<sm-radio-group ng-model="radio">
    <sm-radio-button value="radio1">Radio 1</sm-radio-button>
    <sm-radio-button value="radio2">Radio 2</sm-radio-button>
    <sm-radio-button value="radio3">Radio 3</sm-radio-button>
</sm-radio-group>

I have a couple of questions:

1) Angular-design has a set of custom services such as '$mdConstant', '$mdUtil'. Should I consider analogues services as well since they look like good ideas.

2) Is design using controller's prototype to set helper methods considered suitable for us as well ?

3) What else should I consider ?

@caitp please take a look once you have a chance.

caitp commented 9 years ago

I don't really like requiring the radio-group directive, because this is very different from what HTML does. That said, there are advantages in an angular context: only instantiating one ngModelController, rather than having N different ones, with only the last-registered one associated with the form, all fighting each other. So I'm generally ok with this, but it needs to be made clear that this is the approach taken.

Putting methods on prototype is fine. You need to be a bit careful with it though, because it means you no longer support unbound execution of functions, which can have a performance cost if you need to use it in a $watch listener or similar. It's fine to do this, just be careful and try to avoid relying on support for unbound execution.

I would be careful with helper services. If the plan is to split this project into N different modules it makes some sense, but otherwise I'd rather avoid polluting the injector.

m0t0r commented 9 years ago

I would be careful with helper services. If the plan is to split this project into N different modules it makes some sense, but otherwise I'd rather avoid polluting the injector.

Yes, I planned to make each directive to be in its module. Those services look nice and make code DRY having everything in one place which makes changes much easier.

caitp commented 9 years ago

realistically, having each directive in its own module is going to make using the project very difficult and cumbersome, for relatively little benefit. Better to let people customize what they want to include in their own build, imo. Otherwise, try to group different kinds of directives into specific categories, like "forms", "mobile", etc, and make modules based on those categories so it's less crazy.

m0t0r commented 9 years ago

well, the idea I have is to have each directive in its module which could reflect what type is this element, according to grouping in Semantic-UI. So that 'sm-button', for instance in 'semantic.ui.elements.button' module where elements is a type. Though, when dist is built it would be just one 'semantic.ui.angular' module. So that in the future we could consider custom builds where user can choose which components to include in ''semantic.ui.angular'. What do you think ?

That is how bootstrap-ui and angular-material are organized.

caitp commented 9 years ago

Go for it

m0t0r commented 9 years ago

hi, @caitp I am trying to figure out when I should implement and use $render over $formatters in ngModelController. I am pretty sure I know what are those but still do not see clearly what I would use in which case. Could you provide some explanation, please ?

caitp commented 9 years ago

$formatters transforms model values into view values, $parsers transforms view values into model values, $validators and $asyncValidators perform validation, and $render applies the view value to the control

moribvndvs commented 9 years ago

If the outcome of this discussion is to set precedent for organizing modules and how the modules are named, I'd like to throw in my $.02.

1) I agree that functional components (either supporting services used by multiple directives, or individual directives themselves) should be in separate modules beneath the main module itself. This is not so much, I don't think, because we want to concern ourselves with letting people pick and choose which modules they need (although it's not a bad idea; the breadth of Semantic UI is large and not all of it is relevant to each project), but to isolate functionality for testing and bring in dependencies as needed. Having written a few modules and some large apps myself with the starting thought of "why segment; I'll just throw it all in one place to save time and effort", I've almost always regretted it as you maintain the project over time.

2) Brevity and simple but effective organization is key. Regarding the names, I would avoid redundancy, unnecessary explicitness, and keep things canonical. Using ui-bootstrap as a baseline, I prefer this pattern: ui.semantic.[functional component name]. No need to add in unnecessary stuff like .angular (it's an angular module; we already know it's angular, and there's no significant way to mix angular with non angular code to the extent we have to worry about collisions) or .elements (not really a standard in the lexicon of Angular modules, and I don't think there's much value in over-categorizing, plus everything in this module is either UI or supporting UI). Examples: ui.semantic.button, ui.semantic.modal, ui.semantic.position (a theoretical shared component that supports calculating positions for tooltips and the likes), so on and so forth.

Darkeye9 commented 9 years ago

@HackedByChinese I think your comment should be put under #8 (About this project). I definitely agree that we need to set some guidelines and then write some example modules to test them. Once those guidelines are established, we can start the coding race ;)

m0t0r commented 9 years ago

Closed by #10