onKeyDown callback issue

[ankurk91]

Your Environment

• flatpickr version used: 4.5.1
• Browser name and version: Chrome 67 stable
• OS and version: Any (eOS 4.1 Loki)

JSFiddle

https://jsfiddle.net/p5tc0mg3/1/

Current behavior

onKeyDown callback is being called on every key press from keyboard

Expected behavior

The onKeyDown callback should be called only when flatpickr is doing something internally, for example, close on escape or set value on enter.

Steps

• Pass a onKeyDown callback in config options

onKeyDown: function(selectedDates, dateStr, instance) {
    console.log('onKeyDown: ', selectedDates);
  }

• Open dev tools -> console
• Press any key on keyboard
• Notice that onKeyDown callback is being called

[melgharbawy]

What happened to this issue? how was it resolved? I'm experiencing the same where if the onKeyDown event is configured, it will end up getting executed on every key press regardless of being in the date picker or not.

[ghiscoding]

I also agree this should be reopened and fixed, this a huge problem and is not following good practice of separation of concerns. The fix should have been following what was written in this other issue #1387 and I also wrote the some concern in that issue

This should be reopen and should have been followed according to the original proposition, the current keyDown event is bound to window.body and this is crazy bad, that means that even if the element is not a Flatpickr element, it will still go through that onKeyDown handler, this is bad, bad and is not following the separation of concerns, this is considered leak... and it slows down someone's Angular App using my lib (Flatpickr is a dependency of my lib Angular-Slickgrid) as shown in this post

bind(window.document.body, "keydown", onKeyDown);

There's actually all these other events that are attached to the window.document, again a huge separation of concerns. In some of them it checks if it has the isOpen but that is not enough make sure the element that is listened to is not a Flatpickr element and if so it should disregarded right away but the current code doesn't, see all the other window event (from line 419).

if (!self.config.inline && !self.config.static)
      bind(window, "resize", debouncedResize);

    if (window.ontouchstart !== undefined)
      bind(window.document, "touchstart", documentClick);
    else bind(window.document, "mousedown", documentClick);
    bind(window.document, "focus", documentClick, { capture: true });

    bind(self._input, "focus", self.open);
    bind(self._input, "mousedown", self.open);

Perhaps provide a way to disable the keyDown if we wish to? Not sure why allowInput: false wouldn't disable the keyDown, why do we use keyDown when the input is read only?

[ghiscoding]

I have created PR #2329 to fix this since current code is causing too much performance downgrade (the more Flatpickr inputs we create, the more time it will re-execute the onKeyDown listener (1 character typed will execute the listener 10x times if we have 10x Flatpickr inputs which is a real killer in perf).