Scrolling up with the 'up' arrow key does not work.

[carlcheel]

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

1. Create a ngx-datatable with the scrollbarV property, focus on the table and use the arrow keys to scroll down.
2. 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

1. Visit http://swimlane.github.io/ngx-datatable/#virtual-scroll focus on the table and use the arrow keys to scroll down.
2. 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

[kabb5]

Scrolling up and down with the arrow keys appears to be working for me in Chrome 59 and ngx-datatable version 9.3.1 (looking at http://swimlane.github.io/ngx-datatable/#virtual-scroll)

[carlcheel]

@kabb5 - Scroll down a few dozen rows first, then try to use the arrow keys to move up the table. Tried again in Chrome 59 - still experiencing the issue.

[wizarrc]

@carlcheel Seems to work for me on Windows 10 1703. Tried Chrome Version 59.0.3071.115 (Official Build) (64-bit), and also works on Edge Browser.

[wizarrc]

I wanted to note that it doesn't trigger the virtual scroll, so if a scrollbar is present, it will stop at the bottom of the view, even though more items are hidden below (not yet drawn). I think this should be corrected, and scroll beyond the bottom of the view.

[carlcheel]

@wizarrc - Sorry I should have been more specific in my description, I was referring to the virtual scroll not scrolling when using the arrow keys. However, for me the virtual scroll works when I'm moving down the table, just not up. Here's a gif to demonstrate, you'll notice it stops on 'Oconnor Wade' (i'm still pressing the up arrow key at this point).

[wizarrc]

@carlcheel I can't even get it to virtual scroll down past Santiago Mcclain, so my experience is worse than yours.

[wizarrc]

@carlcheel I figured out what the issue is and why you are having and why I wasn't able to replicate it. If you resize your window so the last visible item is near the bottom of the list, pressing keydown will not trigger the next items to be drawn, it's an accidental artifact of the system that it's even possible. The library calls focus() which triggers a scroll to bring it inside the view, thus triggering more items below it to be drawn (due to a bug, it adds items slightly below the view window) making it appear that you are actually scrolling using the arrow keys, but that bug only over draws on the bottom half, the top half does not have the bug. TLDR - An easy fix would be to create another bug on the top to over draw it by one row as well, but that's not ideal.

It will take a lot of work to enable this properly on the current version. I got it working on my forked copy but I made a significant amount of changes from this repo. I had to create a service that keeps track of what item is focused and scroll just enough for the next item to be drawn, then in a callback, after the scroll occurs, get the next item that was selected and focus it.

I will be sharing my fork shortly, so anyone can see how I made it work. My goal is to bring all my changes back if they will be accepted. I added quite a lot of changes that are major breaking changes, that are needed to support things like immutable data source, that is currently not possible with this library as it is embedded fields like $$index directly in the source data.

[carlcheel]

@wizarrc It would be great if you could share your fork with this change in so I could take a look too :+1:

[wizarrc]

@carlcheel I will upload to GitHub in a week or so hopefully, I don't have plans to make my own npm package however. Will that be good enough?

[carlcheel]

@wizarrc Yeah that would be great! Thank you.

[amcdnl]

@wizarrc - I would be happy to accept PRs!

[wizarrc]

@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):

1. Fixed a lot of re-rendering issues
2. Separated $$index, $$expanded from the rows source (created mappings)
3. Added special update zones for scroll and page events to prevent global change detection
4. Made everything OnPush change detection internally
5. Added types and generics and removed almost all any
6. 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.
7. 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).
8. 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).
9. I'm currently in the process of making the scroller generic so I can add horizontal virtual scrolling as well.
10. 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.

[amcdnl]

@wizarrc -

1. Love to have that!
2. I've broken these out in 10.0.0
3. Would love to have that.
4. I had that at one point and caused issues for those not using OnPush, if it works in that situation still, love to chat that.
5. AWESOME
6. Interesting
7. Interesting
8. I had considered using a faux scrollbar sooner
9. Ya, thats on my list
10. Interesting

Overally, I'd love to see all these changes make it in. Can we do them on a one-by-one basis? Are you on gitter, I'd love to chat with you more!

[wizarrc]

@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.

[ghost]

Any update on this bug?

[JojiePalahang]

If anyone still looking for the solution let me know - I have create a work around

[tbprojects]

@JojiePalahang can you share the workaround here?

[JojiePalahang]

@tbprojects in a case that you need to focus up/down. listen for keyup and keydown event on onActivate Method then manually focus scroll up/down the table snippet below setCurrentRowFocus(index) { const datatableBodyElement = this.dataTable.element.querySelector('datatable-body'); if (datatableBodyElement) { if (index < 5) { datatableBodyElement.scrollTop = 0; } else { datatableBodyElement.scrollTop = (index - 2) * this.rowHeight; } } }

[tbprojects]

@JojiePalahang thanks for the inspiration :) I've prepared monkey patch that wraps DataTableSelectionComponent.prototype.onActivate and scrolls datatable body by 1 pixel which is just enough to render another row with virtualization.

const originalOnActivate = DataTableSelectionComponent.prototype.onActivate;
DataTableSelectionComponent.prototype.onActivate = function(model, index) {
  originalOnActivate.call(this, model, index);
  if (model.event instanceof KeyboardEvent) {
    const elements = model.event.composedPath() as HTMLElement[];
    const datatableBody = elements.find((element: HTMLElement) => element.tagName === 'DATATABLE-BODY');
    if (model.event.key === 'ArrowUp') {
      datatableBody.scrollTop -= 1;
    } else if (model.event.key === 'ArrowDown') {
      datatableBody.scrollTop += 1;
    }
  }
};

[ingbioservicios]

I'm trying to scroll left and right on my table and it doesn't work only if I click or press the horizontal scroll bar