lifenautjoe / droppable

A javascript library to give file dropping super-powers to any HTML element.
https://lifenautjoe.github.io/droppable
MIT License
204 stars 13 forks source link

Getting elementKeyDownEventRemover() is not a function when removing file preview #17

Closed Fralleee closed 6 years ago

Fralleee commented 6 years ago

We are using the drop as a file uploader where you can clear the files when the upload is finished.

When I clear a file, this is done by clicking the file, I get the error message: this.elementKeyDownEventRemover() is not a function when removing file preview

This happens if I drop a file and clear it by clicking on it. If I however focus the droppable element beforehand, clicking on it and browse to the file. I don't get the error.

Since we're using react I can do a workaround like this: componentDidMount() { this.dropzone.focus() }

But it shouldn't be necessary.

lifenautjoe commented 6 years ago

Hey @Fralleee , thanks for reporting this ! I will look into/fix it right away .

Cheers !

lifenautjoe commented 6 years ago

I believe I found the issue on #18 , included in release 2.1.3 #20 and released in a few minutes.

lifenautjoe commented 6 years ago

Hey @Fralleee , could you update your droppable package to 2.1.3 and try again?

npm update droppable --save should do it.

Edit: Just as a double check, are you calling droppabe.destroy when the HTML element is destroyed ?

Fralleee commented 6 years ago

Hey, updated to 2.1.4 and I still get the error.

Screenshot of error+stack: https://imgur.com/a/dk0Sl

lifenautjoe commented 6 years ago

Hi @Fralleee .

For some reason the element you are making droppable does not trigger a focus event, so the event needed for accessibility support is never added.

I will fix the error by making sure the remover gets called only if the event was added but it would be great if you could give me some info on the type of element (div, input ... ) , if you have set thetabindex attribute manually and whether you are doing something with focus events.

This will help make the library more robust.

Thank you in advance! I'll ping you once the fix is released.

Fralleee commented 6 years ago

I am using a regular div as my element, not setting tabindex on the element. Code I am using: https://gist.github.com/Fralleee/36657a6aeb5483d5665b7b75bf101b48

Is the droppable contructor supposed to focus the element?

lifenautjoe commented 6 years ago

Hi @Fralleee , the library adds a tabindex=0 attribute to make the element focusable, as the accessibility button spec requires. It then adds an event listener for focus to handle enter key down (also accessibility spec).

In your case, it seems, that the focusout event is called but the focus is not.

I'll set up a demo with your code and get to the core of the problem.

Thanks for sharing it!

lifenautjoe commented 6 years ago

Hey @Fralleee , I believe fixed with #21 and deployed with #22 .

Please run npm update droppable --save once more.

Fralleee commented 6 years ago

Works great. Good job!