cvn / angular-shims-placeholder

Angular directive to emulate the placeholder attribute on text and password input fields for old browsers, such as IE9, IE8, and IE7. Also works on textareas and html5 input types.
MIT License
63 stars 26 forks source link

Added support for IE7 #23

Closed danielcrisp closed 9 years ago

danielcrisp commented 9 years ago

There are a number of changes, most are for the demo page and a couple are for the directive itself.

I'll explain them here, so you can understand what I've done.

Demo

The last version of Angular that works properly in IE7 is v1.0.8 so if IE7 is detected (using conditional comments) the demo page will use 1.0.8 from the CDN. If you're using the bower version you might end up with a later version that doesn't work (not sure how to get around this).

index.html

bower.json

Gruntfile.js

The changes in the directive were actually pretty minimal.

lib/angular-placeholder.js

cvn commented 9 years ago

This is great! I haven't tested it yet, but it looks good.

There's one thing I'm interested in changing. Instead of making an IE7 user polyfill $animate, I'd like to just handle a missing $animate within the directive. I think it might be as simple as adding if ($animate === undefined) { updatePasswordPlaceholder() } else if ... to the asyncUpdatePasswordPlaceholder function. Without $animate, there's no need for it to be async.

@danielcrisp if you're interested in giving this a shot, go for it, otherwise I'll try it myself soon.

After that, I'd like to get this merged. :)

danielcrisp commented 9 years ago

Makes sense... but because it is being called as a dependency I'm not sure how it would work. If Angular can't find it when it constructs the directive it will error before getting to the if ($animate === undefined)... bit.

I'll see if I can figure something out that involves packaging it up within the directive file rather than in the HTML

Might have time today

cvn commented 9 years ago

I think it's possible using $injector. http://stackoverflow.com/a/18542095 I started doing this but then got preoccupied trying to figure out why the jasmine tests fail with angular 1.0.8. I'm on my way to sleep now, but I'll post what I have tomorrow if you haven't already done it.

cvn commented 9 years ago

Hey @danielcrisp, I was able to eliminate the need to add $animate and the ng-hide styles manually. Now, the directive should work with Angular 1.0.8 and IE7 without any config.

I pushed my changes to this branch: https://github.com/cvn/angular-shims-placeholder/tree/ie7

I don't have a copy of IE7. Can you check out that branch and test it for me?

If it works for you, I'll merge it all to master and make a release.

danielcrisp commented 9 years ago

Great work - yes it works perfectly if you comment out the bower version and use 1.0.8 from the CDN. If you use the bower version (1.3.15) you get errors in IE7, so probably worth making that clear to people in the docs / HTML.

Tested in IE7 on Vista

cvn commented 9 years ago

I added a note to the readme (in the known issues section) about Angular versions and IE7/8.