From b1b7773237554e0690fa483969165950c0f1322c Mon Sep 17 00:00:00 2001 From: poire-z Date: Sun, 18 Dec 2022 01:25:41 +0100 Subject: [PATCH] TouchMenu: tweak menu search (#9926) - Cleanup search and animation codes, fix inconsistencies between animation/no-animation opening, and refreshes glitches on eInk. - Show menu item on tap, with buttons to either open directly, or to walk there (removed earlier "Animation" checkbox, so the choice can be decided later). - Move event handlers into ReaderMenu/FileManagerMenu. - Avoid duplicated and confusing results from gestures and font-family submenus. --- frontend/apps/filemanager/filemanager.lua | 7 - frontend/apps/filemanager/filemanagermenu.lua | 5 + frontend/apps/reader/modules/readerfont.lua | 1 + frontend/apps/reader/modules/readermenu.lua | 5 + frontend/apps/reader/readerui.lua | 7 - frontend/dispatcher.lua | 1 + frontend/ui/widget/touchmenu.lua | 416 +++++++++--------- plugins/gestures.koplugin/main.lua | 2 + 8 files changed, 217 insertions(+), 227 deletions(-) diff --git a/frontend/apps/filemanager/filemanager.lua b/frontend/apps/filemanager/filemanager.lua index f134b5bc6..d977543bd 100644 --- a/frontend/apps/filemanager/filemanager.lua +++ b/frontend/apps/filemanager/filemanager.lua @@ -1385,11 +1385,4 @@ function FileManager:onRefreshContent() self:onRefresh() end -function FileManager:onMenuSearch() - if not self.ui then - self.menu:onShowMenu() - end - self.menu.menu_container[1]:onShowMenuSearch() -end - return FileManager diff --git a/frontend/apps/filemanager/filemanagermenu.lua b/frontend/apps/filemanager/filemanagermenu.lua index b806c763c..134fa87d8 100644 --- a/frontend/apps/filemanager/filemanagermenu.lua +++ b/frontend/apps/filemanager/filemanagermenu.lua @@ -904,6 +904,11 @@ function FileManagerMenu:onSetDimensions(dimen) end end +function FileManagerMenu:onMenuSearch() + self:onShowMenu() + self.menu_container[1]:onShowMenuSearch() +end + function FileManagerMenu:registerToMainMenu(widget) table.insert(self.registered_widgets, widget) end diff --git a/frontend/apps/reader/modules/readerfont.lua b/frontend/apps/reader/modules/readerfont.lua index de00fefd5..29e2b0c08 100644 --- a/frontend/apps/reader/modules/readerfont.lua +++ b/frontend/apps/reader/modules/readerfont.lua @@ -593,6 +593,7 @@ Enabling this will ignore such font names and make sure your preferred family fo self:updateFontFamilyFonts() end, sub_item_table = { + ignored_by_menu_search = true, -- those would be duplicated { text = T(_("Font for %1"), BD.wrap(T("'font-family: %1'", family_tag))), separator = true, diff --git a/frontend/apps/reader/modules/readermenu.lua b/frontend/apps/reader/modules/readermenu.lua index 709a5185f..db8d43fcd 100644 --- a/frontend/apps/reader/modules/readermenu.lua +++ b/frontend/apps/reader/modules/readermenu.lua @@ -538,6 +538,11 @@ function ReaderMenu:onSaveSettings() self.ui.doc_settings:saveSetting("readermenu_tab_index", self.last_tab_index) end +function ReaderMenu:onMenuSearch() + self:onShowMenu() + self.menu_container[1]:onShowMenuSearch() +end + function ReaderMenu:registerToMainMenu(widget) table.insert(self.registered_widgets, widget) end diff --git a/frontend/apps/reader/readerui.lua b/frontend/apps/reader/readerui.lua index 358cee775..9f0f6d819 100644 --- a/frontend/apps/reader/readerui.lua +++ b/frontend/apps/reader/readerui.lua @@ -871,11 +871,4 @@ function ReaderUI:getCurrentPage() end end -function ReaderUI:onMenuSearch() - if not self.ui then - self.menu:onShowMenu() - end - self.menu.menu_container[1]:onShowMenuSearch() -end - return ReaderUI diff --git a/frontend/dispatcher.lua b/frontend/dispatcher.lua index d2f7a5970..b2c14c271 100644 --- a/frontend/dispatcher.lua +++ b/frontend/dispatcher.lua @@ -830,6 +830,7 @@ example usage: --]]-- function Dispatcher:addSubMenu(caller, menu, location, settings) Dispatcher:init() + menu.ignored_by_menu_search = true -- all those would be duplicated table.insert(menu, { text = _("Nothing"), separator = true, diff --git a/frontend/ui/widget/touchmenu.lua b/frontend/ui/widget/touchmenu.lua index 641ced362..1dcde9e90 100644 --- a/frontend/ui/widget/touchmenu.lua +++ b/frontend/ui/widget/touchmenu.lua @@ -33,6 +33,7 @@ local datetime = require("datetime") local getMenuText = require("ui/widget/menu").getMenuText local _ = require("gettext") local ffiUtil = require("ffi/util") +local util = require("util") local T = ffiUtil.template local Input = Device.input local Screen = Device.screen @@ -479,7 +480,6 @@ function TouchMenu:init() end self.layout = {} - self.search_layout = {} self.ges_events.TapCloseAllMenus = { GestureRange:new{ @@ -675,10 +675,8 @@ function TouchMenu:updateItems() self:_recalculatePageLayout() self.item_group:clear() self.layout = {} - self.search_layout = {} table.insert(self.item_group, self.bar) table.insert(self.layout, self.bar.icon_widgets) -- for the focusmanager - table.insert(self.search_layout, self.bar.icon_widgets) -- for the menu search for c = 1, self.perpage do -- calculate index in item_table @@ -686,7 +684,6 @@ function TouchMenu:updateItems() if i <= #self.item_table then local item = self.item_table[i] local item_tmp = TouchMenuItem:new{ - name = "touch_menu_item " .. tostring(item.text) .. " xxx", -- xxx testing only, squish later item = item, menu = self, dimen = Geom:new{ @@ -694,12 +691,12 @@ function TouchMenu:updateItems() h = self.item_height, }, show_parent = self.show_parent, + item_visible_index = c, } table.insert(self.item_group, item_tmp) if item_tmp:isEnabled() then table.insert(self.layout, {[self.cur_tab] = item_tmp}) -- for the focusmanager end - table.insert(self.search_layout, {[self.cur_tab] = item_tmp}) -- for the menu search if item.separator and c ~= self.perpage and i ~= #self.item_table then -- insert split line table.insert(self.item_group, self.split_line) @@ -992,236 +989,233 @@ function TouchMenu:onBack() self:backToUpperMenu() end ------- the menu search functionality +-- Menu search feature function TouchMenu:search(search_for) local found_menu_items = {} - local MAX_MENU_DEPTH = 20 -- currently our menu needs at least 12 here - local function recurse(val, path, text, icon, depth) + local MAX_MENU_DEPTH = 10 -- our menu max depth is currently 6 + local function recurse(item_table, path, text, icon, depth) + if item_table.ignored_by_menu_search then + return + end depth = depth + 1 if depth > MAX_MENU_DEPTH then return end - - for i,v in ipairs(val) do - if type(v) == "table" then + for i, v in ipairs(item_table) do + if type(v) == "table" and not v.ignored_by_menu_search then local entry_text = v.text_func and v.text_func() or v.text local indent = text and ((" "):rep(math.min(depth-1, 6)) .. "→ ") or "→ " -- all spaces here are Hair Space U+200A - local next_text = text and (text .. "\n" .. indent .. entry_text) or (indent .. entry_text) - local next_path = path .. "." .. i - recurse(val[i], next_path, next_text, icon, depth) + local walk_text = text and (text .. "\n" .. indent .. entry_text) or (indent .. entry_text) + local walk_path = path .. "." .. i if Utf8Proc.lowercase(entry_text):find(search_for, 1, true) then - table.insert(found_menu_items, {entry_text, icon, next_path, next_text}) + table.insert(found_menu_items, {entry_text, icon, walk_path, walk_text}) + end + local sub_item_table = v.sub_item_table + if v.sub_item_table_func then + sub_item_table = v.sub_item_table_func() + end + if sub_item_table and not sub_item_table.ignored_by_menu_search then + recurse(sub_item_table, walk_path, walk_text, icon, depth) end end end - - if val.sub_item_table_func then - local sub_item_table = val.sub_item_table_func() - local perpage = "" - if sub_item_table.max_per_page then - perpage = "[" .. sub_item_table.max_per_page .."]" - end - recurse(sub_item_table, path .. ".sub_item_table_func" .. perpage, text, icon, depth) - elseif val.sub_item_table then - local perpage = "" - if val.sub_item_table.max_per_page then - perpage = "[" .. val.sub_item_table.max_per_page .."]" - end - recurse(val.sub_item_table, path .. ".sub_item_table" .. perpage, text, icon, depth) - end end -- recurse - -- initial call of recurse + -- Initial call of recurse, for each tab for i = 1, #self.tab_item_table do - recurse(self.tab_item_table[i], i, self.tab_item_table[i].text, self.tab_item_table[i].icon, 0) + recurse(self.tab_item_table[i], i, nil, self.tab_item_table[i].icon, 0) end ---[[ - for i = 1, #found_menu_items do - print("xxxxxxx->", i, - found_menu_items[i][1], - found_menu_items[i][2], - found_menu_items[i][3]) - end -]] return found_menu_items end --- maybe this could be placed in UIManager? -local function highlightWidget(widget, unhilight_in_s) - if not widget then return end - local highlight_dimen = widget.dimen - if highlight_dimen.w == 0 then - highlight_dimen.w = widget.width +function TouchMenu:openMenu(path, with_animation) + local parts = {} + for part in util.gsplit(path, "%.", false) do -- path is ie. "2.3.3.1" + table.insert(parts, tonumber(part)) end + util.arrayReverse(parts) -- so we can just table.remove() and pop them from end - UIManager:nextTick(function() - -- Highlight - widget.invert = true - UIManager:widgetInvert(widget, highlight_dimen.x, highlight_dimen.y, highlight_dimen.w) - UIManager:setDirty(nil, "fast", highlight_dimen) - - UIManager:forceRePaint() - UIManager:yieldToEPDC() - end) - - if unhilight_in_s then - -- Unhighlight - UIManager:scheduleIn(unhilight_in_s, function() + local function highlightWidget(widget, unhighlight) + if not widget then return end + local highlight_dimen = widget.dimen + if highlight_dimen.w == 0 then + highlight_dimen.w = widget.width + end + if unhighlight then widget.invert = false UIManager:widgetInvert(widget, highlight_dimen.x, highlight_dimen.y, highlight_dimen.w) UIManager:setDirty(nil, "ui", highlight_dimen) - end) - end -end - -function TouchMenu:_get_widget(tab_nb, nb) - return self.search_layout[nb + 1][tab_nb] -end - -function TouchMenu:openMenu(path) - local TrapWidget = require("ui/widget/trapwidget") - - local animation_time_s = G_reader_settings:readSetting("menu_search_animation_time_s", 1.0) - - -- first switch to correct MenuTab - local sep_pos = path:find("%.") - local tab_nb = tonumber(path:sub(1, sep_pos - 1)) - - if not tab_nb then return end - - path = path:sub(sep_pos + 1) - local item = self.tab_item_table[tab_nb] - - self:switchMenuTab(tab_nb) - self.bar:switchToTab(tab_nb) - self:onMenuSelect(self.item_table) - - -- Now go the menu path down the way - local items_to_show = {} - local dummy, num_of_sep = path:gsub("%.", "%.") - while num_of_sep > 1 do - sep_pos = path:find("%.") - local identifier = path:sub(1, sep_pos -1) - local item_nb = tonumber(identifier) - path = path:sub(sep_pos + 1) - if item_nb then - self:updateItems() - item = item[item_nb] - table.insert(items_to_show, {item_nb, item}) - elseif identifier:find("sub_item_table_func") then - item = item.sub_item_table_func() - elseif identifier:find("sub_item_table") then - item = item.sub_item_table - end - num_of_sep = num_of_sep - 1 - end - - -- Now we are in the right menu, but maybe in the wrong page - sep_pos = path:find("%.") - if not sep_pos then - local logger = require("logger") - logger.err("TouchMenu: search; internal error") -- should not happen - return - end - local identifier = path:sub(sep_pos + 1) - local item_nb = tonumber(identifier) - - local perpage - if path:find("sub_item_table_func%[") then - perpage = path:sub(#("sub_item_table_func%["), sep_pos - 2) - elseif path:find("sub_item_table%[") then - perpage = path:sub(#("sub_item_table%["), sep_pos - 2) - end - perpage = tonumber(perpage) or self.perpage - - local function open_final_menu() - print("xxx", #items_to_show) - if #items_to_show > 0 then -- can be zero, if in the last menu - self:onMenuSelect(items_to_show[#items_to_show][2]) + else + widget.invert = true + UIManager:widgetInvert(widget, highlight_dimen.x, highlight_dimen.y, highlight_dimen.w) + UIManager:setDirty(nil, "fast", highlight_dimen) end - local page_nb = math.floor((item_nb - 1) / perpage) + 1 - self:onGotoPage(page_nb) end - if not (animation_time_s and animation_time_s > 0.0) then - open_final_menu() - else - self.trap_widget = TrapWidget:new{ - dismiss_callback = function() - animation_time_s = 0 - self.trap_widget = nil - end - } - UIManager:show(self.trap_widget) -- suppress taps during animaton - - -- Animation functions - local function open_next_page(pages_to_show, force_first_page) - if animation_time_s == 0.0 then - UIManager.close(self.trap_widget) - self.trap_widget = nil - open_final_menu() - -- end of the animation here! - return + -- Steps/state among consecutive calls to walkStep() + local STEPS = { + START = 0, + TARGET_TAB_HIGHLIGHT_ICON = 1, + TARGET_TAB_OPEN = 2, + TARGET_PAGE_OR_HIGHLIGHT_NEXT_PREV = 3, + TARGET_PAGE_OR_NAVIGATE_NEXT_PREV = 4, + MENU_ITEM_HIGHLIGHT = 5, -- intermediate or final menu item + MENU_ITEM_ENTER = 6, -- intermediate menu item only + DONE = 7, + } + local step = STEPS.START + local tab_nb + local item_nb + local walkStep_scheduled + local trap_widget + + local function walkStep() + walkStep_scheduled = false + -- Default delay if not overriden (-1 means no scheduleIn() so no refresh, 0 means nextTick) + local next_delay = with_animation and 1 or -1 + if step == STEPS.START then + -- Ensure some initial delay so search dialog and result list can be closed and refreshed + next_delay = with_animation and 1 or 0 + step = STEPS.TARGET_TAB_HIGHLIGHT_ICON + elseif step == STEPS.TARGET_TAB_HIGHLIGHT_ICON then + tab_nb = table.remove(parts) + if with_animation then + highlightWidget(self.bar.icon_widgets[tab_nb].image) end - print("xxx pages_to_show", pages_to_show, force_first_page) - if force_first_page then - self:onFirstPage() + step = STEPS.TARGET_TAB_OPEN + elseif step == STEPS.TARGET_TAB_OPEN then + -- The tab icon wouldn't be unhighligted by any other action. + -- Animation may have been cancelled, so unhighlight if it was. + if self.bar.icon_widgets[tab_nb].image.invert then + highlightWidget(self.bar.icon_widgets[tab_nb].image, true) end - - if pages_to_show == 1 then -- if we are on the last page - if self.page_num > 1 then - highlightWidget(self.page_info_right_chev, animation_time_s) + self:switchMenuTab(tab_nb) + self.bar:switchToTab(tab_nb) + item_nb = table.remove(parts) + step = STEPS.TARGET_PAGE_OR_HIGHLIGHT_NEXT_PREV + elseif step == STEPS.TARGET_PAGE_OR_HIGHLIGHT_NEXT_PREV or + step == STEPS.TARGET_PAGE_OR_NAVIGATE_NEXT_PREV then + local target_page = math.floor((item_nb - 1) / self.perpage) + 1 + local pages_diff = target_page - self.page + if pages_diff == 0 then -- we are on the right menu page + step = STEPS.MENU_ITEM_HIGHLIGHT + next_delay = -1 -- we paused before, no need for more pause + if not with_animation and #parts == 0 then + -- Except if no animation and we are on the final menu that + -- we want to highlight: this final highlight needs to be + -- delayed for it to be drawn after the final menu page is. + next_delay = 1 end - highlightWidget(self:_get_widget(tab_nb, (item_nb - 1) % perpage + 1)) - -- end of the animation here! - else - if self.page_num ~= 1 then - highlightWidget(self.page_info_right_chev) + elseif step == STEPS.TARGET_PAGE_OR_HIGHLIGHT_NEXT_PREV then + -- No need to highlight chevrons if no animation + if with_animation then + if pages_diff > 0 then + highlightWidget(self.page_info_right_chev) + else + highlightWidget(self.page_info_left_chev) + end + if pages_diff > 1 or pages_diff < -1 then + -- Change pages quicker if more than one needed, but slow on the last one + next_delay = 0.5 + end end - UIManager:scheduleIn(animation_time_s, function() + step = STEPS.TARGET_PAGE_OR_NAVIGATE_NEXT_PREV + else -- STEPS.TARGET_PAGE_OR_NAVIGATE_NEXT_PREV + if pages_diff > 0 then self:onNextPage() - if pages_to_show > 1 then - UIManager:nextTick(open_next_page, pages_to_show - 1, false) - end - end) + else + self:onPrevPage() + end + step = STEPS.TARGET_PAGE_OR_HIGHLIGHT_NEXT_PREV + if with_animation and (pages_diff > 1 or pages_diff < -1) then + -- Change pages quicker if more than one needed, but slow on the last one + next_delay = 0.5 + end end - end - - local function open_next_menu() - if animation_time_s == 0.0 then - UIManager:close(self.trap_widget) - self.trap_widget = nil - open_final_menu() - -- end of the animation here! - return + elseif step == STEPS.MENU_ITEM_HIGHLIGHT then + if with_animation or #parts == 0 then + -- Even if no animation, highlight the final item (and don't unhighlight it) + local item_visible_index = (item_nb - 1) % self.perpage + 1 + local item_widget + for i, w in ipairs(self.item_group) do + if w.item_visible_index == item_visible_index then + item_widget = w + break + end + end + if item_widget then + highlightWidget(item_widget) + end end - - local x = table.remove(items_to_show, 1) - local next_menu_num, next_menu_item = x and x[1], x and x[2] -- might be nil - - if next_menu_item then - highlightWidget(self:_get_widget(tab_nb, next_menu_num)) - UIManager:scheduleIn(animation_time_s, function() - self:onMenuSelect(next_menu_item) - UIManager:nextTick(open_next_menu) - end) + if #parts == 0 then + step = STEPS.DONE else - -- no items left, but maybe on another page - local page_nb = math.floor(item_nb / perpage) + 1 - UIManager:nextTick(open_next_page, page_nb, page_nb > 1) + step = STEPS.MENU_ITEM_ENTER + end + elseif step == STEPS.MENU_ITEM_ENTER then + self:onMenuSelect(self.item_table[item_nb]) + item_nb = table.remove(parts) + step = STEPS.TARGET_PAGE_OR_HIGHLIGHT_NEXT_PREV + else -- STEPS.DONE + if trap_widget then + UIManager:close(trap_widget) + trap_widget = nil end + return + end + if next_delay >= 0 then + walkStep_scheduled = true + UIManager:scheduleIn(next_delay, walkStep) + else + walkStep() end - - -- Animate - UIManager:nextTick(open_next_menu) end + + -- We use an invisible TrapWidget when no animation, so we can + -- cancel the delayed final highlight + local TrapWidget = require("ui/widget/trapwidget") + trap_widget = TrapWidget:new{ + text = with_animation and _("Walking you there…") or nil, + dismiss_callback = function() + trap_widget = nil + if walkStep_scheduled then + UIManager:unschedule(walkStep) + if with_animation then + -- continue walking as if no animation, so we immediately + -- reach the requested menu item. We need a new invisible + -- TrapWidget for the reason explained above in case a + -- second tap happens. + with_animation = false + trap_widget = TrapWidget:new{ + text = nil, + dismiss_callback = function() + trap_widget = nil + if walkStep_scheduled then + UIManager:unschedule(walkStep) + end + end, + resend_event = true, + } + UIManager:show(trap_widget) + walkStep() + end + end + end, + resend_event = not with_animation, -- if not animation, don't eat the tap + } + UIManager:show(trap_widget) -- catch taps during animaton + + -- Call it: it will reschedule itself if animation; if not, it will + -- just execute itself without pause until done. + -- If tap while animating, it will switch to the non-animation + -- behaviour, to reach the requested menu item immediately. + walkStep() end function TouchMenu:onShowMenuSearch() local InputDialog = require("ui/widget/inputdialog") - local CheckButton = require("ui/widget/checkbutton") local ConfirmBox = require("ui/widget/confirmbox") local Menu = require("ui/widget/menu") @@ -1229,23 +1223,31 @@ function TouchMenu:onShowMenuSearch() local found_menu_items = self:search(search_string) local function get_current_search_results() - local function item_callback(i) + local function open_menu(i, animate) UIManager:close(self.results_menu_container) UIManager:setDirty(nil, "ui") - self:openMenu(found_menu_items[i][3]) + self:openMenu(found_menu_items[i][3], animate) end - local function item_hold_callback(i) + local function item_callback(i) local confirm_box confirm_box = ConfirmBox:new{ - text = T(_("Open menu entry:\n'%1'\n\n%2"), found_menu_items[i][1], found_menu_items[i][4]), + text = found_menu_items[i][4], icon = found_menu_items[i][2], ok_text = _("Open"), ok_callback = function() UIManager:close(confirm_box) - item_callback(i) + open_menu(i) end, - cancel_text = _("Dismiss"), - width_percent = 0.95, + other_buttons = {{ + { + text = _("Walk me there"), + callback = function() + UIManager:close(confirm_box) + open_menu(i, true) + end, + }, + }}, + } UIManager:show(confirm_box) end @@ -1256,7 +1258,7 @@ function TouchMenu:onShowMenuSearch() { text = found_menu_items[i][1], callback = function() item_callback(i) end, - hold_callback = function() item_hold_callback(i) end, + hold_callback = function() open_menu(i) end, } ) end @@ -1266,8 +1268,7 @@ function TouchMenu:onShowMenuSearch() if #found_menu_items > 0 then local results_menu = Menu:new{ title = _("Search results"), - item_table = get_current_search_results(found_menu_items), - item_shortcuts = {}, + item_table = get_current_search_results(), width = math.floor(Screen:getWidth() * 0.9), height = math.floor(Screen:getHeight() * 0.9), single_line = true, @@ -1317,6 +1318,7 @@ function TouchMenu:onShowMenuSearch() }, { text = _("Search"), + is_enter_default = true, callback = function() local search_for = search_dialog:getInputText() search_for = Utf8Proc.lowercase(search_for) @@ -1329,18 +1331,6 @@ function TouchMenu:onShowMenuSearch() }, } - local animation_time_s = G_reader_settings:readSetting("menu_search_animation_time_s", 1.0) - local check_button_animation = CheckButton:new{ - text = _("Animation"), - checked = animation_time_s ~= 0.0, - parent = search_dialog, - callback = function() - animation_time_s = animation_time_s ~= 0.0 and 0.0 or 1.0 - G_reader_settings:saveSetting("menu_search_animation_time_s", animation_time_s) - end, - } - search_dialog:addWidget(check_button_animation) - UIManager:show(search_dialog) search_dialog:onShowKeyboard() end diff --git a/plugins/gestures.koplugin/main.lua b/plugins/gestures.koplugin/main.lua index 8ad8b61be..d36d966b1 100644 --- a/plugins/gestures.koplugin/main.lua +++ b/plugins/gestures.koplugin/main.lua @@ -259,6 +259,8 @@ function Gestures:genSubItem(ges, separator, hold_callback) separator = separator, hold_callback = hold_callback, menu_item_id = ges, + ignored_by_menu_search = true, -- This item is not strictly duplicated, but its subitems are. + -- Ignoring it speeds up search. } end