[UX] Add "Follow Link" in hold/highlight dialog (#3190)

* This way you can disable "tap to follow links" on single tap yet still follow links.
* Change menu as well as per @poire-z's suggestion.

frontend/apps/reader/modules/readerdictionary.lua

@@ -194,9 +194,9 @@ If you'd like to change the order in which dictionaries are queried (and their r
     }
 end
 
-function ReaderDictionary:onLookupWord(word, box, highlight)
+function ReaderDictionary:onLookupWord(word, box, highlight, link)
     self.highlight = highlight
-    self:stardictLookup(word, box)
+    self:stardictLookup(word, box, link)
     return true
 end
 
@@ -317,7 +317,7 @@ function ReaderDictionary:dismissLookupInfo()
     self.lookup_progress_msg = nil
 end
 
-function ReaderDictionary:stardictLookup(word, box)
+function ReaderDictionary:stardictLookup(word, box, link)
     logger.dbg("lookup word:", word, box)
     -- escape quotes and other funny characters in word
     word = self:cleanSelection(word)
@@ -402,10 +402,10 @@ function ReaderDictionary:stardictLookup(word, box)
             }
         }
     end
-    self:showDict(word, tidyMarkup(final_results), box)
+    self:showDict(word, tidyMarkup(final_results), box, link)
 end
 
-function ReaderDictionary:showDict(word, results, box)
+function ReaderDictionary:showDict(word, results, box, link)
     self:dismissLookupInfo()
     if results and results[1] then
         logger.dbg("showing quick lookup window", word, results)
@@ -416,6 +416,8 @@ function ReaderDictionary:showDict(word, results, box)
             dialog = self.dialog,
             -- original lookup word
             word = word,
+            -- selected link, if any
+            selected_link = link,
             results = results,
             dictionary = self.default_dictionary,
             width = Screen:getWidth() - Screen:scaleBySize(80),

frontend/apps/reader/modules/readerhighlight.lua

@@ -3,6 +3,7 @@ local ConfirmBox = require("ui/widget/confirmbox")
 local Device = require("device")
 local Event = require("ui/event")
 local InputContainer = require("ui/widget/container/inputcontainer")
+local ReaderLink = require("apps/reader/modules/readerlink")
 local UIManager = require("ui/uimanager")
 local logger = require("logger")
 local _ = require("gettext")
@@ -250,6 +251,14 @@ function ReaderHighlight:onHold(arg, ges)
     if ok and word then
         logger.dbg("selected word:", word)
         self.selected_word = word
+        ReaderLink.ui = self.ui
+        ReaderLink.view = self.view
+        local link = ReaderLink:getLinkFromGes(ges)
+        self.selected_link = nil
+        if link then
+            logger.dbg("link:", link)
+            self.selected_link = link
+        end
         if self.ui.document.info.has_pages then
             local boxes = {}
             table.insert(boxes, self.selected_word.sbox)
@@ -296,17 +305,17 @@ function ReaderHighlight:onHoldPan(_, ges)
     UIManager:setDirty(self.dialog, "ui")
 end
 
-function ReaderHighlight:lookup(selected_word)
+function ReaderHighlight:lookup(selected_word, selected_link)
     -- if we extracted text directly
     if selected_word.word then
         local word_box = self.view:pageToScreenTransform(self.hold_pos.page, selected_word.sbox)
-        self.ui:handleEvent(Event:new("LookupWord", selected_word.word, word_box, self))
+        self.ui:handleEvent(Event:new("LookupWord", selected_word.word, word_box, self, selected_link))
     -- or we will do OCR
     elseif selected_word.sbox and self.hold_pos then
         local word = self.ui.document:getOCRWord(self.hold_pos.page, selected_word)
         logger.dbg("OCRed word:", word)
         local word_box = self.view:pageToScreenTransform(self.hold_pos.page, selected_word.sbox)
-        self.ui:handleEvent(Event:new("LookupWord", word, word_box, self))
+        self.ui:handleEvent(Event:new("LookupWord", word, word_box, self, selected_link))
     end
 end
 
@@ -323,7 +332,7 @@ end
 
 function ReaderHighlight:onHoldRelease()
     if self.selected_word then
-        self:lookup(self.selected_word)
+        self:lookup(self.selected_word, self.selected_link)
         self.selected_word = nil
     elseif self.selected_text then
         logger.dbg("show highlight dialog")
@@ -346,6 +355,20 @@ function ReaderHighlight:onHoldRelease()
                         end,
                     },
                 },
+                {
+                    {
+                        text = "_",
+                        enabled = false,
+                    },
+                    {
+                        text = _("Translate"),
+                        enabled = false,
+                        callback = function()
+                            self:translate(self.selected_text)
+                            self:onClose()
+                        end,
+                    },
+                },
                 {
                     {
                         text = _("Wikipedia"),
@@ -357,27 +380,27 @@ function ReaderHighlight:onHoldRelease()
                         end,
                     },
                     {
-                        text = _("Translate"),
-                        enabled = false,
+                        text = _("Dictionary"),
                         callback = function()
-                            self:translate(self.selected_text)
+                            self:onHighlightDictLookup()
                             self:onClose()
                         end,
                     },
                 },
                 {
                     {
-                        text = _("Search"),
+                        text = _("Follow Link"),
+                        enabled = self.selected_link ~= nil,
                         callback = function()
-                            self:onHighlightSearch()
+                            ReaderLink:onGotoLink(self.selected_link)
                             UIManager:close(self.highlight_dialog)
                         end,
                     },
                     {
-                        text = _("Dictionary"),
+                        text = _("Search"),
                         callback = function()
-                            self:onHighlightDictLookup()
-                            self:onClose()
+                            self:onHighlightSearch()
+                            UIManager:close(self.highlight_dialog)
                         end,
                     },
                 },

frontend/apps/reader/modules/readerlink.lua

@@ -1,13 +1,17 @@
-local InputContainer = require("ui/widget/container/inputcontainer")
+--[[
+ReaderLink is an abstraction for document-specific link interfaces.
+]]--
+
+local Device = require("device")
+local Event = require("ui/event")
+local Geom = require("ui/geometry")
 local GestureRange = require("ui/gesturerange")
+local InputContainer = require("ui/widget/container/inputcontainer")
 local LinkBox = require("ui/widget/linkbox")
 local UIManager = require("ui/uimanager")
-local Geom = require("ui/geometry")
-local Screen = require("device").screen
-local Device = require("device")
 local logger = require("logger")
-local Event = require("ui/event")
 local _ = require("gettext")
+local Screen = Device.screen
 local T = require("ffi/util").template
 
 local ReaderLink = InputContainer:new{
@@ -55,8 +59,8 @@ function ReaderLink:initGesListener()
     end
 end
 
-local function isFollowLinksOn()
-    return G_reader_settings:readSetting("follow_links") ~= false
+local function isTapToFollowLinksOn()
+    return not G_reader_settings:isFalse("tap_to_follow_links")
 end
 
 local function isSwipeToGoBackEnabled()
@@ -70,22 +74,24 @@ end
 function ReaderLink:addToMainMenu(menu_items)
     -- insert table to main reader menu
     menu_items.follow_links = {
-        text = _("Follow links"),
+        text = _("Links"),
         sub_item_table = {
             {
-                text_func = function()
-                    return isFollowLinksOn() and _("Disable") or _("Enable")
-                end,
-                callback = function()
-                    G_reader_settings:saveSetting("follow_links",
-                        not isFollowLinksOn())
-                end
-            },
-            {
-                text = _("Go back"),
+                text = _("Go back to previous location"),
                 enabled_func = function() return #self.location_stack > 0 end,
                 callback = function() self:onGoBackLink() end,
+                separator = true,
+            },
+            {
+                text = _("Tap to follow links"),
+                checked_func = isTapToFollowLinksOn,
+                callback = function()
+                    G_reader_settings:saveSetting("tap_to_follow_links",
+                        not isTapToFollowLinksOn())
+                end,
+                separator = true,
             },
+
             {
                 text = _("Swipe to go back"),
                 checked_func = isSwipeToGoBackEnabled,
@@ -106,42 +112,64 @@ function ReaderLink:addToMainMenu(menu_items)
     }
 end
 
-function ReaderLink:onSetDimensions(dimen)
-    -- update listening according to new screen dimen
-    if Device:isTouchDevice() then
-        self:initGesListener()
-    end
-end
-
-function ReaderLink:onTap(_, ges)
-    if not isFollowLinksOn() then return end
+--- Gets link from gesture.
+-- `Document:getLinkFromPosition()` behaves differently depending on
+-- document type, so this function provides a wrapper.
+function ReaderLink:getLinkFromGes(ges)
     if self.ui.document.info.has_pages then
         local pos = self.view:screenToPageTransform(ges.pos)
         if pos then
             -- link box in native page
             local link, lbox = self.ui.document:getLinkFromPosition(pos.page, pos)
             if link and lbox then
-                -- screen box that holds the link
-                local sbox = self.view:pageToScreenTransform(pos.page,
-                    self.ui.document:nativeToPageRectTransform(pos.page, lbox))
-                if sbox then
-                    UIManager:show(LinkBox:new{
-                        box = sbox,
-                        timeout = FOLLOW_LINK_TIMEOUT,
-                        callback = function() self:onGotoLink(link) end
-                    })
-                    return true
-                end
+                return link, lbox, pos
             end
         end
     else
         local link = self.ui.document:getLinkFromPosition(ges.pos)
+        if link ~= "" then
+            return link
+        end
+    end
+end
+
+--- Highlights a linkbox if available and goes to it.
+function ReaderLink:showLinkBox(link, lbox, pos)
+    if link and lbox then
+        -- screen box that holds the link
+        local sbox = self.view:pageToScreenTransform(pos.page,
+            self.ui.document:nativeToPageRectTransform(pos.page, lbox))
+        if sbox then
+            UIManager:show(LinkBox:new{
+                box = sbox,
+                timeout = FOLLOW_LINK_TIMEOUT,
+                callback = function() self:onGotoLink(link) end
+            })
+            return true
+        end
+    else
         if link ~= "" then
             return self:onGotoLink(link)
         end
     end
 end
 
+function ReaderLink:onSetDimensions(dimen)
+    -- update listening according to new screen dimen
+    if Device:isTouchDevice() then
+        self:initGesListener()
+    end
+end
+
+function ReaderLink:onTap(_, ges)
+    if not isTapToFollowLinksOn() then return end
+    local link, lbox, pos = self:getLinkFromGes(ges)
+    if link then
+        self:showLinkBox(link, lbox, pos)
+    end
+end
+
+--- Goes to link.
 function ReaderLink:onGotoLink(link)
     logger.dbg("onGotoLink:", link)
     if self.ui.document.info.has_pages then
@@ -196,6 +224,7 @@ function ReaderLink:onGotoLink(link)
     return true
 end
 
+--- Goes back to previous location.
 function ReaderLink:onGoBackLink()
     local saved_location = table.remove(self.location_stack)
     if saved_location then
@@ -216,8 +245,8 @@ function ReaderLink:onSwipe(_, ges)
     end
 end
 
+--- Goes to first link on page.
 function ReaderLink:onGoToFirstLink(ges)
-    if not isFollowLinksOn() then return end
     local firstlink = nil
     if self.ui.document.info.has_pages then
         local pos = self.view:screenToPageTransform(ges.pos)

frontend/ui/widget/dictquicklookup.lua

@@ -14,6 +14,7 @@ local InputDialog = require("ui/widget/inputdialog")
 local LeftContainer = require("ui/widget/container/leftcontainer")
 local LineWidget = require("ui/widget/linewidget")
 local OverlapGroup = require("ui/widget/overlapgroup")
+local ReaderLink = require("apps/reader/modules/readerlink")
 local ScrollTextWidget = require("ui/widget/scrolltextwidget")
 local TextWidget = require("ui/widget/textwidget")
 local UIManager = require("ui/uimanager")
@@ -367,12 +368,17 @@ function DictQuickLookup:update()
                 {
                     -- if more than one language, enable it and display "current lang > next lang"
                     -- otherwise, just display current lang
-                    text = self.is_wiki and ( #self.wiki_languages > 1 and self.wiki_languages[1].." > "..self.wiki_languages[2] or self.wiki_languages[1] ) or "-",
-                    enabled = self.is_wiki and #self.wiki_languages > 1,
+                    text = self.is_wiki and ( #self.wiki_languages > 1 and self.wiki_languages[1].." > "..self.wiki_languages[2] or self.wiki_languages[1] ) or "Follow Link",
+                    enabled = (self.is_wiki and #self.wiki_languages > 1) or self.selected_link ~= nil,
                     callback = function()
-                        self:resyncWikiLanguages(true) -- rotate & resync them
-                        UIManager:close(self)
-                        self:lookupWikipedia()
+                        if self.is_wiki then
+                            self:resyncWikiLanguages(true) -- rotate & resync them
+                            UIManager:close(self)
+                            self:lookupWikipedia()
+                        else
+                            UIManager:close(self)
+                            ReaderLink:onGotoLink(self.selected_link)
+                        end
                     end,
                 },
                 {