Skip to content

Conversation

umar-khan
Copy link

Description

Currently the Download.js component does not work on IE11.
Instead of downloading the file, it asks the user to choose an app to open the blob file.

Changes

Update the componentDidMount() method in Download.js to check if window.navigator has a msSaveOrOpenBlob method. This method should only be available on IE so it shouldn't other users.

If msSaveOrOpenBlob exists, we use the msSaveBlob to download the file.
This functionality should be consistent with Link.js component.

Steps to Reproduce

I'm able to reproduce this consistently using IE11 on Windows 10.

  1. Have the Download.js mount after a specific action (this should trigger the file download to begin).
  2. componentDidMount() in the Download.js component triggers window.open which opens a new window.
  3. The new window should result in a file download, but instead users are prompted to allow this website to open an app.

app_prompt

  1. If user clicks Allow they are prompted to choose an app to open the blob file.

blob_app_selection

  1. Searching the Windows Store for an app returns no results.

app_store_search

@coveralls
Copy link

coveralls commented Sep 5, 2018

Coverage Status

Coverage decreased (-2.4%) to 80.208% when pulling ed977a1 on umar-khan:ie11-download-component into dbc0467 on abdennour:master.

@umar-khan umar-khan force-pushed the ie11-download-component branch from ed977a1 to c742cef Compare September 13, 2018 21:15
@mccabemj
Copy link
Collaborator

mccabemj commented Oct 12, 2018

I think this will be a useful addition to support download on IE11 since the change to using blob. However can you add tests for it. @umar-khan Maybe in your test, define then delete the presence of msSaveOrOpenBlob

Object.defineProperty(window.navigator, 'msSaveOrOpenBlob', {value: true});

@mccabemj mccabemj requested review from mccabemj and removed request for mccabemj October 12, 2018 13:03
@mriccid mriccid force-pushed the master branch 2 times, most recently from 4c691cb to f129262 Compare February 5, 2019 05:12
@mccabemj
Copy link
Collaborator

@umar-khan can you confirm if this is still an issue in v1.1.2?

Copy link
Collaborator

@abdennour abdennour left a comment

Choose a reason for hiding this comment

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

  • Test coverage decreased

);
const {data, headers, separator, uFEFF, target, specs, replace, filename} = this.props;

if (window.navigator.msSaveOrOpenBlob) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great feature! But unit-tests are required for this block

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.

4 participants