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

Keyboard mixin throws TypeError #1297

Closed
zub2 opened this issue Jul 27, 2016 · 1 comment · Fixed by #1469
Closed

Keyboard mixin throws TypeError #1297

zub2 opened this issue Jul 27, 2016 · 1 comment · Fixed by #1469
Assignees

Comments

@zub2
Copy link

zub2 commented Jul 27, 2016

The moveFocusHorizontal and moveFocusVertical in Keyboard mixin throw TypeError (or pass undefined to Grid.prototype.up/down/left/right which then throw TypeError) when the List or Grid contain fewer items than what would fill the whole height of the table.

If the List or Grid are completely empty (the collection they render contains 0 items), then pressing any arrow or pg up/down keys results in the TypeError for both List + Keyboard and Grid + Keyboard (both cellNavigation true and false).

If the List or Grid are not empty, but there is an unused space at the bottom (e.g. the list contains only 1 item, but the height is so that there is unused space below the item), then the TypeError is thrown only for Grid + Keyboard case, with cellNavigation true. This happens when user clicks in the empty area and then presses any arrow or pg up/down. For cellNavigation false, the arrows or pg up/down just bring the focus back to one of the existing elements.

I'm seeing this issue with DGrid 1.1.0 with Firefox 45.2.0 and Chrome 52.0.2743.82.

To reproduce with the official tests:

Issue with empty grid/list:

  1. edit test/data/genericData.js: change line 20 to: var rows = 0;
  2. display test/Keyboard.html in browser
  3. click in one of the three empty grid displayed to focus them
  4. press one of arrow left/right/up/down or page up/down
  5. look into browser JS log - there are TypeErrors logged

Issue with few items:

  1. edit test/data/genericData.js: change line 20 to: var rows = 3;
  2. display test/Keyboard.html in browser
  3. click in the empty space in the middle grid (Grid + Keyboard + cellNavigation true) to focus the grid
  4. press one of arrow left/right/up/down or page up/down
  5. look into browser JS log - there are TypeErrors logged
@zub2
Copy link
Author

zub2 commented Jul 27, 2016

Clarification: It seems that left/right arrows only cause an exception for Grid + Keyboard + cellNavigation true + tabableHeader: false.

This can be reproduced by editing the middle grid in test/Keyboard.html, e.g.:

                    grid = window.grid = new KeyboardGrid({
+                       tabableHeader: false,
                        collection: testStore,
                        columns: columns
                    }, "grid"),

For tabableHeader: true, left/right arrow just focus the header w/o any error. And left/right arrows for List + Keyboard and Grid + Keyboard + cellNavigation false do nothing, and don't trigger an error.

msssk added a commit to msssk/dgrid that referenced this issue May 20, 2020
In a grid with few enough rows that there is empty space below them the user
can click below the rows causing the grid's contentNode to receive focus.
If the arrow keys are then pressed to attempt cell/row navigation an error
is thrown. This change checks for and ignores selection of the contentNode.

It specifically checks for contentNode instead of the more resilient/generic
"not a row or cell" because that condition is more difficult to determine.
grid.row() returns undefined for grid.contentNode, but also for any node
within the header. Header navigation logic depends on this.

Fixes SitePen#1297
msssk added a commit that referenced this issue May 22, 2020
In a grid with few enough rows that there is empty space below them the user
can click below the rows causing the grid's contentNode to receive focus.
If the arrow keys are then pressed to attempt cell/row navigation an error
is thrown. This change checks for and ignores selection of the contentNode.

It specifically checks for contentNode instead of the more resilient/generic
"not a row or cell" because that condition is more difficult to determine.
grid.row() returns undefined for grid.contentNode, but also for any node
within the header. Header navigation logic depends on this.

Fixes #1297
msssk added a commit that referenced this issue Jun 19, 2020
In a grid with few enough rows that there is empty space below them the user
can click below the rows causing the grid's contentNode to receive focus.
If the arrow keys are then pressed to attempt cell/row navigation an error
is thrown. This change checks for and ignores selection of the contentNode.

It specifically checks for contentNode instead of the more resilient/generic
"not a row or cell" because that condition is more difficult to determine.
grid.row() returns undefined for grid.contentNode, but also for any node
within the header. Header navigation logic depends on this.

Fixes #1297
msssk added a commit that referenced this issue Jun 19, 2020
In a grid with few enough rows that there is empty space below them the user
can click below the rows causing the grid's contentNode to receive focus.
If the arrow keys are then pressed to attempt cell/row navigation an error
is thrown. This change checks for and ignores selection of the contentNode.

It specifically checks for contentNode instead of the more resilient/generic
"not a row or cell" because that condition is more difficult to determine.
grid.row() returns undefined for grid.contentNode, but also for any node
within the header. Header navigation logic depends on this.

Fixes #1297
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

Successfully merging a pull request may close this issue.

2 participants