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

IE8 support #264

Closed
tomwasd opened this issue Aug 10, 2015 · 4 comments
Closed

IE8 support #264

tomwasd opened this issue Aug 10, 2015 · 4 comments

Comments

@tomwasd
Copy link

tomwasd commented Aug 10, 2015

Hi,

Thanks for sorting out IE9 - that works nicely now thanks!

I think I've managed to get IE8 working in a small test app that I've made.

The only changes required to flow-router would be to create weak dependencies on two packages (so that if the user adds them they're definitely loaded before flow-router).

api.use('tomwasd:history-polyfill', {weak: true});
api.use('tomwasd:flow-router-ie8', {weak: true});

You can see an example here: http://www.github.com/tomwasd/flow-router

My version of the history-polyfill uses the latest version of the history polyfill (https://github.com/devote/HTML5-History-API). tmeasday's package uses an older version of the polyfill that seemed to cause an infinite redirect loop in IE8.

The flow-router-ie8 package adds the three polyfills required to get page.js working: one for event listeners, one for Object.create and another for forEach. The polyfills are all those recommended on the MDN site.

Edit: looks like a couple more polyfills are required for isArray and Object.keys - I've added these to the package.

The packages are on github and atmosphere. Afraid I haven't done very extensive tests - just enough to see that it actually loads the page and navigates on my site which is the extent of IE8 support I need.

https://github.com/tomwasd/flow-router-ie8
https://github.com/tomwasd/history-polyfill

Not sure if you want to add IE8 into core but this will hopefully be useful if anyone else reading wants to try and hack in their own IE8 support.

@arunoda
Copy link
Contributor

arunoda commented Aug 12, 2015

I think we don't need anything from the FR layer for this. Of course we can ask use your wrapper. How about using Autopublish for the history polyfill.

If you can work on that, that's super awesome.
May be @alanning also interested in it.

@alanning
Copy link
Contributor

Yes, I'd be interested in helping out. Seems like the only change to FR would be to update this section in the readme. I can do a PR for that if you'd like. Adding the weak package references would be best since then load order will be handled automatically but that's not required.

I think there's also some tests missing since the complete FR test suite was passing for me and @pahans in IE9 even though @tomwasd reported that clicking links didn't work in a real app. Not sure if anything needs to be tested specifically for IE8 but at least we should do something for IE9 to help prevent future regressions.

Autopublish looks pretty cool. Looks like you just have to add their userbot to your org and it can publish for you.

@arunoda
Copy link
Contributor

arunoda commented Aug 12, 2015

Hi,

It was an issue with the clicking. I think it's hard to write a test case
for that.

We fixed it.

I think I will stay away from adding it as a weak dependency. We can add a
proper read me asking to add it before the flow-router.
On 2015 අගෝ 12, බදාදා at ප.ව. 8.14 Adrian Lanning notifications@github.com
wrote:

Yes, I'd be interested in helping out. Seems like the only change to FR
would be to update this section
https://github.com/kadirahq/flow-router#ie9-support in the readme. I
can do a PR for that if you'd like. Adding the weak package references
would be best since then load order will be handled automatically but
that's not required.

I think there's also some tests missing since the complete FR test suite
was passing for me and @pahans https://github.com/pahans in IE9 even
though @tomwasd https://github.com/tomwasd reported
#254 that clicking links
didn't work in a real app. Not sure if anything needs to be tested
specifically for IE8 but at least we should do something for IE9 to help
prevent future regressions.

Autopublish looks pretty cool. Looks like you just have to add their
userbot to your org and it can publish for you.


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

@arunoda
Copy link
Contributor

arunoda commented Aug 16, 2015

Closing this and waiting for the PR to the README.

@arunoda arunoda closed this as completed Aug 16, 2015
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

No branches or pull requests

3 participants