-
Notifications
You must be signed in to change notification settings - Fork 6.7k
bs.config or ui.config #10
Comments
Sure, having a config shared by a certain directive instances is a much needed thing, we should add it to the modal for example. Now, what I kind of don't like in the current angular-ui approach is that we've got a global namespace. Couldn't we have config on the directive-type basis? Sth like:
|
Do you mean "constant"? Constants can only be set in config blocks, like angular.module('myModule', ['bs.modal']).value('bs.modal': {fade:true, Also do we need to worry about bs.modal being used for the constant/value Pete On 24 October 2012 21:50, Pawel Kozlowski [email protected] wrote:
|
@petebacondarwin yep, I've meant Regarding the variable - sure it is good as well. Constants got the advantage of being "injectable" in any block (so config and run). But sure, variables would work as well. Now, for the naming - no, those don't have to be the same. We could have:
or whatever other convention we choose to support. |
Oh yes. Of course: the constant itself cannot be swapped out but the On 24 October 2012 21:59, Pawel Kozlowski [email protected] wrote:
|
@petebacondarwin exactly :-) While we can define a constant only once we can swap object properties anywhere. And you can define those constants on your app module. |
There is a growing schism in the architecture between bootstrap and ui that
|
Fully agree that we need to discus! Had a chat with Dean on IRC and I I'm definitively against diverging even further, we need to sync both |
So a constant, then, for each module that needs one that is defined by the library and is updated by the user of the library in a config or run section if they want different defaults? |
And perhaps it should be named [component name].config, e.g. "$dialog.config" or "accordion.config"? |
@petebacondarwin this would be probably the simplest solution. At the same time it should be transparent for users that don't care about changing defaults as in this case there is not need to define dependencies on any special modules. I would say +1 for this. |
Sounds good to me! Thanks! |
If there are no complaints then I will knock up a PR for this tonight. |
@petebacondarwin awesome! Just one remark: maybe those constants should follow the usual naming convention for constants, that is: upper-case and underscores as words separators. So WDYT? |
Actually I wonder if constant is not really the right idea so I am not sure How about accordionConfig? On 21 January 2013 18:02, Pawel Kozlowski [email protected] wrote:
|
Hmm, my idea was that we suggesting that people can override values of this constant and not the constant itself. This way people don't need to define anything, just inject the config constant and change whatever values they feel like changing. But I don't have strong opinion here. I like constant since I can inject them in both config and run blocks as well as in services etc. |
Yes sure the object itself is not going to change but given that you can ... sent from my tablet
|
OK, whatever then, as long as we are talking about AngularJS constants :-) |
So, after the 4ea002b landed we've got a pattern for directives config. We should stick to this approach unless there is strong objection / better idea. If not let's use this ticket to add defaults to other directives. |
Looks great to me! I can try to work on a PR today to accept configuration for modal and tooltip, unless you want to claim these. |
@Skivvies, go for it! |
No name spacing needed?
|
@ProLoser Good point... Just one remark - would avoid a namespace with a dot, since it is a pain in a &@^%@! if you don't minify files. |
You mean our 'ui.config' -> 'uiConfig'? Lol. I am surprised you guys have
|
I know I have occasionally forgot but I don't know of a better convention.
|
I made it responsive earlier, but now it's just messing stuff up. We should probably just remove bootstrap-responsive.js and the mobile meta tags from the header. |
Hi, guys. We also need the way to configure template location for the components. We use angular bootstrap for spring application, and just templateUrl: template/pagination/pagination.html (for example), doesn't work. Right now I've just used the ui.config approach and update the directive directly, but then I need to maintain my own version of angular bootstrap. Also I will redo it using constants for configuration (I like the opportunity to inject it into the config method, because my templatePath config depends on another constant applicationContext). I will send the pull request this week, just to have the code in place to discuss. |
@zshamrock actually the recommended approach here would be to bundle templates and stick them into Having said the above I recognize that for development if might be useful to download partials on-the-fly. I've opened the issue #105 to look into this. Out of curiosity: are you customizing default bootstrap templates? |
Hey guys, I've been holding off on the PR for modal and tooltip config since @ProLoser raised the namespace issue to wait for that to get resolved, but I can go ahead and just match 4ea002b for now if you think you'd accept it? I'd love to be able to move to angular-ui bootstrap and I think that's the only thing stopping me! |
Thanks @pkozlowski-opensource, just submitted #127 for the modal config. Since updating to the latest I'm having issues with tooltip again. Will try to create a plunker and link to it from #78. Once that's resolved I can submit another PR for tooltip config. |
Closing this issue for now as many of the directives got their config options already. We should be adding systematically global config options for new directives and add them to existing directives as requested. |
Hi @pkozlowski-opensource, thought I'd give angular ui bootstrap's tooltip another shot as of the 0.2.0 release, and still haven't figured out how to specify tooltip-animation="true" globally. I'm guessing it's possible though. Seems like a much more common use case to want to specify this globally rather than per element, but http://angular-ui.github.com/bootstrap/#/tooltip only covers the per-element case. |
I am surprised you guys haven't already indoctrinated this practice yet because I think it's one of the best parts of AngularUI.
Any thoughts to adding this so that you can do app-wide configurations?
The text was updated successfully, but these errors were encountered: