Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

refactor(): configurable template paths #105

Closed
pkozlowski-opensource opened this issue Jan 29, 2013 · 25 comments
Closed

refactor(): configurable template paths #105

pkozlowski-opensource opened this issue Jan 29, 2013 · 25 comments

Comments

@pkozlowski-opensource
Copy link
Member

Currently a path to templates is hard-coded to template/alert/alert.html. We can explore ways of making the template prefix configurable.

@zshamrock
Copy link

Would be nice, as I wrote here (#10 (comment)) - my use case, is that I use angular bootstrap with spring java appl, and of course, hard-coded paths don't work.

@pkozlowski-opensource
Copy link
Member Author

@zshamrock see my comments in #10. I think that that there is an easy way out even today by simply pre-loading partials (or getting the distribution file with partials pre-loaded if you don't customize them). But yes, let's see how to make it configurable.

@ProLoser
Copy link
Member

Yeah... I don't really agree with how you guys went about doing this.

For example: https://github.com/angular-ui/bootstrap/blob/master/template/alert/alert.html

The template is what? 4 lines of code? Line 3 is useless and could be placed onto line 1, line 2 could have been done programmatically via JS or just let the user do it himself without the ngClick. However, lets say he wants other links other than the close button, he is going to be doing the close behavior himself anyway.

My point is, I believe a lot of these templates are useless and just aspects of imposing opinionated conventions. Even bootstrap itself still expects you to put the html together yourself, so why is the angular implementation more restrictive?

Most of the properties can be added to existing DOM elements using JS and we would not be imposing extra http requests.

@darwin
Copy link

darwin commented Feb 19, 2013

I don't agree with @ProLoser. I think Angular conventions should be stronger than Bootstrap conventions.

Angular UI Bootstrap should provide nice set of directives. And directives should wrap snippets of HTML code + behaviours into new tags (which are reusable and configurable). Whenever there is a repeating of HTML boilerplate it should be wrapped in an Angular directive. Twitter Bootstrap forces you to repeat HTML constructs, because it is designed to be usable for static pages even without javascript enabled. This is not the case of an Angular app. And I don't agree UI Bootstrap is more restrictive than Twitter Bootstrap. When it allows you to redefine template there is nothing stopping you to provide custom templates for every new directive instance. IMO it is more flexible thanks to javascript.

@ProLoser
Copy link
Member

@darwin my mentality in AngularUI has been that we provide DOM attributes, and YOU provide the templates.

More accurately:

Templates and partials are reserved for app-specific DSL.

AngularUI is a third party lib you use to 'decorate' your own app. Attributes decorate elements. They do not replace them.

Someone can then say that to THEIR app (or personal definition) a tag called <tabs> is actually the equivalent to:

<ul bs-tooltip bs-tabs ui-validate ui-if>

and whatever other crap you want to mix in.

I just believe that the HTML is the sole responsibility of the user, and not us. This is also how Bootstrap (vanilla) works.

@ProLoser
Copy link
Member

This also ties into my discussing a new 'alias' tool that lets you strip prefixes.

@ProLoser
Copy link
Member

Also, one small note @darwin:

You can only override the templates ONCE. And it MUST be using an inline script tag. You can NOT create a unique template for different instances. This is too constric

To me this is equivalent to defining a global app-wide default (ui.config) but then overriding this configuration on demand (ui-options).

@darwin
Copy link

darwin commented Feb 19, 2013

Ok, I'm pretty new to Angular and AngularUI. So take my opinion with grain of salt.

As an AngularUI user I want to have nice directive which works nicely out of box without many options. If I needed more complex stuff, I could dive in and override defaults, mix in more attributes or even redefine template. If I'm not sure how directive renders, I can use web inspector or check out AngularUI source code. But default usage should be short and sweet. Why would users define this over and again in their apps? I guess someone would start a library "AngularUI DRY", which would define this layer on top of AngularUI for them. And there will be probably more than one.

I'm fine with designing AngularUI into two layers. But it kinda already works like that. You may include AngularUI without templates. In the future when templates will be replaceable on per-instance basis, the situation will be even more flexible IMHO.

@ProLoser
Copy link
Member

Honestly I think there is a slight flaw in this aspect of Angular's design in general. It's great that you can optionally specify templates, but I've never liked the idea of a third-party lib packaging partials unless you knew for a FACT that they'd absolutely NEVER be customized.

My opinion has been that people should be creating their own app-specific directives and that this is the only time it should leverage partials.

@petebacondarwin
Copy link
Member

This is a great discussion because considering how a library is used is fundamental to good API design. I would like to see more examples from @ProLoser showing where he feels the current API is too restrictive.

In the case of alert I don't actually agree that the template is pointless. The application developer just needs to put the following in their application:

<alert ng-repeat="alert in getAlerts()" type="alert.type" on-close="removeAlert(alert)">
  {{alert.message}}
</alert>

This is clear and usable by even the least capable JavaScript developer. As soon as you start trying to remove this layer of DSL then you are adding unnecessary burden onto the application developer and you get into two tricky situations. First you put more pressure on the library developer to make the component ultra generic which means more potential for bugs and more wasted code dealing with edge cases that are rarely used. Second, you risk making the component so generic it is too difficult to grok for the average developer so they don't bother using it.

I would argue that it is more likely that an application will developer will want to tweak the layout of the items in a template that to add or remove functionality. By putting as much in the template as possible we are actually making it more modifiable from this point of view. They are free to rearrange the HTML to fit the CSS framework of their choice.

Finally regarding this overriding of templates. I think we should not get so caught up about "packaging" libraries. Angular is already highly modular by its very nature and it is quite straightforward to pick and choose bits of a "library" for use in an application. We should be moving more to a fine grained distribution model to support this.

@ProLoser
Copy link
Member

I don't think the extra HTTP request is necessary for 1 HTML tag.

I don't think having 1 HTML tag just to specify transclusion is also useful, which you could just do replace:false essentially.

Do all the properties transfer to the outter DOM element in the template? Such as color changes, etc (I seriously don't remember).

We are already running into conflicts with imposing our idea of a good 'dsl' that we're being forced to backpedal and add prefixes, etc.

Just breaking the package into pieces does not void the fact that you simply cannot easily replace the template.

NOW, I DO concede that if the template is lengthy and convoluted, it's worth putting into a partial. But that's like putting the calendar of the datepicker into a template instead of into js. That is in no way applicable to alert, tabs, accordion, where the user STILL has to create each DOM node.

REALLY though, 4 line html file? SERIOUSLY?

@darwin
Copy link

darwin commented Feb 19, 2013

What do you mean by that extra HTTP request? Templates are pre-populated in template cache. There is no HTTP request needed. Am I missing something?

@ProLoser
Copy link
Member

They are cached after they are retrieved, yes. But that initial request is still a request. Now lets consider every single directive in the lib has it's own partial (I am probably wrong), but that's 12+ (or something) partials containing only a few lines of HTML? Add that onto the developer's own partials. You could be hitting tens of partials. Lets say AngularUI started doing partials too. etc. etc.

If you check on the Yahoo recommendations, that's way too many requests lol.

Dean Sofer
DeanSofer.com
714.900.2254

On Tuesday, February 19, 2013 at 12:14 AM, Antonin Hildebrand wrote:

What do you mean by that extra HTTP request? Templates are pre-populated in template cache. There is no HTTP request needed. Am I missing something?


Reply to this email directly or view it on GitHub (#105 (comment)).

@pkozlowski-opensource
Copy link
Member Author

@ProLoser I will write more later on as I think there are number of misunderstandings here. But for now let's make it clear that partials (templates) are cached up-front if you chose so. In other words, we've got a distribution file that has all the templates bundled together.

Check this info about build files: https://github.com/angular-ui/bootstrap/tree/gh-pages#build-files
and the content of a file bundling templates: https://github.com/angular-ui/bootstrap/blob/gh-pages/ui-bootstrap-tpls-0.1.0.js#L1336

In short: we don't force any superfluous HTTP requests - people can just include one file and they are done, nothing will be downloaded on the fly!

@ProLoser
Copy link
Member

Interesting.

@ProLoser
Copy link
Member

I was unaware you guys were doing that.

Then I suppose my only legitimate argument is that I think it's superfluous in most situations.

@petebacondarwin
Copy link
Member

I think a major problem with angular is that it makes writing widgets so
easy (4 lines!) that it looks pointless

Pete
...from my mobile.
On Feb 19, 2013 8:38 AM, "Dean Sofer" [email protected] wrote:

I was unaware you guys were doing that.

Then I suppose my only legitimate argument is that I think it's
superfluous in most situations.


Reply to this email directly or view it on GitHubhttps://github.com//issues/105#issuecomment-13762061.

@ggoodman
Copy link

ggoodman commented Mar 1, 2013

I would like to see some progress on this issue. As it stands it seems that the -tpl distribution is unusable on Plunker as plunks are all served from a subdirectory (whether previews or saved plunks).

Perhaps a good compromise right now would be to make these relative to the base url in order to be consistent with the way $locationProvider works.

@pkozlowski-opensource
Copy link
Member Author

@ggoodman this issue is relevant only for development purposes and only for people actually changing templates for widgets (!). Which means that this is limited to a specific use case and should be visible for most of the users.

The -tpls distribution contains all the templates bundled (see https://github.com/angular-ui/bootstrap/blob/gh-pages/ui-bootstrap-tpls-0.1.0-SNAPSHOT.js#L1322) and no requests will be made to download templates for widgets.

Ping me on the gchat if you need more info, I really want to make sure it is clear.

@ggoodman
Copy link

ggoodman commented Mar 1, 2013

Please ignore my previous comment. It stemmed from a misunderstanding of mine.

The issue was that I was requiring ui.bootstrap.tabs only and need to require ui.bootstrap instead.

@sudhakar
Copy link

sudhakar commented Mar 2, 2013

With angular/angular.js#1849 closed, looking forward for having configurable directive templates soon..

My usecase for configurable templates is little different. I want to use tabs directive in two different places with different templates for each of them.

If I need to open a new issue, please let me know.

@petebacondarwin
Copy link
Member

@sudhakar - trouble is this would have to wait for 1.2 to come out and also would mean breaking compatibility with 1.0.x

@petebacondarwin
Copy link
Member

@sudhakar - in the meantime, if you only have two different sets of templates, you could always create a second directive that is basically a copy of the first but uses the other set of templates?

@sudhakar
Copy link

sudhakar commented Mar 2, 2013

Thanks @petebacondarwin. Anyways we are in early development stage in our project so I can wait until 1.2 is released :)

@pkozlowski-opensource
Copy link
Member Author

OK, guys, I've spent a lot of time pondering different options for this issue and I don't think it makes much sense to make the path configurable. The only use-case for this I'm aware of is development time where someone is customizing default templates.

Let me repeat it again: configurable path is only needed for people convenience while developing custom version of templates for directives from this repo.

In production there is no issue really as templates should be pre-populated into the $templateCache anyway. And for development there is very easy solution by using the <script> tag, as described here: http://stackoverflow.com/a/17677437/1418796

All in all I don't think we want to introduce additional complexity for this use-case as:

  • it is pretty rare
  • there is an easy work-arround

Closing for now, if anyone has another use case or please leave a comment here so we can discuss it further.

jdewit added a commit to jdewit/bootstrap that referenced this issue Jan 30, 2014
I need the ability to have different markup for some tabs on a page
by page basis. Injecting another template with the script tag is
getting ugly.

@sudhakar mentions a similar need for this in angular-ui#105.
jdewit added a commit to jdewit/bootstrap that referenced this issue Jan 30, 2014
I need the ability to have different markup for some tabs on a page
by page basis. Injecting templates with the script tag on each page
is getting ugly.

This topic was recently brought up again in angular-ui#1611.

@sudhakar mentions a similar need for this in angular-ui#105.

_Usage_

```html
    <tabset>
      <tab ng-repeat="tab in tabs">
        <tabset template-url="custom_tab_template">
          <tab ng-repeat="innerTab in tab.tabs">
            <span class="inner-tab-content">{{ innerTab.content }}</span>
          </tab>
        </tabset>
      </tab>
    </tabset>
```
jdewit added a commit to jdewit/bootstrap that referenced this issue Jan 30, 2014
I need the ability to have different markup for some tabs on a page
by page basis. Injecting templates with the script tag on each page
is getting ugly.

This topic was recently brought up again in angular-ui#1611.

@sudhakar mentions a similar need for this in angular-ui#105.

Usage

<tabset>
  <tab ng-repeat="tab in tabs">
    <tabset template-url="custom_tab_template">
      <tab ng-repeat="innerTab in tab.tabs">
        <span class="inner-tab-content">{{ innerTab.content }}</span>
      </tab>
    </tabset>
  </tab>
</tabset>
jdewit added a commit to jdewit/bootstrap that referenced this issue Jan 30, 2014
I need the ability to have different markup for some tabs on a page
by page basis. Injecting templates with the script tag on each page
is getting ugly.

This topic was recently brought up again in angular-ui#1611.

@sudhakar mentions a similar need for this in angular-ui#105.

Usage

    <tabset>
      <tab ng-repeat="tab in tabs">
        <tabset template-url="custom_tab_template">
          <tab ng-repeat="innerTab in tab.tabs">
            <span class="inner-tab-content">{{ innerTab.content }}</span>
          </tab>
        </tabset>
      </tab>
    </tabset>
jdewit added a commit to jdewit/bootstrap that referenced this issue Jan 30, 2014
I need the ability to have different markup for some tabs on a page
by page basis. Injecting templates with the script tag on each page
is getting ugly.

This topic was recently brought up again in angular-ui#1611.

@sudhakar mentions a similar need for this in angular-ui#105.

Usage

    <tabset template-url="custom_tab_template">
      <tab heading="Stuff">
        Some stuff here
      </tab>
    </tabset>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants