feat(): IE 8 Compatibility #249
Conversation
- Included in the demo-template.html a polyfill and ieshiv.js for IE < 9
… ie8-compatibility
|
@redaemn great stuff, this looks very promising! I think that hand-crafted shiv is fine to get started with. As for the demo - my personal opinion is that installation instructions are already taking up too much space. Maybe we should have a very short step-by-step thing on the demo page and a link to longer, detailed instructions? Is https://code.google.com/p/ie7-js/ hosted on any CDN? Like cloudflare (http://cdnjs.com/) or something? Would be awesome something we can simply link and not be obliged to check-in. This all looks cool and the plan is solid! I will try to help as much as possible, full IE8 support would be totally awesome! |
…directives that are present in the last build
… ie8-compatibility
|
@pkozlowski-opensource taken from the
Anyway, I quickly checked http://cdnjs.com/ site but I didn't find it.
That was exactly my idea...
I think that a short step-by-step thing should be just enough. |
|
@redaemn I will look into this one later today but was wondering if the https://github.com/kriskowal/es5-shim wouldn't do the trick here as a shim. It is hosted on http://cdnjs.com/index.html I would really, really try hard not to maintain IE8 compatibility code in this repo, even if it is copy & paste of come other code. So my personal preference would be not to introduce code like in https://github.com/redaemn/bootstrap/commit/6e9b8f5a189f9aa4416a0ef9e9f8605d3fa4e220 |
|
@pkozlowski-opensource I agree with you; I will remove the commit https://github.com/redaemn/bootstrap/commit/6e9b8f5a189f9aa4416a0ef9e9f8605d3fa4e220 and look into the What do you think about the TODO list I made? In particular what about the 3rd item (IE8-specific testacular configuration)? I'm concentrating on that ATM, I'm creating a grunt parametrized task that runs tests with the standard configuration or with the IE8-specific one |
This reverts commit 6e9b8f5.
… ie8-compatibility
|
@pkozlowski-opensource The version of es5-shim hosted on cdnjs is two years old... I'll try and make a push on their repo so that it will be updated edit: |
|
@redaemn I'm starting to look on the commits in this PR right now. First of all, this is great stuff, thnx so much helping on this and pushing us to provide proper IE8 support :-) |
|
@redaemn great stuff. I went over all the commits and left some comments. As for the separate testacular config - I don't think we need additional complexity of it right now. I would simply include both a polifil for JavaScrip and a shiv for tags and be done with it. New tags shouldn't bother other browsers and normally JavaScript shivs are done in the way that shouldn't have any effect in modern browsers. Disclaimer - didn't try the above so I might be wrong :-) I would like keep changes to the build system minimal since we start to have many devs and 2 CI servers. |
|
Great work guys. I've been using the es5-shim with angular-bootstrap. I suppose writing IE8 compatible js is not an option? (only asking because angular itself doesn't require the shim - maybe they have something built in) Something else to note is that IE8 seems quite sensitive to DOM structure. E.g. a blank line in the wrong place can cause the angular engine to fail. This is probably a general angular thing but I noticed it first with angular-bootstrap. |
|
@adam77 thanks for your suggestion! I'll try to keep that into consideration. |
- Modified grunt server task to accept an :ie8 parameter that lets you run testacular using IE8-specific configuration
I just committed what I had in mind, seems pretty similar to what you said; WDYT? Is the update to |
…ave spaces between the open parenthesis and the name of the directive
…ful creation of the file
…included ONLY if explicitly used inside one of the project's directives
|
@pkozlowski-opensource I started to look at the tests in IE8, first thing I found was that some templates inside tests were created using tag names (i.e. I posted a question on the karma project karma-runner/karma#430 about this issue, waiting for the answer. Do you think that it would be bad to substitute the templates with the attribute version (i.e. |
|
@redaemn Great effort, you are really having a good look into it, awesome! As for testacular tests - I guess that ieshiv needs to be executed very early in the process and it might be too late for files loaded by testacular. Let's see if anyone will respond to the opened issue. We could change templates to use attributes but I would like to explore all other options before doing so :-) Would really like to help you on this one but I'm desperately trying to finish AngularJS book atm... |
|
@pkozlowski-opensource No problem!! I do it because I like it!! :) |
|
IE7.js is pretty buggy based on my experience, it solve some problems, also introduce some new problems(CSS related). The project lost developer's love for almost 3 years. |
|
@L42y we are targeting https://github.com/kriskowal/es5-shim as a shim right now. Any opinion on this one? Or a suggested alternative to IE7.js? |
|
@pkozlowski-opensource I think es5-shim is fit and enough for this project, IE7.js mainly patch IE for CSS related functionalities and bugs(http://ie7-js.googlecode.com/svn/test/index.html), they're not this project's job anymore. |
There was a problem hiding this comment.
As the comment says, this row is needed by the ieshiv grunt task because the <tab-heading> tag can be used when building a tabSet but it does not correspond to an angular directive, instead it is parsed by the tab directive as you can see here. The ieshiv task searches the sources to find all directives declarations and without this line it wouldn't find it.
Maybe I should find a cleaner solution, maybe define an empty tabHeading directive... ideas?
There was a problem hiding this comment.
This looks OK to me. In the future we should have actual comments that say @directive which will generate docs, and the ieshiv task could perhaps look for those. For now though, this is good :-)
|
@redaemn This it totally epic!!! You rock! It is a big PR with many changes so it will take a bit of time before it get merged. Hopefully I will have a bit of time this weekend to start reviewing it. But rest assured - it will get merged! |
There was a problem hiding this comment.
Strange thing here, inside this directive's template I had to substitute the custom tag name with the corresponding custom attribute version to make it work in IE8, even though the custom tag name was declared inside the ieshiv file. I'm beginning to think that this could be an angular bug with directive's templates containing custom tag names in IE8 but I have to investigate further the issue.
Does anyone have any suggestion?
There was a problem hiding this comment.
That's entirely possibly an angular bug ... Maybe open an issue with them?
But for us, just switching to the attribute is good.
There was a problem hiding this comment.
Definitely an Angular bug, I was able to reproduce it!! Just opened an issue: angular/angular.js#2751
|
Hi @redaemn ! Could you rebase this against master and fix the conflicts? Then we could look at merging it :-) |
… ie8-compatibility Conflicts: src/accordion/test/accordionSpec.js src/typeahead/test/typeahead.spec.js
|
@ajoslin this PR seems to be done to me, I just made some finishing touches:
let me know if you need anything else. |
|
I'm totally new to this - I haven't ever contributed or anything but I figure this is the right place for it. I had to use IE8 with a Typeahead and found a bug and a fix for it (If this was already fixed I'm sorry - I read through the post and didn't see it). If you have a typeahead at the bottom of the screen and type in something without scrolling down it works fine - however if you have scrolled down and then you type in something into the typeahead it will appear higher up on the page (sometimes even off the page depending on how far you've scrolled). The issue is in typeahead.js line 103: I had to change it to this to make it work in IE8 (There's no guarantee this works in other browsers - it does work in Chrome): |
|
Good catch @schriste! I will look into this one!! |
|
can u give us an estamitation when this PR will be integrated? |
|
@dcg Sorry for the delay... The thing is that we are just pondering IE8 support... |
|
@pkozlowski-opensource Are you wondering if IE8 support is worth the effort? I have some statistics of some big German non technical websites where 20-40% of all IE Users are using IE8. So I guess even if this browser is no fun at all it is worth the effort. |
|
@dcg Would you like to be a maintainer of the IE8-specific branch? We are looking for someone who could give us a hand with this. Please jump into the discussion on angular-ui@googlegroups.com |
|
@pkozlowski-opensource i would like to, but i'm too new to angular.js in particular and to javascript development in general. So I guess i dont have the needed skill for this at the moment. |
|
This is the problem @dcg. We know that IE8 has a considerable market share in certain places (corporate world mostly). But supporting it is considerable effort. So we either need someone who would be willing to put this effort or sponsor it. |
|
According to the FAQ, AngularJS itself supports IE8 (providing you follow certain conventions), so our team made the assumption that the official companion suite(s) would too, and started incorporating them extensively into our build. It actually came as a fairly nasty surprise when we realised ui-bootstrap did not support IE8. At the very least, I would like to see this made clearer in the documentation, to help prospective user pick the right frameworks for their build. However, having developed some 'jQuery-free' AngularJS directives myself, I understand this is a tricky decision. You could easily end up trying to recreate all the cross-browser goodness of jQuery 1.9, and failing. Going IE9 + is certainly a valid choice for a framework these days, but the fact the AngularJS's own support goes lower does suggest to me that (maybe) the companion suites should too(?) I'd gladly jump into the discussion on angular-ui@googlegroups.com, but I'm not quite sure how to do this. Is there a specific thread that I can look at? https://groups.google.com/forum/#!categories/angular-ui/bootstrap Thanks. |
|
@pkozlowski-opensource I'd like to discuss about being the maintainer of IE8 compatibility; right now I'm leaving for vacation and in august I will be occupied with a job but from september/october I would be glad to help during my spare time (evenings and week-ends) |
|
@redaemn Awesome. Ping us on the angular-ui mailing list when you've got time so we can discuss the details. |
|
Hi, I read many comments here stating that ui-bootstrap don't have support for IE8 but as I can see, if you follow angular.js tips for IE compability it works fine - http://docs.angularjs.org/guide/ie . As you can see the main problem with IE8 is not reading custom tags like <tabset> or <accordion>. So if you, acording to docs change it to something like <div tabset> it should work like a charm, without adding html5shiv etc. I didn't test all ui-bootstrap components but that ones I did test works fine. P.S. Tested with newest angular 1.1.5. |
|
@redaemn I've clarified current policy for IE8 support in https://github.com/angular-ui/bootstrap#supported-browsers If you are still into driving IE8 support your help would be more than welcomed!!! Just ping us here or on the mailing list whenever you want. Going to close this one for now as I'm cleaning up PRs / issues queue before the new release. |
9 participants
Hi all,
I'm trying to address issues #8 and #93. I'm creating this PR just to have a place to discuss about what to do and how, the work is not done yet (obviously...)
Just for a start, I edited the demo page to see how it feels in IE 8
ieshiv.jsfrom angular-ui and inserted in the demo page as wellMany widgets seemed to start working correctly just with this little effort (see yourself)
TODO (just as they come to mind)
ieshiv.jsbetween the demo-assets, as a next step I'd like to edit thebuildtask and make it generate the shiv automatically. I think it should be released together with the framework, because it should stay in sync with the framework version, but it should be an external optional file (I mean, not minified with the framework) and so it should be moved from the demo-assets directory. Any idea where to put it? I don't want to put it inside thesrc, it's not a module, it's more of optional stuff.ieshiv-template.jsfile, currently it has many useless things inside...