handsontable / handsontable

JavaScript data grid with a spreadsheet look & feel. Works with React, Angular, and Vue. Supported by the Handsontable team ⚡
https://handsontable.com
Other
20.01k stars 3.04k forks source link

Inconsistent hook targeting/firing #4650

Open YakovL opened 6 years ago

YakovL commented 6 years ago

This issue originates from a complicated app/scenario which I haven't reproduced literally outside the app; however, I've made a fiddle that shows a similar issue so I hope my report will be helpful for either fixing, working around or for me understanding how to use hot better to avoid such problems.

To simplify things, let's start from the fiddle. It's somewhat self-explanatory: the hot instance has a 'beforeKeyDown' hook, second keypress throws cursor into the text input before the hot; however, next keypresses still fire the hook, although typed stuff goes of the input – despite the fact that in the handler event.target is the text field (why it was body instead of the hot container – I have no idea).

In my app, one of the problematic scenarios is the following: I have a hot instance with a 'beforeKeyDown' hook and an area with static text and "edit" button which substitutes the static text with a textarea. If I click inside the hot instance (and may be do smth there but not necessary) and then click the "edit" button and start editing, the hook keeps firing there, despite the fact I'm editing outside hot. If, however, I click inside that textarea, the hook stops firing on keydown.

Any suggestions? For now, the only hack-fix I can think of is to check the target of the hook event and return from handler if it's not body.

PS Tested the fiddle in Vivaldi, FireFox and Chrome, results are the same.

AMBudnik commented 6 years ago

Hi @YakovL

Thank you for sharing this example. There are some issues related to the subject at https://github.com/handsontable/handsontable/issues/4293 and https://github.com/handsontable/handsontable/issues/4082. We need to check it. It looks like when we already have the focus on the table we are not able to pass it to other elements.

YakovL commented 6 years ago

Hi, actually, the overall issue in multiple cases seems to me to be the following: the beforeKeyDown hook, even if attached to a hot instance, fires elsewhere once we have selected any cell of that instance. Like I've mentioned, the event's target is document.body, not the hot instance container.

Although I failed to create a minimal reproducible example in jsfiddle, I've made one in plain html. Here's the code (change the paths to hot.js and hot.css from those I used locally):

<html>
  <head>
    <link rel="stylesheet" type="text/css" href="../jsLibs/handsontable/hot.min.css">
    <script src="../jsLibs/handsontable/hot.min.js"></script>
  </head>
  <body>
    <div id="inline_table1"></div>
    <script>
var data1 = [
    ["xx","Ford","Volvo","Toyota","Honda"],
    ["2014","10","14","13","text"],
    ["2015",20,"12","14","13"],
    ["2016","40","","12","24"],
    ["","","","df",""]
];

var hot = new Handsontable(document.getElementById('inline_table1'), {
    data: data1,
    minSpareRows: 1
});

hot.addHook('beforeKeyDown',function(event){
    console.log('key handler fired!',event.target);
});
    </script>
  </body>
</html>

The problem is, once we click inside HOT, the event/handler fires on key ~press (or down or up, haven't tested) elsewhere and when I click just outside HOT and press a key, event.target is document.body, just like inside HOT, so I have no idea how to work around this. Or rather, like I mentioned above, the problem is event.target is document.body inside HOT, not the HOT container or certain cell. PS Well, the latter statement is not entirely correct: once I press a letter button and hence start typing inside a cell, the second press gives event.target of either hidden

<textarea class="copyPaste" style="width: 10000px; height: 10000px; overflow: hidden; opacity: 0;"></textarea>

or

<textarea class="handsontableInput" style="width: 43px; height: 22px; font-size: 16px; font-family: &quot;Times New Roman&quot;; resize: none; min-width: 43px; max-width: 837px; background-color: rgb(255, 255, 255); overflow-y: hidden;"></textarea>

which is the textarea inside the cell editor.

AMBudnik commented 6 years ago

Thank you for your effort to investigate it and share a reproducible example. I hope that our developers will be able to take a look at it soon. This seems to be nearly solved but there are always some other parts of the code that need to be retouched to work.

AMBudnik commented 5 years ago

I wanted to replicate this issue using 7.1.1 but it doesn't fire when I use the outsideClickDeselects: false and type anything in the input. Mentioned

the overall issue in multiple cases seems to me to be the following: the beforeKeyDown hook, even if attached to a hot instance, fires elsewhere once we have selected any cell of that instance.

http://jsfiddle.net/z98rLev1/

Hey @YakovL if you're still doing the project can you tell me what I'm missing to replicate the issue using 7.1.1?

jmanos commented 5 years ago

I have a related issue in a project where I am forcing the user into an input outside the HOT table when certain keys are pressed in cells.

The following fiddle represents a simplified case where the beforeKeyDown event is firing incorrectly: https://jsfiddle.net/tm9dqpc8/1/

To replicate the issue, type a key into the HOT cell to be taken to the input. Then, press any key. The event will fire even though focus is outside the table and the HOT cell has been deselected. Clicking anywhere after the input has focus will prevent further misfire (until another key is typed in the HOT cell). This fiddle doesn't use outsideClickDeselects but it did not solve the problem in this case.

AMBudnik commented 5 years ago

Thank you for sharing the example @jmanos You're right. The value is transferred to the input. This behavior stops if we add a delay to the input focus.

setTimeout(function(){test_input.focus()}, 1)

I think it might be connected to https://github.com/handsontable/handsontable/issues/6236

pnowak commented 4 years ago

This issue is still replicable using v8.0.0. Here's an updated demo - https://jsfiddle.net/nrw53dz6/

aninde commented 2 years ago

Still valid with v12.0.0 https://jsfiddle.net/7u6xoezq/