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

UI: File API compat for IE #4376

Merged
merged 2 commits into from
Apr 17, 2018
Merged

UI: File API compat for IE #4376

merged 2 commits into from
Apr 17, 2018

Conversation

meirish
Copy link
Contributor

@meirish meirish commented Apr 17, 2018

IE11 / Edge both don't seem to have a File constructor, so this detects the MS-specific API for saving Blobs, and will use it instead of using the File constructor to add download links to the UI.

Before: it'd error out the rendering on the Show page leaving a blank textarea.

After:
screen shot 2018-04-16 at 9 54 38 pm

Manually tested in IE11 Win7 (Note: we need to get cross-browser testing set up and getting the UI tests up and running in CI in general).

@meirish meirish requested a review from a team April 17, 2018 14:07
event.preventDefault();
let file = this.get('fileLike');
//lol whyyyy
window.navigator.msSaveOrOpenBlob(file, file.name);

Choose a reason for hiding this comment

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

Wow, this is annoying. Makes it harder to pretend that browsers are all the same.

It would be nice to hide all this msSaveOrOpenBlob stuff in some polyfill rather than in this component, but if it's only going to be used here, I guess it's not so bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah @johncowen mentioned https://github.com/eligrey/FileSaver.js/ which may be worth looking at if this gives us any other issues. I think for now since it's just in this component we'll try this for now.

Copy link

@DingoEatingFuzz DingoEatingFuzz left a comment

Choose a reason for hiding this comment

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

Just one non-blocking comment. Thank you for fighting the good fight.

Copy link
Contributor

@joshuaogle joshuaogle left a comment

Choose a reason for hiding this comment

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

Can confirm this is working for me now. Thanks!

@meirish meirish merged commit fc82415 into master Apr 17, 2018
@meirish meirish deleted the ui-ie-policy-show branch April 17, 2018 20:02
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.

3 participants