Row Selection and Column Selection Keyboard Nav not working
Hi! Not sure if this bug is related with the problem i've found but here it goes. While testing the new 2.0.0 release, i've noticed that the keyboard navigation was broken, noticeable by the following error in chrome dev tools:
Uncaught TypeError: Cannot read property 'getCellOwnProperties' of undefined
at CellEvent.get (cellEventFactory.js:56)
at CellEvent.get (cellEventFactory.js:61)
at Constructor.handleKeyDown (CellSelection.js:112)
at Constructor.handleKeyDown (Filters.js:34)
at Constructor.onKeyDown (Behavior.js:916)
at Constructor.delegateKeyDown (events.js:579)
at events.js:456
at HTMLCanvasElement.decorator (events.js:30)
at Canvas.dispatchNewEvent (Canvas.js:311)
at Canvas.finkeydown (Canvas.js:451)
This does not affect v1.3.0, but was already present in 2.0.0-alpha.
From the demo below, we can see that this somehow related with the grid being cropped in the viewport (having more columns that it can show).
For some reason, when it tries to handle the 'keydown' event by retrieving cellOwnProperties, the CellEvent looks empty, without the column property and this.column.getCellOwnProperties throws up.
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width">
<title>JS Bin</title>
<script type="text/javascript" src="https://openfin.github.io/fin-hypergrid/build/fin-hypergrid.js"></script>
</head>
<style type="text/css" media="screen">
.grid {
width: 800px;
height: 220px;
margin-bottom: 12px;
background-color: #fafafa;
}
</style>
<body>
<h1>Keyboard navigation bug demo</h1>
<div id="smallGrid" class="grid"></div>
<div id="bigGrid" class="grid"></div>
<script type="text/javascript">
function createRow(text, cols) {
var arr = {};
for(var i = 0; i < cols; i++) {
arr['key_' + i] = text;
}
return arr;
}
var smallGrid = new fin.Hypergrid(document.getElementById('smallGrid'), {
data: [
createRow('this', 10),
createRow('works', 10),
createRow('great', 10),
createRow('with', 10),
createRow('a', 10),
createRow('few', 10),
createRow('columns', 10)
]
});
var bigGrid = new fin.Hypergrid(document.getElementById('bigGrid'), {
data: [
createRow('but', 20),
createRow('it\s', 20),
createRow('bugged', 20),
createRow('when', 20),
createRow('there', 20),
createRow('are', 20),
createRow('columns', 20),
createRow('outside', 20),
createRow('viewport', 20)
]
});
</script>
</body>
</html>
I'll try to take some time to see this. Thanks!
Looks like the method scrollToMakeVisible() in Hyppergrid.js was updated in line 1260 to use (delta = c - dw.origin.x + 1) > 0 instead of (delta = c - dw.corner.x + 1) > 0.
By changing this back, I'm able to navigate using the keyboard again, but there are still issues:
- cannot mouse scroll/navigate with keys until the bottom of the grid (e.g. can only go until row 128 in a grid of 150 rows);
- cannot expand selection with SHIFT + ARROW_DOWN until the bottom;
- can expand a fixed number of rows before crashing;
- the number is the same regarding if we start expanding downwards from a cell at the top of the viewport or at the bottom (scrolls fine before crashing);
Both issues are also present in release v1.3.0.
@hugoeanogueira Thanks for the extra information. We will add this to our backlog. You will get an update when its in our sprint. That is unless you wish to submit a PR
Found out that the first issue, the render only goes to row 129 of 150, can be easily bypassed if we use grid.setData() instead/after passing the data in Hypergrid's constructor. I'll try to do some more investigation on this (e.g. test without react around) before opening any PR. Thanks!
Awesome. Sorry we're not available to look into it right away, but definitely want more community involvement
No problem, you've done a lot ;)
I've found out what was causing the cropped scroll problem on first render. Looks like when you update the defaultRowHeight with grid.addProperties, the getFixedRowsHeight() reports an outdated value for defaultRowHeight. This can be easily fixed by calling synchronizeScrollingBoundaries inside grid.refreshProperties().
I'm opening a PR on this.
Managed to do some work (PR #598).
I've found some more issues regarding the user selection overriding the column name and the editor not opening on keyup when outside viewport (google docs and others scroll into view and then open the editor). I'm going to open different issues for these two.
Hope this helps!
Awesome, thanks!