From 9cd305177e223853b9af1fe1ba4e325e2520a24e Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sun, 25 Aug 2024 19:34:31 +0200 Subject: [PATCH] FocusManager: Fix focus_flags check in moveFocusTo, and deal with the fallout (#12361) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * FocusManager: Fix `focus_flags` check in `moveFocusTo` (0 is truthy in Lua, can't do AND checks like in C ;).) * FileManager+FileChooser: Pass our custom title bar directly to FileChooser (which also means we can now use FC's FocusManager layout directly). * FileChooser/Menu: Get rid of the weird `outer_title_bar` hack, and simply take a `custom_title_bar` pointer to an actual TitleBar instance instead. * FileManager/Menu/ListMenu/CoverMenu: Fix content height computations in `_recalculateDimen` (all the non-FM cases were including an old and now unused padding value, `self.header_padding`, leading to more blank space at the bottom than necessary, and, worse, leading to different item heights between FM views, possibly leading to unnecessary thumbnail scaling !) * ButtonDialog: Proper focus management when the ButtonTable is wrapped in a ScrollableContainer. * ConfigDialog: Implement a stupid workaround for a weird FocusManager issue when going back from `[⋮]` buttons. * ConfigDialog: Don't move the visual focus in `update` (i.e., we use `NOT_FOCUS` now that it works as intended). * DictQuickLookup: Ensures the `Menu` key bind does the exact same thing as the hamburger icon. * DictQuickLookup: Ensure we refocus after having mangled the FocusManager layout (prevents an old focus highlight from lingering on the wrong button). * FileChooser: Stop flagging it as no_title, because it is *never* without a title. (This behavior was a remnant of the previous FM-specific title bar hacks, which are no longer a thing). * FileChooser: Stop calling `mergeTitleBarIntoLayout` twice in `updateItems`. We already call Menu's, which handles it. (Prevents the title bar from being added twice to the FocusManager layout). * FocusManager: Relax the `Unfocus` checks in `moveFocusTo` to ensure we *always* unfocus something (if unfocusing was requested), even if we have to blast the whole widget tree to do so. This ensures callers that mangle self.layout can expect things to work after calling it regardless of how borked the current focus is. * FocusManager: Allow passing `focus_flags` to `refocusWidget`, so that it can be forwarded to the internal `moveFocusTo` call. * FocusManager: The above also allows us to enforce a default that ensures we do *not* send a Focus event on Touch devices, even if they have the hasDPad devcap. This essentially restores the previous/current behavior of not showing the visual feedback from such focus "events" sent programmatically, given the `focus_flags` check fix at the root of this PR ;). * InputDialog: Fix numerous issues relating to double/ghost instances of both InputText and VirtualKeyboard, ensuring we only ever have a single InputText & VK instance live. * InputDialog: Make sure every way we have of hiding the VK play nice together, especially when the `toggleKeyboard` button (shown w/ `add_nav_bar`) is at play. And doubly so when we're `fullscreen`, as hiding the VK implies resizing the widget. * InputText: Make sure we're flagged as in-focus when tapping inside the text field. * InputText: Make sure we don't attempt to show an already-visible VK in the custom `hasDPad` `onFocus` handler. * Menu: Get rid of an old and no longer used (nor meaningful) hack in `onFocus` about the initial/programmatically-sent Focus event. * Menu: Get rid of the unused `header_padding` field mentioned earlier in the FM/FC fixes. * Menu: Use `FOCUS_ONLY_ON_NT` in the explicit `moveFocusTo` call in `updatePageInfo`, so as to keep the current behavior of not showing the visual feedback of this focus on Touch devices. * Menu: Make sure *all* the `moveFocusTo` calls are gated behind the `hasDPad` devcap (previously, that was only the case for `updatePageInfo`, but not `mergeTitleBarIntoLayout` (which is called by `updateItems`). * MultiInputDialog: Actively get rid of the InputText & VK instances from the base class's constructor that we do not use. * MultiInputDialog: Ensure the FocusManager layout is *slightly* less broken (password fields can still be a bit weird, though). * TextViewer: Get rid of the unfocus -> layout mangling -> refocus hack now that `refocusWidget` handles this case sanely. * VirtualKeyboard: Notify our parent InputDialog when we get closed, so it can act accordingly (e.g., resize itself when `fullscreen`). * ScrollableContainer: Implement the necessary machinery for focus handling inside ButtonDialog (specifically, when scrolling via PgUp/PgDwn). * TextEditor: Given the above fixes, the plugin is no longer disabled on non-touch devices. * ReaderBookMark: Make sure we request a full refresh when closing the "Edit note" dialog, as CRe highlights may extend past its dimensions, and if it's closed separately from VK, the refresh would have been limited to its own dimensions, leaving a neat InputDialog-sized hole in the highlights ;). --- frontend/apps/filemanager/filemanager.lua | 18 +++--- .../apps/reader/modules/readerbookmark.lua | 4 +- frontend/ui/widget/buttondialog.lua | 34 +++++++++- frontend/ui/widget/buttontable.lua | 2 +- frontend/ui/widget/configdialog.lua | 9 ++- .../widget/container/scrollablecontainer.lua | 10 +++ frontend/ui/widget/dictquicklookup.lua | 14 +++-- frontend/ui/widget/filechooser.lua | 3 +- frontend/ui/widget/focusmanager.lua | 40 +++++++++--- frontend/ui/widget/inputdialog.lua | 28 ++++++++- frontend/ui/widget/inputtext.lua | 8 ++- frontend/ui/widget/menu.lua | 62 +++++++++---------- frontend/ui/widget/multiinputdialog.lua | 19 +++++- frontend/ui/widget/textviewer.lua | 6 +- frontend/ui/widget/virtualkeyboard.lua | 14 ++++- plugins/coverbrowser.koplugin/listmenu.lua | 1 - plugins/coverbrowser.koplugin/mosaicmenu.lua | 1 - plugins/texteditor.koplugin/main.lua | 6 -- 18 files changed, 192 insertions(+), 87 deletions(-) diff --git a/frontend/apps/filemanager/filemanager.lua b/frontend/apps/filemanager/filemanager.lua index 3fac02e16..ac6b5438d 100644 --- a/frontend/apps/filemanager/filemanager.lua +++ b/frontend/apps/filemanager/filemanager.lua @@ -32,7 +32,6 @@ local ReaderWikipedia = require("apps/reader/modules/readerwikipedia") local ReadHistory = require("readhistory") local Screenshoter = require("ui/widget/screenshoter") local TitleBar = require("ui/widget/titlebar") -local VerticalGroup = require("ui/widget/verticalgroup") local UIManager = require("ui/uimanager") local filemanagerutil = require("apps/filemanager/filemanagerutil") local lfs = require("libs/libkoreader-lfs") @@ -146,11 +145,10 @@ function FileManager:setupLayout() self:updateTitleBarPath(self.root_path) local file_chooser = FileChooser:new{ - -- remember to adjust the height when new item is added to the group path = self.root_path, focused_path = self.focused_file, show_parent = self.show_parent, - height = Screen:getHeight() - self.title_bar:getHeight(), + height = Screen:getHeight(), is_popout = false, is_borderless = true, file_filter = function(filename) return DocumentRegistry:hasProvider(filename) end, @@ -159,8 +157,8 @@ function FileManager:setupLayout() return_arrow_propagation = true, -- allow Menu widget to delegate handling of some gestures to GestureManager filemanager = self, - -- let Menu widget merge our title_bar into its own TitleBar's FocusManager layout - outer_title_bar = self.title_bar, + -- Tell FileChooser (i.e., Menu) to use our own title bar instead of Menu's default one + custom_title_bar = self.title_bar, } self.file_chooser = file_chooser self.focused_file = nil -- use it only once @@ -348,16 +346,11 @@ function FileManager:setupLayout() return true end - self.layout = VerticalGroup:new{ - self.title_bar, - file_chooser, - } - local fm_ui = FrameContainer:new{ padding = 0, bordersize = 0, background = Blitbuffer.COLOR_WHITE, - self.layout, + file_chooser, } self[1] = fm_ui @@ -366,6 +359,9 @@ function FileManager:setupLayout() ui = self } + -- No need to reinvent the wheel, use FileChooser's layout + self.layout = file_chooser.layout + self:registerKeyEvents() end diff --git a/frontend/apps/reader/modules/readerbookmark.lua b/frontend/apps/reader/modules/readerbookmark.lua index 8cff6d2f0..1d54604c6 100644 --- a/frontend/apps/reader/modules/readerbookmark.lua +++ b/frontend/apps/reader/modules/readerbookmark.lua @@ -1241,7 +1241,9 @@ function ReaderBookmark:setBookmarkNote(item_or_index, is_new_note, new_note, ca text = _("Cancel"), id = "close", callback = function() - UIManager:close(input_dialog) + -- NOTE: We'll want a full refresh on close, as the CRe highlight may extend past our own dimensions, + -- especially if we're closed separately from our VirtualKeyboard. + UIManager:close(input_dialog, "flashui") if is_new_note then -- "Add note" called from highlight dialog and cancelled, remove saved highlight self:removeItemByIndex(index) end diff --git a/frontend/ui/widget/buttondialog.lua b/frontend/ui/widget/buttondialog.lua index 75c034a1d..b3522cb3d 100644 --- a/frontend/ui/widget/buttondialog.lua +++ b/frontend/ui/widget/buttondialog.lua @@ -44,10 +44,10 @@ local ButtonTable = require("ui/widget/buttontable") local CenterContainer = require("ui/widget/container/centercontainer") local Device = require("device") local Font = require("ui/font") +local FocusManager = require("ui/widget/focusmanager") local FrameContainer = require("ui/widget/container/framecontainer") local Geom = require("ui/geometry") local GestureRange = require("ui/gesturerange") -local InputContainer = require("ui/widget/container/inputcontainer") local LineWidget = require("ui/widget/linewidget") local MovableContainer = require("ui/widget/container/movablecontainer") local ScrollableContainer = require("ui/widget/container/scrollablecontainer") @@ -59,7 +59,7 @@ local VerticalSpan = require("ui/widget/verticalspan") local Screen = Device.screen local util = require("util") -local ButtonDialog = InputContainer:extend{ +local ButtonDialog = FocusManager:extend{ buttons = nil, width = nil, width_factor = nil, -- number between 0 and 1, factor to the smallest of screen width and height @@ -152,6 +152,7 @@ function ButtonDialog:init() title_widget = VerticalSpan:new{} title_widget_height = 0 end + self.top_to_content_offset = Size.padding.buttontable + Size.margin.default + title_widget_height -- If the ButtonTable ends up being taller than the screen, wrap it inside a ScrollableContainer. -- Ensure some small top and bottom padding, so the scrollbar stand out, and some outer margin @@ -233,6 +234,11 @@ function ButtonDialog:init() } } + -- No need to reinvent the wheel, ButtonTable's layout is perfect as-is + self.layout = self.buttontable.layout + -- But we'll want to control focus in its place, though + self.buttontable.layout = nil + self[1] = CenterContainer:new{ dimen = Screen:getSize(), self.movable, @@ -294,8 +300,30 @@ function ButtonDialog:onTapClose(arg, ges) end function ButtonDialog:paintTo(...) - InputContainer.paintTo(self, ...) + FocusManager.paintTo(self, ...) self.dimen = self.movable.dimen end +function ButtonDialog:onFocusMove(args) + local ret = FocusManager.onFocusMove(self, args) + + -- If we're using a ScrollableContainer, ask it to scroll to the focused item + if self.cropping_widget then + local focus = self:getFocusItem() + if self.dimen and focus and focus.dimen then + local button_y_offset = focus.dimen.y - self.dimen.y - self.top_to_content_offset + -- NOTE: The final argument ensures we'll always keep the neighboring item visible. + -- (i.e., the top/bottom of the scrolled view is actually the previous/next item). + self.cropping_widget:_scrollBy(0, button_y_offset, true) + end + end + + return ret +end + +function ButtonDialog:_onPageScrollToRow(row) + -- ScrollableContainer will pass us the row number of the top widget at the current scroll offset + self:moveFocusTo(1, row) +end + return ButtonDialog diff --git a/frontend/ui/widget/buttontable.lua b/frontend/ui/widget/buttontable.lua index 6b07ee4b2..00d10ed68 100644 --- a/frontend/ui/widget/buttontable.lua +++ b/frontend/ui/widget/buttontable.lua @@ -222,7 +222,7 @@ function ButtonTable:getStepScrollGrid() local row_num = 1 while idx <= #self.container do local row = { - row_num = row_num, -- (not used, but may help with debugging) + row_num = row_num, -- (used by ScrollableContainer, and generally helpful for debugging) top = offsets[idx].y, -- top of our vspan above text content_top = offsets[idx+1].y, -- top of our text widget content_bottom = offsets[idx+2].y - 1, -- bottom of our text widget diff --git a/frontend/ui/widget/configdialog.lua b/frontend/ui/widget/configdialog.lua index 8321c0fb4..d6a086a27 100644 --- a/frontend/ui/widget/configdialog.lua +++ b/frontend/ui/widget/configdialog.lua @@ -687,7 +687,7 @@ function ConfigOption:_itemGroupToLayoutLine(option_items_group) if v.layout and v.disableFocusManagement then -- it is a FocusManager -- merge child layout to one row layout -- currently child widgets are all one row - -- need improved if two or more rows widget existed + -- needs improvement if we ever implement widgets with two or more rows for _, row in ipairs(v.layout) do for _, widget in ipairs(row) do layout_line[j] = widget @@ -910,7 +910,7 @@ end function ConfigDialog:updateConfigPanel(index) end function ConfigDialog:update() - self:moveFocusTo(1, 1) -- reset selected for re-created layout + self:moveFocusTo(1, 1, FocusManager.NOT_FOCUS) -- reset selected for re-created layout self.layout = {} if self.config_menubar then @@ -1134,6 +1134,11 @@ function ConfigDialog:onConfigMoreChoose(values, default_value_orig, name, event UIManager:setDirty(self, function() return "ui", self.dialog_frame.dimen end) + -- FocusManager loses its marbles (we can only navigate on the row of the selected option) if we don't update the widget *again*... + -- (possibly because of the layout nastiness happening in ConfigOption:init) + if Device:hasDPad() then + self:update() + end end end local hide_on_picker_show = more_options_param.hide_on_picker_show diff --git a/frontend/ui/widget/container/scrollablecontainer.lua b/frontend/ui/widget/container/scrollablecontainer.lua index 40a9f56e7..2eec01f27 100644 --- a/frontend/ui/widget/container/scrollablecontainer.lua +++ b/frontend/ui/widget/container/scrollablecontainer.lua @@ -632,11 +632,20 @@ function ScrollableContainer:onScrollablePanRelease(_, ges) return false end +function ScrollableContainer:_notifyParentOfPageScroll() + -- For ButtonDialog's focus shenanigans, as we ourselves are not a FocusManager + if self.show_parent and self.show_parent._onPageScrollToRow then + local top_row = self:_getStepScrollRowAtY(self._scroll_offset_y, true) + self.show_parent:_onPageScrollToRow(top_row and top_row.row_num or 1) + end +end + function ScrollableContainer:onScrollPageUp() if not self._is_scrollable then return false end self:_scrollBy(0, -self._crop_h, true) + self:_notifyParentOfPageScroll() return true end @@ -645,6 +654,7 @@ function ScrollableContainer:onScrollPageDown() return false end self:_scrollBy(0, self._crop_h, true) + self:_notifyParentOfPageScroll() return true end diff --git a/frontend/ui/widget/dictquicklookup.lua b/frontend/ui/widget/dictquicklookup.lua index dcf1cb456..dbb0bc90e 100644 --- a/frontend/ui/widget/dictquicklookup.lua +++ b/frontend/ui/widget/dictquicklookup.lua @@ -104,7 +104,7 @@ function DictQuickLookup:init() self.key_events.ReadPrevResult = { { Input.group.PgBack } } self.key_events.ReadNextResult = { { Input.group.PgFwd } } self.key_events.Close = { { Input.group.Back } } - self.key_events.ShowResultsMenu = { { "Menu" } } + self.key_events.MenuKeyPress = { { "Menu" } } if Device:hasKeyboard() then self.key_events.ChangeToPrevDict = { { "Shift", "Left" } } self.key_events.ChangeToNextDict = { { "Shift", "Right" } } @@ -726,10 +726,12 @@ function DictQuickLookup:init() -- NT: add dict_title.left_button and lookup_edit_button to FocusManager. -- It is better to add these two buttons into self.movable, but it is not a FocusManager. - -- Only self.button_table is a FocusManager, so workaground is inserting these two buttons into self.button_table.layout. + -- Only self.button_table is a FocusManager, so the workaround is inserting these two buttons into self.button_table.layout. if Device:hasDPad() then - table.insert(self.button_table.layout, 1, { self.dict_title.left_button }); - table.insert(self.button_table.layout, 2, { lookup_edit_button }); + table.insert(self.button_table.layout, 1, { self.dict_title.left_button }) + table.insert(self.button_table.layout, 2, { lookup_edit_button }) + -- Refocus on the updated layout + self.button_table:refocusWidget() end -- We're a new window @@ -1098,6 +1100,10 @@ function DictQuickLookup:onReadPrevResult() return true end +function DictQuickLookup:onMenuKeyPress() + return self.dict_title.left_icon_tap_callback() +end + function DictQuickLookup:onTap(arg, ges_ev) if ges_ev.pos:notIntersectWith(self.dict_frame.dimen) then self:onClose() diff --git a/frontend/ui/widget/filechooser.lua b/frontend/ui/widget/filechooser.lua index 04c1fdeab..31c6e7eb4 100644 --- a/frontend/ui/widget/filechooser.lua +++ b/frontend/ui/widget/filechooser.lua @@ -17,8 +17,8 @@ local _ = require("gettext") local Screen = Device.screen local T = ffiUtil.template +-- NOTE: It's our caller's responsibility to setup a title bar and pass it to us via custom_title_bar (c.f., FileManager) local FileChooser = Menu:extend{ - no_title = true, path = lfs.currentdir(), show_path = true, parent = nil, @@ -494,7 +494,6 @@ end function FileChooser:updateItems(select_number, no_recalculate_dimen) Menu.updateItems(self, select_number, no_recalculate_dimen) -- call parent's updateItems() - self:mergeTitleBarIntoLayout() self.path_items[self.path] = (self.page - 1) * self.perpage + (select_number or 1) end diff --git a/frontend/ui/widget/focusmanager.lua b/frontend/ui/widget/focusmanager.lua index abb33089b..9968d022f 100644 --- a/frontend/ui/widget/focusmanager.lua +++ b/frontend/ui/widget/focusmanager.lua @@ -293,10 +293,12 @@ function FocusManager:onPhysicalKeyboardDisconnected() end -- constant, used to reset focus widget after layout recreation --- not send Unfocus event +-- do not send an Unfocus event FocusManager.NOT_UNFOCUS = 1 --- not need to send Focus event +-- do not send a Focus event FocusManager.NOT_FOCUS = 2 +-- In some cases, we may only want to send Focus events on non-Touch devices +FocusManager.FOCUS_ONLY_ON_NT = (Device:hasDPad() and not Device:isTouchDevice()) and 0 or FocusManager.NOT_FOCUS --- Move focus to specified widget function FocusManager:moveFocusTo(x, y, focus_flags) @@ -318,10 +320,22 @@ function FocusManager:moveFocusTo(x, y, focus_flags) self.selected.y = y -- widget create new layout on update, previous may be removed from new layout. if Device:hasDPad() then - if not bit.band(focus_flags, FocusManager.NOT_UNFOCUS) and current_item and current_item ~= target_item then - current_item:handleEvent(Event:new("Unfocus")) + if bit.band(focus_flags, FocusManager.NOT_UNFOCUS) ~= FocusManager.NOT_UNFOCUS then + -- NOTE: We can't necessarily guarantee the integrity of self.layout, + -- as some callers *will* mangle it and call us expecting to fix things ;). + -- Since we do not want to leave *multiple* items (visually) focused, + -- we potentially need to be a bit heavy-handed ;). + if current_item and current_item ~= target_item then + -- This is the absolute best-case scenario, when self.layout's integrity is sound + current_item:handleEvent(Event:new("Unfocus")) + else + -- Couldn't find the current item, or it matches the target_item: blast the whole widget container, + -- just in case we still have a different, older widget visually focused. + -- Can easily happen if caller calls refocusWidget *after* having manually mangled self.layout. + self:handleEvent(Event:new("Unfocus")) + end end - if not bit.band(focus_flags, FocusManager.NOT_FOCUS) then + if bit.band(focus_flags, FocusManager.NOT_FOCUS) ~= FocusManager.NOT_FOCUS then target_item:handleEvent(Event:new("Focus")) UIManager:setDirty(self.show_parent or self, "fast") end @@ -468,24 +482,32 @@ function FocusManager:disableFocusManagement(parent) end -- constant for refocusWidget method to ease code reading +FocusManager.RENDER_NOW = false FocusManager.RENDER_IN_NEXT_TICK = true --- Container calls this method to re-set focus widget style --- Some container regenerate layout on update and lose focus style -function FocusManager:refocusWidget(nextTick) +function FocusManager:refocusWidget(nextTick, focus_flags) + -- On touch devices, we do *not* want to show visual focus changes generated programmatically, + -- we only want to see them for actual user input events (#12361). + if not focus_flags then + focus_flags = FocusManager.FOCUS_ONLY_ON_NT + end + if not self._parent then if not nextTick then - self:moveFocusTo(self.selected.x, self.selected.y) + self:moveFocusTo(self.selected.x, self.selected.y, focus_flags) else -- sometimes refocusWidget called in widget's action callback -- widget may force repaint after callback, like Button with vsync = true -- then focus style will be lost, set focus style to next tick to make sure focus style painted UIManager:nextTick(function() - self:moveFocusTo(self.selected.x, self.selected.y) + self:moveFocusTo(self.selected.x, self.selected.y, focus_flags) end) end else - self._parent:refocusWidget(nextTick) + self._parent:refocusWidget(nextTick, focus_flags) + self._parent = nil end end diff --git a/frontend/ui/widget/inputdialog.lua b/frontend/ui/widget/inputdialog.lua index 14b41bbf3..2091d611d 100644 --- a/frontend/ui/widget/inputdialog.lua +++ b/frontend/ui/widget/inputdialog.lua @@ -303,6 +303,7 @@ function InputDialog:init() local line_height = input_widget:getLineHeight() local input_pad_height = input_widget:getSize().h - text_height local keyboard_height = self.keyboard_visible and input_widget:getKeyboardDimen().h or 0 + input_widget:onCloseKeyboard() -- we don't want multiple VKs, as the show/hide tracking assumes there's only one input_widget:onCloseWidget() -- free() textboxwidget and keyboard -- Find out available height local available_height = self.screen_height @@ -350,6 +351,10 @@ function InputDialog:init() end end end + -- In case of reinit, murder our previous input widget to prevent stale VK instances from lingering + if self._input_widget then + self._input_widget:onCloseWidget() + end self._input_widget = self.inputtext_class:new{ text = self.input, hint = self.input_hint, @@ -381,7 +386,11 @@ function InputDialog:init() } table.insert(self.layout[1], self._input_widget) self:mergeLayoutInVertical(self.button_table) - self:refocusWidget() + -- NOTE: Never send a Focus event, as, on hasDPad device, InputText's onFocus *will* call onShowKeyboard, + -- and that will wreak havoc on toggleKeyboard... + -- Plus, the widget at (1, 1) will not have changed, so we don't actually need to change the visual focus anyway? + -- If it turns out something actually needed this, make this conditional on a new `reinit` arg passed to `init`, for toggleKeyboard & co. + self:refocusWidget(FocusManager.RENDER_NOW, FocusManager.NOT_FOCUS) -- Complementary setup for some of our added buttons if self.save_callback then local save_button = self.button_table:getButtonById("save") @@ -641,10 +650,27 @@ function InputDialog:toggleKeyboard(force_toggle) self:lockKeyboard(true) end + -- Clear the FocusManager highlight, because that gets lost in the mess somehow... + self.button_table:getButtonById("keyboard"):onUnfocus() + -- Make sure we refresh the nav bar, as it will have moved, and it belongs to us, not to VK or our input widget... self:refreshButtons() end +-- fullscreen mode & add_nav_bar breaks some of our usual assumptions about what should happen on "Back" input events... +function InputDialog:onKeyboardClosed() + if self.add_nav_bar and self.fullscreen then + -- If the keyboard was closed via a key event (Back), make sure we reinit properly like in toggleKeyboard... + self.input = self:getInputText() + self:onClose() + self:free() + + self:init() + + self:refreshButtons() + end +end + function InputDialog:onKeyboardHeightChanged() local visible = self:isKeyboardVisible() self.input = self:getInputText() -- re-init with up-to-date text diff --git a/frontend/ui/widget/inputtext.lua b/frontend/ui/widget/inputtext.lua index 0e92b1086..2eafcd745 100644 --- a/frontend/ui/widget/inputtext.lua +++ b/frontend/ui/widget/inputtext.lua @@ -142,6 +142,8 @@ local function initTouchEvents() if self.keyboard then self.keyboard:showKeyboard() end + -- Make sure we're flagged as in focus again + self:focus() end if self._frame_textwidget.dimen ~= nil -- zh keyboard with candidates shown here has _frame_textwidget.dimen = nil and #self.charlist > 0 then -- do not move cursor within a hint @@ -297,13 +299,15 @@ end local function initDPadEvents() if Device:hasDPad() then function InputText:onFocus() - -- Event called by the focusmanager + -- Event sent by focusmanager if self.parent.onSwitchFocus then self.parent:onSwitchFocus(self) elseif (Device:hasKeyboard() or Device:hasScreenKB()) and G_reader_settings:isFalse("virtual_keyboard_enabled") then do end -- luacheck: ignore 541 else - self:onShowKeyboard() + if not self:isKeyboardVisible() then + self:onShowKeyboard() + end end self:focus() return true diff --git a/frontend/ui/widget/menu.lua b/frontend/ui/widget/menu.lua index c64dec6c5..147fd4e78 100644 --- a/frontend/ui/widget/menu.lua +++ b/frontend/ui/widget/menu.lua @@ -475,25 +475,19 @@ function MenuItem:getDotsText(face) return _dots_cached_info.text, _dots_cached_info.min_width end -function MenuItem:onFocus(initial_focus) - if Device:isTouchDevice() then - -- Devices which are Keys capable will get this onFocus called by - -- updateItems(), which will toggle the underline color of first item. - -- If the device is also Touch capable, let's not show the initial - -- underline for a prettier display (it will be shown only when keys - -- are used). - if not initial_focus or self.menu.did_focus_with_keys then - self._underline_container.color = Blitbuffer.COLOR_BLACK - self.menu.did_focus_with_keys = true - end - else - self._underline_container.color = Blitbuffer.COLOR_BLACK - end +function MenuItem:onFocus() + self._underline_container.color = Blitbuffer.COLOR_BLACK + -- NOTE: Medium is really, really, really thin; so we'd ideally swap to something thicker... + -- Unfortunately, this affects vertical text positioning, + -- leading to an unsightly refresh of the item :/. + --self._underline_container.linesize = Size.line.thick return true end function MenuItem:onUnfocus() self._underline_container.color = self.line_color + -- See above for reasoning. + --self._underline_container.linesize = self.linesize return true end @@ -580,13 +574,13 @@ local Menu = FocusManager:extend{ no_title = false, title = "", + custom_title_bar = nil, subtitle = nil, show_path = nil, -- path in titlebar subtitle -- default width and height width = nil, -- height will be calculated according to item number if not given height = nil, - header_padding = Size.padding.default, dimen = nil, item_table = nil, -- NOT mandatory (will be empty) item_table_stack = nil, @@ -610,7 +604,7 @@ local Menu = FocusManager:extend{ items_font_size = nil, items_mandatory_font_size = nil, multilines_show_more_text = nil, - -- Global settings or default values will be used if not provided + -- Global settings or default values will be used if not provided -- Setting this to a number enables flexible height of items -- and sets the maximum number of lines in an item, longer items are truncated items_max_lines = nil, @@ -620,7 +614,7 @@ local Menu = FocusManager:extend{ -- if you want to embed the menu widget into another widget, set -- this to false is_popout = true, - title_bar_fm_style = nil, -- set to true to build increased title bar like in FileManager + title_bar_fm_style = nil, -- set to true to mimic FileManager's custom title bar (extra padding & subtitle) -- set icon to add title bar left button title_bar_left_icon = nil, -- close_callback is a function, which is executed when menu is closed @@ -644,14 +638,12 @@ function Menu:_recalculateDimen(no_recalculate_dimen) local top_height = 0 if self.title_bar and not self.no_title then top_height = self.title_bar:getHeight() - if not self.title_bar_fm_style then - top_height = top_height + self.header_padding - end end local bottom_height = 0 if self.page_return_arrow and self.page_info_text then + -- The extra padding is for UX reasons only, to leave a bit of space above the footer. bottom_height = math.max(self.page_return_arrow:getSize().h, self.page_info_text:getSize().h) - + 2 * Size.padding.button + + Size.padding.button end self.available_height = self.inner_dimen.h - top_height - bottom_height self.item_dimen = Geom:new{ @@ -698,7 +690,7 @@ function Menu:init() if self.subtitle == nil and (self.show_path or self.title_bar_fm_style) then self.subtitle = "" end - self.title_bar = TitleBar:new{ + self.title_bar = self.custom_title_bar or TitleBar:new{ width = self.dimen.w, fullscreen = "true", align = "center", @@ -1004,8 +996,10 @@ end function Menu:updatePageInfo(select_number) if #self.item_table > 0 then if Device:hasDPad() then - -- reset focus manager accordingly - self:moveFocusTo(1, select_number) + -- Reset focus manager accordingly. + -- NOTE: Since this runs automatically on init, + -- we use FOCUS_ONLY_ON_NT as we don't want to see the initial underline on Touch devices. + self:moveFocusTo(1, select_number, FocusManager.FOCUS_ONLY_ON_NT) end -- update page information self.page_info_text:setText(T(_("Page %1 of %2"), self.page, self.page_num)) @@ -1132,20 +1126,20 @@ function Menu:mergeTitleBarIntoLayout() return end local menu_item_layout_start_row = 1 - local titlebars = {self.title_bar, self.outer_title_bar} - for _, v in ipairs(titlebars) do - -- Menu uses the right key to trigger the context menu: we can't use it to move focus in horizontal directions. - -- So, add title bar buttons to FocusManager's layout in a vertical-only layout - local title_bar_layout = v:generateVerticalLayout() - for _, row in ipairs(title_bar_layout) do - table.insert(self.layout, menu_item_layout_start_row, row) - menu_item_layout_start_row = menu_item_layout_start_row + 1 - end + -- On hasFewKeys devices, Menu uses the "Right" key to trigger the context menu: we can't use it to move focus in horizontal directions. + -- So, add title bar buttons to FocusManager's layout in a vertical-only layout + local title_bar_layout = self.title_bar:generateVerticalLayout() + for _, row in ipairs(title_bar_layout) do + table.insert(self.layout, menu_item_layout_start_row, row) + menu_item_layout_start_row = menu_item_layout_start_row + 1 end if menu_item_layout_start_row > #self.layout then -- no menu items menu_item_layout_start_row = #self.layout -- avoid index overflow end - self:moveFocusTo(1, menu_item_layout_start_row) -- move focus to first menu item if any, keep original behavior + if Device:hasDPad() then + -- Move focus to the first menu item, if any, in keeping with the pre-FocusManager behavior + self:moveFocusTo(1, menu_item_layout_start_row, FocusManager.NOT_FOCUS) + end end --[[ diff --git a/frontend/ui/widget/multiinputdialog.lua b/frontend/ui/widget/multiinputdialog.lua index cfb5c86af..0a2494755 100644 --- a/frontend/ui/widget/multiinputdialog.lua +++ b/frontend/ui/widget/multiinputdialog.lua @@ -104,12 +104,24 @@ local MultiInputDialog = InputDialog:extend{ function MultiInputDialog:init() -- init title and buttons in base class InputDialog.init(self) + -- Kick InputDialog's own field out of the layout, we're not using it + table.remove(self.layout, 1) + -- Also murder said input field *and* its VK, or we get two of them and shit gets hilariously broken real fast... + self:onCloseKeyboard() + self._input_widget:onCloseWidget() + local VerticalGroupData = VerticalGroup:new{ align = "left", self.title_bar, } local content_width = math.floor(self.width * 0.9) + -- In case of reinit, murder our previous input widgets to prevent stale VK instances from lingering + if self.input_fields then + for i, widget in ipairs(self.input_fields) do + widget:onCloseWidget() + end + end self.input_fields = {} local input_description = {} for i, field in ipairs(self.fields) do @@ -117,7 +129,7 @@ function MultiInputDialog:init() text = field.text, hint = field.hint, input_type = field.input_type, - text_type = field.text_type, -- "password" + text_type = field.text_type, -- "password" face = self.input_face, width = content_width, idx = i, @@ -136,7 +148,10 @@ function MultiInputDialog:init() enter_callback = self.enter_callback, } table.insert(self.input_fields, input_field_tmp) - table.insert(self.layout, { input_field_tmp }) + --- @fixme: This is semi-broken when text_type is password, as we actually end up with the checkbox instead of the field, + -- and a "Press" on the checkbox will actually focus the password field and *not* check the box. + -- addWidget may have added stuff below us, so make sure we insert above that... + table.insert(self.layout, i, { input_field_tmp }) if field.description then input_description[i] = FrameContainer:new{ padding = self.description_padding, diff --git a/frontend/ui/widget/textviewer.lua b/frontend/ui/widget/textviewer.lua index c7e3c1ba8..c700baee5 100644 --- a/frontend/ui/widget/textviewer.lua +++ b/frontend/ui/widget/textviewer.lua @@ -15,7 +15,6 @@ local ButtonTable = require("ui/widget/buttontable") local CenterContainer = require("ui/widget/container/centercontainer") local CheckButton = require("ui/widget/checkbutton") local Device = require("device") -local Event = require("ui/event") local Geom = require("ui/geometry") local Font = require("ui/font") local FrameContainer = require("ui/widget/container/framecontainer") @@ -276,11 +275,8 @@ function TextViewer:init(reinit) -- NT: add titlebar.left_button (hamburger menu) to FocusManager. if Device:hasDPad() and not (Device:hasSymKey() or Device:hasScreenKB()) then - -- ButtonTable calls refocusWidget on init, but we'll mangle the layout, - -- so kill the initial highlight while FocusManager can still find the current focused item... - self.button_table:handleEvent(Event:new("Unfocus")) table.insert(self.button_table.layout, 1, { self.titlebar.left_button }) - -- And refocus manually on the *actual* layout + -- Refocus on the updated layout self.button_table:refocusWidget() end diff --git a/frontend/ui/widget/virtualkeyboard.lua b/frontend/ui/widget/virtualkeyboard.lua index 3f00d38ff..37de4bd4a 100644 --- a/frontend/ui/widget/virtualkeyboard.lua +++ b/frontend/ui/widget/virtualkeyboard.lua @@ -938,8 +938,14 @@ end function VirtualKeyboard:onClose() UIManager:close(self) if self.inputbox and Device:hasDPad() then - -- let input text handle Back event to unfocus - -- otherwise, another extra Back event needed + -- Let InputText handle this KeyPress "Back" event to unfocus, otherwise, another extra Back event is needed. + -- NOTE: Keep in mind InputText is a special snowflake, and implements the raw onKeyPress handler for this! + -- Also, notify another widget that actually may want to know when *we* get closed, i.e., the parent (Input*Dialog*). + -- We need to do this manually because InputText's onKeyPress handler will very likely return true, + -- stopping event propagation (c.f., the last hasDPad branch of said handler). + if self.inputbox and self.inputbox.parent and self.inputbox.parent.onKeyboardClosed then + self.inputbox.parent:onKeyboardClosed() + end return false end return true @@ -974,6 +980,10 @@ function VirtualKeyboard:onCloseWidget() -- this could be moved to InputDialog's onShow/onCloseWidget handlers (but, it would allow input on unfocused fields). -- NOTE: But something more complex, possibly based on an in-class ref count would have to be implemented in order to be able to deal -- with multiple InputDialogs being shown and closed in asymmetric fashion... Ugh. + -- NOTE: You would also have to deal with the fact that, once InputText loses focus, + -- it will stop dealing with key events because it wouldn't know where to send them when there are multiple live instances of it, + -- specifically because, given how we propagate events, the key event will go to whichever inputtext comes earlier in the container's array... + -- c.f., 2ccf7601fe1cbd9794aea0be754ea4166b9767d7 in #12361 and the comments surrounding it ;). Device:stopTextInput() end diff --git a/plugins/coverbrowser.koplugin/listmenu.lua b/plugins/coverbrowser.koplugin/listmenu.lua index 9dc26c605..da8f838b7 100644 --- a/plugins/coverbrowser.koplugin/listmenu.lua +++ b/plugins/coverbrowser.koplugin/listmenu.lua @@ -899,7 +899,6 @@ function ListMenu:_recalculateDimen() self.others_height = self.others_height + 2 end if not self.no_title then - self.others_height = self.others_height + self.header_padding self.others_height = self.others_height + self.title_bar.dimen.h end if self.page_info then diff --git a/plugins/coverbrowser.koplugin/mosaicmenu.lua b/plugins/coverbrowser.koplugin/mosaicmenu.lua index 34b35946a..796951539 100644 --- a/plugins/coverbrowser.koplugin/mosaicmenu.lua +++ b/plugins/coverbrowser.koplugin/mosaicmenu.lua @@ -883,7 +883,6 @@ function MosaicMenu:_recalculateDimen() self.others_height = self.others_height + 2 end if not self.no_title then - self.others_height = self.others_height + self.header_padding self.others_height = self.others_height + self.title_bar.dimen.h end if self.page_info then diff --git a/plugins/texteditor.koplugin/main.lua b/plugins/texteditor.koplugin/main.lua index 952651ecd..d7174a71a 100644 --- a/plugins/texteditor.koplugin/main.lua +++ b/plugins/texteditor.koplugin/main.lua @@ -1,9 +1,3 @@ -local Device = require("device") - -if not Device:isTouchDevice() then - return { disabled = true } -end - local BD = require("ui/bidi") local ConfirmBox = require("ui/widget/confirmbox") local DataStorage = require("datastorage")