Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: support IE9 #604

Closed
wants to merge 1 commit into from
Closed

feat: support IE9 #604

wants to merge 1 commit into from

Conversation

marclaval
Copy link
Contributor

This PR adds support for IE9,. which doesn't require that many changes.
Some more details about what was needed to make the demo app to work and the full test suite to pass:

Source:

  • Time picker: in IE9, a disabled <fieldset> doesn't disable children.

Demo:

  • Added classList polyfill to the demo website.

Testing:

  • Added Angular 2's testing polyfills to Karma's setup.
  • Increased Karma's timers for slow IE9.

Spec:

  • Pagination: in IE9, with the polyfill, classList need to be retrieved each time.
  • Progress bar: no value attribute in IE9 (not sure why attribute was used).
  • Radio group: some race condition was happening, making this test to fail. Note: it was occuring also in Chrome when focusing this particular test suite, but not when running the full campaign.
  • Typeahead: IE9 requires a different way to fire events.

@maxokorokov
Copy link
Member

maxokorokov commented Aug 17, 2016

Actually instead of adding classlist polyfill I would throw it away altogether and fix that one test with more usual expect(...).toHaveCssClass('...'). I think it was one of the first tests I wrote and forgot to fix that...

P.S. Ah, I see you need it for the demo too

@pkozlowski-opensource
Copy link
Member

@maxokorokov I think that we need a polyfill for the core of Angular itself (ex. [class.foo] is using classList behind the scenes). But yes, we could / should change this test for sure.

captureTimeout: 60000,
browserDisconnectTimeout : 60000,
browserDisconnectTolerance : 3,
browserNoActivityTimeout : 60000
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super minor, but probably should remove the spaces before : here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants