Skip to content

sm-radio-button design questions #5

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
m0t0r opened this issue Dec 13, 2014 · 10 comments
Closed

sm-radio-button design questions #5

m0t0r opened this issue Dec 13, 2014 · 10 comments

Comments

@m0t0r
Copy link
Contributor

m0t0r commented Dec 13, 2014

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
Copy link
Member

caitp commented Dec 13, 2014

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
Copy link
Contributor Author

m0t0r commented Dec 13, 2014

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
Copy link
Member

caitp commented Dec 13, 2014

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
Copy link
Contributor Author

m0t0r commented Dec 13, 2014

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
Copy link
Member

caitp commented Dec 13, 2014

Go for it

@m0t0r
Copy link
Contributor Author

m0t0r commented Dec 14, 2014

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
Copy link
Member

caitp commented Dec 14, 2014

$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

@grbsk
Copy link
Contributor

grbsk commented Apr 15, 2015

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
Copy link

@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
Copy link
Contributor Author

m0t0r commented May 14, 2015

Closed by #10

@m0t0r m0t0r closed this as completed May 14, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants