[FocusManager] More intuitive key navigation + spec (#3774)

FocusManager now finds the closest widget on the right or left on inner horizontal border.

See : https://github.com/koreader/koreader/pull/3765#issuecomment-373944897

frontend/ui/widget/focusmanager.lua

@@ -57,17 +57,20 @@ function FocusManager:onFocusMove(args)
     while true do
         if not self.layout[self.selected.y + dy] then
             --horizontal border, try to wraparound
-            if not self:wrapAround(dy) then
+            if not self:_wrapAround(dy) then
                 break
             end
+        elseif not self.layout[self.selected.y + dy][self.selected.x] then
+            --inner horizontal border, trying to be clever and step down
+            self:_verticalStep(dy)
         elseif not self.layout[self.selected.y + dy][self.selected.x + dx] then
             --vertical border, no wraparound
             break
         else
             self.selected.y = self.selected.y + dy
             self.selected.x = self.selected.x + dx
-            logger.dbg("Cursor position : ".. self.selected.y .." : "..self.selected.x)
         end
+        logger.dbg("Cursor position : ".. self.selected.y .." : "..self.selected.x)
 
         if self.layout[self.selected.y][self.selected.x] ~= current_item
         or not self.layout[self.selected.y][self.selected.x].is_inactive then
@@ -83,21 +86,42 @@ function FocusManager:onFocusMove(args)
     return true
 end
 
-function FocusManager:wrapAround(dy)
+function FocusManager:_wrapAround(dy)
     --go to the last valid item directly above or below the current item
     --return false if none could be found
     local y = self.selected.y
-    while self.layout[y - dy] and self.layout[y - dy][self.selected.x] do
+    while self.layout[y - dy] do
         y = y - dy
     end
     if y ~= self.selected.y then
         self.selected.y = y
+        if not self.layout[self.selected.y][self.selected.x] then
+            --call verticalStep on the current line to perform the search
+            self:_verticalStep(0)
+        end
         return true
     else
         return false
     end
 end
 
+function FocusManager:_verticalStep(dy)
+    local x = self.selected.x
+    --looking for the item on the line below, the closest on the left side
+    while not self.layout[self.selected.y + dy][x] do
+        x = x - 1
+        if x == 0 then
+            --if he is not on the left, must be on the right
+            x = self.selected.x
+            while not self.layout[self.selected.y + dy][x] do
+                x = x + 1
+            end
+        end
+    end
+    self.selected.x = x
+    self.selected.y = self.selected.y + dy
+end
+
 function FocusManager:getFocusItem()
     return self.layout[self.selected.y][self.selected.x]
 end

spec/unit/focusmanager_spec.lua

@@ -0,0 +1,91 @@
+describe("FocusManager module", function()
+    local FocusManager
+    local layout
+    local Up,Down,Left,Right
+    Up = function(self) self:onFocusMove({0, -1}) end
+    Down = function(self) self:onFocusMove({0, 1}) end
+    Left = function(self) self:onFocusMove({-1, 0}) end
+    Right = function(self) self:onFocusMove({1, 0}) end
+    setup(function()
+        require("commonrequire")
+        FocusManager = require("ui/widget/focusmanager")
+        local Widget = require("ui/widget/textwidget")
+        local w = Widget:new{}
+        layout= {
+                {w,  w,  w},
+                {nil,w,nil},
+                {nil,w,nil},
+                }
+
+    end)
+    it("should go right", function()
+        local focusmanager = FocusManager:new{}
+        focusmanager.layout = layout
+        focusmanager.selected = {y = 1,x = 1}
+        Right(focusmanager)
+        assert.are.same({y = 1,x = 2}, focusmanager.selected)
+    end)
+    it("should go left", function()
+        local focusmanager = FocusManager:new{}
+        focusmanager.layout = layout
+        focusmanager.selected = {y = 1,x = 2}
+        Left(focusmanager)
+        assert.are.same({y = 1,x = 1}, focusmanager.selected)
+    end)
+    it("should go up", function()
+        local focusmanager = FocusManager:new{}
+        focusmanager.layout = layout
+        focusmanager.selected = {y = 2,x = 2}
+        Up(focusmanager)
+        assert.are.same({y = 1,x = 2}, focusmanager.selected)
+    end)
+    it("should go down", function()
+        local focusmanager = FocusManager:new{}
+        focusmanager.layout = layout
+        focusmanager.selected = {y = 2,x = 2}
+        Down(focusmanager)
+        assert.are.same({y = 3,x = 2}, focusmanager.selected)
+    end)
+    it("should vertical wrapAround up", function()
+        local focusmanager = FocusManager:new{}
+        focusmanager.layout = layout
+        focusmanager.selected = {y = 1,x = 1}
+        Up(focusmanager)
+        assert.are.same({y = 3,x = 2}, focusmanager.selected)
+    end)
+    it("should vertical wrapAround down", function()
+        local focusmanager = FocusManager:new{}
+        focusmanager.layout = layout
+        focusmanager.selected = {y = 3,x = 2}
+        Down(focusmanager)
+        assert.are.same({y = 1,x = 2}, focusmanager.selected)
+    end)
+    it("should do vertical step to the right", function()
+        local focusmanager = FocusManager:new{}
+        focusmanager.layout = layout
+        focusmanager.selected = {y = 1,x = 1}
+        Down(focusmanager)
+        assert.are.same({y = 2,x = 2}, focusmanager.selected)
+    end)
+    it("should do vertical step to the left", function()
+        local focusmanager = FocusManager:new{}
+        focusmanager.layout = layout
+        focusmanager.selected = {y = 1,x = 3}
+        Down(focusmanager)
+        assert.are.same({y = 2,x = 2}, focusmanager.selected)
+    end)
+    it("should respect left limit", function()
+        local focusmanager = FocusManager:new{}
+        focusmanager.layout = layout
+        focusmanager.selected = {y = 2,x = 2}
+        Left(focusmanager)
+        assert.are.same({y = 2,x = 2}, focusmanager.selected)
+    end)
+    it("should respect right limit", function()
+        local focusmanager = FocusManager:new{}
+        focusmanager.layout = layout
+        focusmanager.selected = {y = 2,x = 2}
+        Right(focusmanager)
+        assert.are.same({y = 2,x = 2}, focusmanager.selected)
+    end)
+end)