Topic List Keyboard shortcut Previous(K) does not scroll properly leaving selected item hidden (Firefox)


(lid) #1

Replication

  1. open a category with a topic list view
  2. use keyboard navigation to go to the last item and trigger load more 2-3 times.
  3. now try to go up the list using the previous item keyboard binding.

###Result
The item selected might be out of the screen, when getting to the items at the top the scroll might fix it self.

The issue does not seem to appear on next(J) only on previous (K)

Tested on
Firefox 32 / Windows 8


(TechnoBear) #2

Confirmed - Firefox 32 on Ubuntu.


(cpradio) #3

Okay, so again, I’m lost. I never have a hidden result or maybe I’m not following terminology. When it runs Load More, you’ll see an item quickly blink blue, that is the topic that will be auto selected by j/k if none have been selected yet (happens in Chrome, Firefox, everywhere).

If you scroll down far enough to where that blue highlighted link is no longer visible and it doesn’t load more, then yes, where it starts selection will be off the screen, but it seems by design and pressing j/k puts the focus on the blue highlighted link, bringing it back to view.

So what am I missing from the repro steps (as my version does affect j and k)


(TechnoBear) #4

It seems to be a Firefox thing; Chromium behaves differently.

Go to a category list - e.g. https://meta.discourse.org/category/bug

Use j to move down through the list, and allow it to load more a couple of times. All is well. Use k to move back up. When you reach the top of the screen, k will continue to move up, but will not scroll into view. Pressing j at this point reloads the screen with the focused topic at the bottom, and k will move up again until the top of the screen, when it again vanishes from sight.

On Chromium, the screen scrolls as k moves up, so it’s never lost from view, (but the scrolling is a bit jerky, and personally I prefer the Firefox version).


(cpradio) #5

Ah, that is what I did wrong, I didn’t use j to go through multiple pages. Going to re-test (I was using Firefox though :slight_smile: )


(cpradio) #6

I think it is related to this line:

$('html, body').scrollTop($body.scrollTop() + distToElement);

https://github.com/discourse/discourse/blob/master/app/assets/javascripts/discourse/lib/keyboard_shortcuts.js#L257

In another post, I responded that Firefox doesn’t work when using $(‘body’).scrollTop(), it has to be $(document).scrollTop(), which might fix this issue.

I don’t have time to do this today (due to traveling), but @Lid, if you have time to test it out, it may be a simple fix.


(cpradio) #7

I was right, it dealt with $body.scrollTop().

PR Sent:
https://github.com/discourse/discourse/pull/2802


(Jeff Atwood) #8