Fix various keyboard navigation issues

Signed-off-by: Rhys Tyers <mail@rhy.si>

4 files changed, 45 insertions, 30 deletions

diff --git a/js/app/Config.js b/js/app/Config.js
index c22d7e9a5..441c6e54d 100644
--- a/js/app/Config.js
+++ b/js/app/Config.js
@@ -34,6 +34,10 @@ app.config(function ($routeProvider, $provide, $httpProvider, $locationProvider)
     $provide.constant('MARK_READ_TIMEOUT', 0.5);
     $provide.constant('SCROLL_TIMEOUT', 0.1);
 
+    const majorVersion = parseInt($('#app-content').data('nc-major-version') || 0, 10);
+    $provide.constant('NC_MAJOR_VERSION', majorVersion);
+    window.NEWS_NC_MAJOR_VERSION = majorVersion;
+
     // make sure that the CSRF header is only sent to the Nextcloud domain
     $provide.factory('CSRFInterceptor', function ($q, BASE_URL, $window) {
         return {
diff --git a/js/directive/NewsScroll.js b/js/directive/NewsScroll.js
index afbd3b703..a5e620b80 100644
--- a/js/directive/NewsScroll.js
+++ b/js/directive/NewsScroll.js
@@ -8,21 +8,17 @@
  * @copyright Bernhard Posselt 2014
  */
 app.directive('newsScroll', function ($timeout, ITEM_AUTO_PAGE_SIZE,
-    MARK_READ_TIMEOUT, SCROLL_TIMEOUT) {
+    MARK_READ_TIMEOUT, SCROLL_TIMEOUT, NC_MAJOR_VERSION) {
     'use strict';
     var timer;
 
 
     var scrollElement = function() {
-        const appContentElem = $('#app-content');
-        const majorVersion = parseInt($('#app-content').data('nc-major-version') || 0, 10);
-        if (majorVersion >= 25) {
-            return appContentElem;
+        // This should be in sync with the same function in js/gui/KeyboardShortcuts.js
+        if (NC_MAJOR_VERSION >= 25) {
+            return  $('#app-content');
         }
-        if (majorVersion === 24) {
-            return $(window);
-        }
-        return $('html');
+        return $(window);
     };
 
     // autopaging
diff --git a/js/gui/KeyboardShortcuts.js b/js/gui/KeyboardShortcuts.js
index b6310fef5..9ddcb582d 100644
--- a/js/gui/KeyboardShortcuts.js
+++ b/js/gui/KeyboardShortcuts.js
@@ -16,6 +16,14 @@
 (function (window, document, $) {
     'use strict';
 
+    var scrollElement = function() {
+        // This should be in sync with the same function in js/directive/NewsScroll.js
+        if (window.NEWS_NC_MAJOR_VERSION >= 25) {
+            return  $('#app-content');
+        }
+        return $(window);
+    };
+
     var noInputFocused = function (element) {
         return !(
             element.is('input') ||
@@ -223,34 +231,34 @@
         }
     };
 
-    var getActiveElement = function (scrollArea) {
-        return scrollArea.find('.item.active:first');
+    var getActiveElement = function () {
+        return $('#app-content').find('.item.active:first');
     };
 
-    var onActiveItem = function (scrollArea, callback) {
-        callback(getActiveElement(scrollArea));
+    var onActiveItem = function (callback) {
+        callback(getActiveElement());
     };
 
-    var toggleUnread = function (scrollArea) {
-        onActiveItem(scrollArea, function (item) {
+    var toggleUnread = function () {
+        onActiveItem(function (item) {
             item.find('.toggle-keep-unread').trigger('click');
         });
     };
 
-    var toggleStar = function (scrollArea) {
-        onActiveItem(scrollArea, function (item) {
+    var toggleStar = function () {
+        onActiveItem(function (item) {
             item.find('.star').trigger('click');
         });
     };
 
-    var expandItem = function (scrollArea) {
-        onActiveItem(scrollArea, function (item) {
+    var expandItem = function () {
+        onActiveItem(function (item) {
             item.find('.utils').trigger('click');
         });
     };
 
-    var openLink = function (scrollArea) {
-        onActiveItem(scrollArea, function (item) {
+    var openLink = function () {
+        onActiveItem(function (item) {
             item.trigger('click');  // mark read
             var url = item.find('.external:visible').attr('href');
             var newWindow = window.open(url, '_blank');
@@ -265,9 +273,14 @@
     var scrollToItem = function (scrollArea, item, expandItemInCompact) {
         // if you go to the next article in compact view, it should
         // expand the current one
-        scrollArea.scrollTop(
-            item.offset().top - 50
-        );
+
+        if (window.NEWS_NC_MAJOR_VERSION >= 25) {
+            scrollArea.scrollTop(scrollArea.scrollTop() + item.offset().top - 50);
+        } else {
+            scrollArea.scrollTop(
+                item.offset().top - 50
+            );
+        }
 
         setItemActive(item[0]);
 
@@ -279,7 +292,7 @@
     };
 
     var scrollToNextItem = function (scrollArea, expandItemInCompact) {
-        var activeElement = getActiveElement(scrollArea);
+        var activeElement = getActiveElement();
         // in expand in compact mode, jumping to the next item should open
         // the current one if it's not open yet
         if (expandItemInCompact && !activeElement.hasClass('open')) {
@@ -300,7 +313,7 @@
 
     var scrollToPreviousItem = function (scrollArea,
                                          expandItemInCompact) {
-        var activeElement = getActiveElement(scrollArea);
+        var activeElement = getActiveElement();
         var previousElement = activeElement.prev();
 
         // if the active element has been scrolled, the previous element
@@ -321,18 +334,19 @@
             items.each(function (index, item) {
                 var $item = $(item);
                 var bottom = $item.position().top + $item.outerHeight(true);
-                if ((bottom - 20) >= 0) {
+                var scrollBottom =  scrollElement().scrollTop();
+                if (bottom - 20 >= scrollBottom) {
                     setItemActive(item);
-                    return false;
+                    return false;                        
                 }
             });
         };
-        $('#app-content').scroll(_.debounce(detectAndSetActiveItem, 250));
+        scrollElement().scroll(_.debounce(detectAndSetActiveItem, 250));
     });
 
     $(document).keyup(function (event) {
         var keyCode = event.keyCode;
-        var scrollArea = $(document);
+        var scrollArea = scrollElement();
         var navigationArea = $('#app-navigation');
         var isCompactView = $('#articles.compact').length > 0;
         var isExpandItem = $('#articles')
diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php
index 9f17f5690..abdd3f2f7 100644
--- a/lib/Controller/PageController.php
+++ b/lib/Controller/PageController.php
@@ -86,6 +86,7 @@ class PageController extends Controller
             $this->appName,
             'index',
             [
+                'nc_major_version' => \OCP\Util::getVersion()[0],
                 'warnings' => $status['warnings'],
                 'url_generator' => $this->urlGenerator
             ]