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

Made some modifications #9

Open
kctang opened this issue Mar 21, 2019 · 8 comments
Open

Made some modifications #9

kctang opened this issue Mar 21, 2019 · 8 comments

Comments

@kctang
Copy link

kctang commented Mar 21, 2019

Hi,

Thanks for this.

I made some changes for my TypeScript based application as the provided TS definitions was not good for me.

The only minor issue I have is the promise will never resolve or reject if user clicks cancel when the file input dialog is opened - potential memory leak if done repeatedly.

Sharing here FWIW:

// modified based on alnorris/file-dialog
// https://github.com/alnorris/file-dialog/blob/master/file-dialog.d.ts
// - removed support for callback
// - improved TypeScript type definitions
// - directly click() on input
// - do not add input to DOM (seems to work in Chrome - no plans to support MSIE)
export const fileDialog = (options?: { multiple?: boolean, accept?: string }) => {
  const input = document.createElement('input')

  // Set config
  if (options) {
    if (options.multiple) {
      input.setAttribute('multiple', '')
    }
    if (options.accept !== undefined) {
      input.setAttribute('accept', options.accept)
    }
  }

  input.setAttribute('type', 'file')

  // Return promise
  return new Promise<FileList | null>(resolve => {
    input.addEventListener('change', () => resolve(input.files))
    input.click()
  })
}
@alnorris
Copy link
Owner

@kctang I'm unable to find a proper solution to this, as there is no API available to tell if a user cancels

@icfantv
Copy link

icfantv commented Dec 11, 2019

@alnorris correct. I was able to hack around that issue with the following:

            let firstTime = true;
            filePicker.onclick = function(event: MouseEvent) {
              // check for cancel being clicked
              if (firstTime) {
                firstTime = false;
                const input: HTMLInputElement = event.target as HTMLInputElement;
                if (!input.value.length) {
                  return reject();
                }
              }
            };

NB: We are not using this component in our code, but were thinking about it. But this issue would be a blocker for us in consuming this.

@alnorris
Copy link
Owner

@icfantv Please make a PR! 🙏🏻 I will review and if it works I will definitely merge

@icfantv
Copy link

icfantv commented Dec 11, 2019

Hah. Ok. I'll try to take a look either tonight or tomorrow.

@icfantv
Copy link

icfantv commented Dec 12, 2019

@alnorris haven't forgotten about this. I'm trying to test out my change in our environment and the environment is having issues...which is higher priority...

@icfantv
Copy link

icfantv commented Dec 13, 2019

So the primary problem here is that a click handler is triggered when the Browse button is clicked and nothing is triggered when the user clicks Cancel. The Internet has provided a hack mechanism using the onfocus event handler of document.body to solve this...and there's loads of issues with that.

The onchange event handler is only triggered when the user selects a file (or files).

So, so far as I've been able to prove, there's no way to solve this in a generic Promise-like fashion.

@icfantv
Copy link

icfantv commented Jan 17, 2020

So, it turns out I/we have to solve this problem. I'm going to take another crack at it, but know that the solution will basically be the Internet hack. I will attempt to get it working in this library, but it's going to be up to you, @alnorris, if you want to actually uptake it.

@icfantv
Copy link

icfantv commented Jan 22, 2020

Ok, I have a working solution for this, however, it requires/involves a hack. The "chosen" solution on the web to detect when cancel is clicked is to add a focus listener to document.body to detect when the file picker window is closed. The problem with this approach is that that focus handler can fire before the onchange handler for when the user does select a file. The only solution I found to work around this was by using window.setTimeout which I thoroughly loathe but we effectively need a way to execute a conditional reject, I.e., only do a reject if the change handler isn't invoked.

I'm happy to paste the logic, but I'm not convinced it's the right way to go. PLMK what you'd like to do.

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