ngx-datatable: Scrolling up with the 'up' arrow key does not work.

I’m submitting a … (check one with “x”)

[x] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, post on Stackoverflow or Gitter

Current behavior

Create a ngx-datatable with the scrollbarV property, focus on the table and use the arrow keys to scroll down.
Try to scroll back up using the arrow keys and you can’t.

Expected behavior

Using the arrow keys should allow you to scroll back up the table.

Reproduction of the problem

Visit http://swimlane.github.io/ngx-datatable/#virtual-scroll focus on the table and use the arrow keys to scroll down.
Try to scroll back up using the arrow keys and you can’t.

What is the motivation / use case for changing the behavior?

Accessibility

Please tell us about your environment:

MacOS Sierra (Also tried on Ubuntu 14.04) Chrome

Table version: 9.3.0
Angular version: 4.1.3
Browser: Only tested in Chrome
Language: All

About this issue

Original URL
State: open
Created 7 years ago
Reactions: 2
Comments: 19 (9 by maintainers)

Most upvoted comments

@amcdnl I’m a one man shop here and rather new to opensource and typescript/angular in general. I’m not sure if I have a gitter account, but I will signup for one if I don’t. I worked real hard to not break anything with onpush. I do a lot of ngOnCheck and perform my own change detection. I forgot to mention, but I added a mode for change detection on the table (‘manual’ and ‘auto’), and it helps with the live demo. I set it to manual for better performance when I’m not dealing with live row data. I probably know your codebase really well now, along with parts of the angular 2/4 framework trying to fix major performance issues I was having the last 6 months.

My codebase is all broken at the moment, doing a lot of refactoring, but if you would like to check out an older commit, I can publish it, otherwise, I hope to be public in a week or so.

wizarrc on Aug 4, 2017

@amcdnl It’s rather big and it’s a decent breaking change at the API level. I forked it to add the following features (mostly for perf issues I was having):

Fixed a lot of re-rendering issues
Separated $$index, $$expanded from the rows source (created mappings)
Added special update zones for scroll and page events to prevent global change detection
Made everything OnPush change detection internally
Added types and generics and removed almost all any
Restructured the flow of properties to use services or separate classes instead of pass though the component @Input(). It was impossible to reason about how changing one input property would affect the rest of the library. Also, there was a back and forth flow of data from the body to the datatable, making some things highly difficult to test.
Virtualizing the scrollbar itself to fix the perceived click and drag scrolling performance that showed whitespace during moving the scroll bar on IE/Edge. Without virtualization, not allowing it to scroll till after it’s rendered (currently impossible since the scroll event occurs after scrolling but before it’s actually rendered, requiring it to play catch up with the virtual scroller).
Making the scrollbar virtual, allows me to add things like animated scrollbar (collapsing, shrinking, and removing the buttons when not in use like done on some scrollers).
I’m currently in the process of making the scroller generic so I can add horizontal virtual scrolling as well.
Allow using keyboard keys to scroll up and down with the virtual scroller by using a service instead of over estimating the bottom scroller and only by accident scrolling when the system scrolls after focus() is called, triggering updateIndexes(), etc.

These changes change how some of the demos work, especially when using functions that rely on $$index. I originally tried to make my changes in small bite size chunks, but some of the things I needed for my own project needed some fundamental changes, and while I was at it, I made some more bigger changes so that I could reason better about the code base. I figured the change is way too big to merge, but once I share my results, you are welcome to break it up into smaller pieces, if possible. I’m not really sure how that PR would go if I attempt it.

wizarrc on Aug 4, 2017