client: Fix keyboard navigation with scroll_to_article_header=0

[PhrozenByte]

Keyboard navigation is virtually impossible with scroll_to_article_header=0, because buttons like Space also move the view point. We could also prevent the browser from moving the view point, but I believe that visually scrolling to the selected entry always is the expected behaviour when using keyboard navigation.

[netlify]

✅ Deploy Preview for selfoss canceled.

Name	Link
🔨 Latest commit	0e59b31
🔍 Latest deploy log	https://app.netlify.com/sites/selfoss/deploys/656354b30c9430000808274b

[jtojnar]

Keyboard navigation is virtually impossible with scroll_to_article_header=0, because buttons like Space also move the view point.

I cannot reproduce this in either Firefox or Chromium – we are already preventing the default browser behaviour:

selfoss/client/js/shortcuts.js

Line 31 in e0c9805

event.preventDefault();

The only time the space should move the viewport is when the next item does not fit into the viewport:

selfoss/client/js/templates/EntriesPage.jsx

Line 1001 in e0c9805

autoScroll(currentElement);

selfoss/client/js/helpers/navigation.js

Line 18 in e0c9805

if (viewportScrollTop + viewportHeight < targetTop + targetHeight + 80) {

but I believe that visually scrolling to the selected entry always is the expected behaviour when using keyboard navigation.

Hmm, I guess that depends on how you use selfoss. Personally, I read multiple item titles in a row and only open those that I find interesting – when it opens in place, I can just continue reading on the next line. Of course there will still be jump when near the end but that is inevitable.

Also, as is the PR makes it scroll to article header even when clicking a title, not just for keyboard navigation.

[PhrozenByte]

Something weird is going on here 🙈 I definitely once had the issue that Space was moving the viewport, but I no longer can reproduce that. Maybe it was just some intermediate issue while developing my other PRs.

However, I did write this fix after all my other PRs, thus there definitely still is a similar issue with moving the viewport - and after some research I managed to reproduce it. Due to your explanations (thanks! 👍) I now also understand what the issue is: It's not that Space moves the viewport, but there's an issue with autoScroll(), so I might just have mixed these up.

Assume you have four entries: The 1st, 2nd and 4th all consist of just a single line of content. The 3rd in contrast is huge, longer than the screen. You hit Space and the first entry opens, easily fitting into the screen. You hit Space again, collapsing the first and opening the second entry; both still easily fitting into the screen. You hit Space again, the second entry collapses and the third entry opens, now overflowing the screen. Hitting Space again sends your viewport way down, because it calculates the target viewport based on this huge 3rd entry that wasn't collapsed yet, but which is about to collapse.

In native JavaScript we just have to wait for another animation frame, i.e. wrap this

selfoss/client/js/templates/EntriesPage.jsx

Lines 998 to 1004 in e0c9805

	const currentElement = document.querySelector(`.entry[data-entry-id="${current}"]`);
	// scroll to element
	autoScroll(currentElement);
	// focus the title link for better keyboard navigation
	currentElement.querySelector('.entry-title-link').focus();

into window.requestAnimationFrame(() => { /* */ });. I just commited that (see force-push of 0e59b31) - and it works now.

However, I still know next to nothing about React, so I might rather use some feature provided by React instead, I just don't know which. Just let me know, I'll change it accordingly. 👍

Also, as is the PR makes it scroll to article header even when clicking a title, not just for keyboard navigation.

This happens when one writes patches in a hurry and doesn't properly test it 😒 Sorry about that.