Button: Better handling of translucent MovableContainer (#7223)

* DictQuickLookup: Preserve alpha when switching dict, and scrolling
inside a dict.

* Start moving the NumberPicker alpha hack to Button itself

This makes handling flash_ui easier and saner, avoiding flickering.

* Handle the transparency hack entirely from within Button

* Murder the now unnecessary NumberPicker update_callback hack

* Tweak comments

* And the Button handling made that redundant, too

* Squish debug print

* More comment tweaks

* Reset transparency on scrolling instead of rpeserving it

* Reset alpha when switching dictionaries

* Simplify the pre/post callbakc transparency state handling

And explain why we need to care.

* Give a named reference to ButtonDialog's MovableContainer, so the Button
alpha hack behaves with it

* Document the "self.movable" convention

* Amend that comment a bit

e.g., we don't care much about MultiConfirmBox'w MpvableContainer, as
any button action will close it.

* And make SkimTo's MovableContainer accessible so that Button can grok
that it's translucent

frontend/apps/reader/skimtowidget.lua

@@ -316,6 +316,10 @@ function SkimToWidget:init()
             vertical_group_down,
         }
     }
+    self.movable = MovableContainer:new{
+        -- alpha = 0.8,
+        self.skimto_frame,
+    }
     self[1] = WidgetContainer:new{
         align = "center",
         dimen =Geom:new{
@@ -323,10 +327,7 @@ function SkimToWidget:init()
             w = self.screen_width,
             h = self.screen_height,
         },
-        MovableContainer:new{
-            -- alpha = 0.8,
-            self.skimto_frame,
-        }
+        self.movable,
     }
 
     if Device:hasDPad() then

frontend/ui/uimanager.lua

@@ -674,6 +674,10 @@ to, among other things, flag the right widget as setDirty (c.f., those pesky deb
 This is why you often see stuff doing, when instantiating a new widget, FancyWidget:new{ show_parent = self.show_parent or self };
 meaning, if I'm already a subwidget, cascade my parent, otherwise, it means I'm a window-level widget, so cascade myself as that widget's parent ;).
 
+Another convention (that a few things rely on) is naming a (persistent) MovableContainer wrapping a full widget `movable`, accessible as an instance field.
+This is useful when it's used for transparency purposes, which, e.g., Button relies on to handle highlighting inside a transparent widget properly,
+by checking if self.show_parent.movable exists and is currently translucent ;).
+
 @usage
 
 UIManager:setDirty(self.widget, "partial")

frontend/ui/widget/button.lua

@@ -234,6 +234,13 @@ function Button:showHide(show)
 end
 
 function Button:onTapSelectButton()
+    -- NOTE: We have a few tricks up our sleeve in case our parent is inside a translucent MovableContainer...
+    local was_translucent = self.show_parent and self.show_parent.movable and self.show_parent.movable.alpha
+    -- We make a distinction between transparency pre- and post- callback, because if a widget *was* transparent,
+    -- but no longer is post-callback, we want to ensure that we refresh the *full* container,
+    -- instead of just the button's frame, in order to avoid leaving bits of the widget transparent ;).
+    local is_translucent = was_translucent
+
     if self.enabled and self.callback then
         if G_reader_settings:isFalse("flash_ui") then
             self.callback()
@@ -276,7 +283,13 @@ function Button:onTapSelectButton()
                 UIManager:forceRePaint() -- Ensures we have a chance to see the highlight
             end
             self.callback()
-            UIManager:forceRePaint() -- Ensures whatever the callback wanted to paint will be shown *now*...
+            -- Check if the callback reset transparency...
+            is_translucent = was_translucent and self.show_parent.movable.alpha
+            -- We don't want to fence the callback when we're *still* translucent, because we want a *single* refresh post-callback *and* post-unhighlight,
+            -- in order to avoid flickering.
+            if not is_translucent then
+                UIManager:forceRePaint() -- Ensures whatever the callback wanted to paint will be shown *now*...
+            end
             if self.vsync then
                 -- NOTE: This is mainly useful when the callback caused a REAGL update that we do not explicitly fence already,
                 --       (i.e., Kobo Mk. 7).
@@ -346,6 +359,16 @@ function Button:onTapSelectButton()
     elseif type(self.tap_input_func) == "function" then
         self:onInput(self.tap_input_func())
     end
+
+    -- If our parent belongs to a translucent MovableContainer, repaint all the things to honor alpha without layering glitches,
+    -- and refresh the full container, because the widget might have inhibited its own setDirty call to avoid flickering (c.f., *SpinWidget).
+    if was_translucent then
+        -- If the callback reset the transparency, we only need to repaint our parent
+        UIManager:setDirty(is_translucent and "all" or self.show_parent, function()
+            return "ui", self.show_parent.movable.dimen
+        end)
+    end
+
     if self.readonly ~= true then
         return true
     end

frontend/ui/widget/buttondialog.lua

@@ -37,9 +37,7 @@ function ButtonDialog:init()
             }
         }
     end
-    self[1] = CenterContainer:new{
-        dimen = Screen:getSize(),
-        MovableContainer:new{
+    self.movable = MovableContainer:new{
             alpha = self.alpha,
             FrameContainer:new{
                 ButtonTable:new{
@@ -56,19 +54,23 @@ function ButtonDialog:init()
                 padding_top = 0,
                 padding_bottom = 0,
             }
-        }
+    }
+
+    self[1] = CenterContainer:new{
+        dimen = Screen:getSize(),
+        self.movable,
     }
 end
 
 function ButtonDialog:onShow()
     UIManager:setDirty(self, function()
-        return "ui", self[1][1].dimen
+        return "ui", self.movable.dimen
     end)
 end
 
 function ButtonDialog:onCloseWidget()
     UIManager:setDirty(nil, function()
-        return "flashui", self[1][1].dimen
+        return "flashui", self.movable.dimen
     end)
 end
 
@@ -87,7 +89,7 @@ end
 
 function ButtonDialog:paintTo(...)
     InputContainer.paintTo(self, ...)
-    self.dimen = self[1][1].dimen -- FrameContainer
+    self.dimen = self.movable.dimen
 end
 
 return ButtonDialog

frontend/ui/widget/dictquicklookup.lua

@@ -828,14 +828,15 @@ function DictQuickLookup:update()
         end
     end
 
-    -- Reset alpha to avoid stacking transparency on top of the previous content.
-    -- NOTE: This doesn't take care of the Scroll*Widget, which will preserve alpha on scroll,
-    --       leading to increasingly opaque and muddy text as half-transparent stuff gets stacked on top of each other...
-    self.movable.alpha = nil
-
-    UIManager:setDirty(self, function()
-        return "partial", self.dict_frame.dimen
-    end)
+    -- If we're translucent, reset alpha to make the new definition actually readable.
+    if self.movable.alpha then
+        self.movable.alpha = nil
+        -- And skip the setDirty, Button will handle it post-callback & post-unhighlight.
+    else
+        UIManager:setDirty(self, function()
+            return "partial", self.dict_frame.dimen
+        end)
+    end
 end
 
 function DictQuickLookup:getInitialVisibleArea()

frontend/ui/widget/doublespinwidget.lua

@@ -83,10 +83,6 @@ function DoubleSpinWidget:init()
 end
 
 function DoubleSpinWidget:update()
-    -- This picker_update_callback will be redefined later.
-    -- It's a hack to restore transparency after a Button unhighlight in NumberPicker,
-    -- in case the MovableContainer was actually made transparent.
-    local picker_update_callback = function() end
     local left_widget = NumberPickerWidget:new{
         show_parent = self,
         width = self.picker_width,
@@ -96,7 +92,6 @@ function DoubleSpinWidget:update()
         value_step = self.left_step,
         value_hold_step = self.left_hold_step,
         wrap = false,
-        update_callback = function() picker_update_callback() end,
     }
     local right_widget = NumberPickerWidget:new{
         show_parent = self,
@@ -107,7 +102,6 @@ function DoubleSpinWidget:update()
         value_step = self.right_step,
         value_hold_step = self.right_hold_step,
         wrap = false,
-        update_callback = function() picker_update_callback() end,
     }
     local left_vertical_group = VerticalGroup:new{
         align = "center",
@@ -287,31 +281,11 @@ function DoubleSpinWidget:update()
         },
         self.movable,
     }
-    UIManager:setDirty(self, function()
-        return "ui", self.widget_frame.dimen
-    end)
-    picker_update_callback = function()
-        -- If we're actually transparent, force an alpha-aware repaint.
-        if self.movable.alpha then
-            if G_reader_settings:nilOrTrue("flash_ui") then
-                -- It's delayed to the next tick to actually catch a Button unhighlight.
-                UIManager:nextTick(function()
-                    UIManager:setDirty("all", function()
-                        return "ui", self.movable.dimen
-                    end)
-                end)
-            else
-                -- This should only really be necessary for the up/down buttons here,
-                -- because they repaint the center value button & text, unlike said button,
-                -- which just pops up the VK.
-                -- On the upside, we shouldn't need to delay anything without flash_ui ;).
-                UIManager:setDirty("all", function()
-                    return "ui", self.movable.dimen
-                end)
-            end
-        end
-        -- If we'd like to have the values auto-applied, uncomment this:
-        -- self.callback(left_widget:getValue(), right_widget:getValue())
+    -- If we're translucent, Button itself will handle that post-callback, in order to preserve alpha without flickering.
+    if not self.movable.alpha then
+        UIManager:setDirty(self, function()
+            return "ui", self.widget_frame.dimen
+        end)
     end
 end
 

frontend/ui/widget/numberpickerwidget.lua

@@ -12,7 +12,6 @@ Example:
         value_step = 1,
         value_hold_step = 4,
         wrap = true,
-        update_callback = function() end,
     }
 --]]
 
@@ -46,7 +45,6 @@ local NumberPickerWidget = InputContainer:new{
     value_table = nil,
     value_index = nil,
     wrap = true,
-    update_callback = function() end,
     -- in case we need calculate number of days in a given month and year
     date_month = nil,
     date_year = nil,
@@ -169,7 +167,6 @@ function NumberPickerWidget:init()
                     },
                 },
             }
-            self.update_callback()
             UIManager:show(input_dialog)
             input_dialog:onShowKeyboard()
         end
@@ -229,7 +226,6 @@ function NumberPickerWidget:update()
     UIManager:setDirty(self.show_parent, function()
         return "ui", self.dimen
     end)
-    self.update_callback()
 end
 
 --[[--

frontend/ui/widget/scrollhtmlwidget.lua

@@ -119,9 +119,19 @@ function ScrollHtmlWidget:scrollToRatio(ratio)
     self.htmlbox_widget:freeBb()
     self.htmlbox_widget:_render()
 
-    UIManager:setDirty(self.dialog, function()
-        return "partial", self.dimen
-    end)
+    -- If our dialog is currently wrapped in a MovableContainer and that container has been made translucent,
+    -- reset the alpha and refresh the whole thing, because we assume that a scroll means the user actually wants to
+    -- *read* the content, which is kinda hard on a nearly transparent widget ;).
+    if self.dialog.movable and self.dialog.movable.alpha then
+        self.dialog.movable.alpha = nil
+        UIManager:setDirty(self.dialog, function()
+            return "partial", self.dialog.movable.dimen
+        end)
+    else
+        UIManager:setDirty(self.dialog, function()
+            return "partial", self.dimen
+        end)
+    end
 end
 
 function ScrollHtmlWidget:scrollText(direction)
@@ -147,9 +157,17 @@ function ScrollHtmlWidget:scrollText(direction)
     self.htmlbox_widget:freeBb()
     self.htmlbox_widget:_render()
 
-    UIManager:setDirty(self.dialog, function()
-        return "partial", self.dimen
-    end)
+    -- Handle the container's alpha as above...
+    if self.dialog.movable and self.dialog.movable.alpha then
+        self.dialog.movable.alpha = nil
+        UIManager:setDirty(self.dialog, function()
+            return "partial", self.dialog.movable.dimen
+        end)
+    else
+        UIManager:setDirty(self.dialog, function()
+            return "partial", self.dimen
+        end)
+    end
 end
 
 function ScrollHtmlWidget:onScrollText(arg, ges)

frontend/ui/widget/scrolltextwidget.lua

@@ -150,9 +150,17 @@ function ScrollTextWidget:updateScrollBar(is_partial)
         if is_partial then
             refreshfunc = "partial"
         end
-        UIManager:setDirty(self.dialog, function()
-            return refreshfunc, self.dimen
-        end)
+        -- Reset transparency if the dialog's MovableContainer is currently translucent...
+        if is_partial and self.dialog.movable and self.dialog.movable.alpha then
+            self.dialog.movable.alpha = nil
+            UIManager:setDirty(self.dialog, function()
+                return refreshfunc, self.dialog.movable.dimen
+            end)
+        else
+            UIManager:setDirty(self.dialog, function()
+                return refreshfunc, self.dimen
+            end)
+        end
         if self.scroll_callback then
             self.scroll_callback(low, high)
         end

frontend/ui/widget/spinwidget.lua

@@ -78,10 +78,6 @@ function SpinWidget:init()
 end
 
 function SpinWidget:update()
-    -- This picker_update_callback will be redefined later.
-    -- It's a hack to restore transparency after a Button unhighlight in NumberPicker,
-    -- in case the MovableContainer was actually made transparent.
-    local picker_update_callback = function() end
     local value_widget = NumberPickerWidget:new{
         show_parent = self,
         width = math.floor(self.screen_width * 0.2),
@@ -93,7 +89,6 @@ function SpinWidget:update()
         value_step = self.value_step,
         value_hold_step = self.value_hold_step,
         precision = self.precision,
-        update_callback = function() picker_update_callback() end,
     }
     local value_group = HorizontalGroup:new{
         align = "center",
@@ -239,29 +234,11 @@ function SpinWidget:update()
         },
         self.movable,
     }
-    UIManager:setDirty(self, function()
-        return "ui", self.spin_frame.dimen
-    end)
-    picker_update_callback = function()
-        -- If we're actually transparent, force an alpha-aware repaint.
-        if self.movable.alpha then
-            if G_reader_settings:nilOrTrue("flash_ui") then
-                -- It's delayed to the next tick to actually catch a Button unhighlight.
-                UIManager:nextTick(function()
-                    UIManager:setDirty("all", function()
-                        return "ui", self.movable.dimen
-                    end)
-                end)
-            else
-                -- This should only really be necessary for the up/down buttons here,
-                -- because they repaint the center value button & text, unlike said button,
-                -- which just pops up the VK.
-                -- On the upside, we shouldn't need to delay anything without flash_ui ;).
-                UIManager:setDirty("all", function()
-                    return "ui", self.movable.dimen
-                end)
-            end
-        end
+    -- If we're translucent, Button itself will handle that post-callback, in order to preserve alpha without flickering.
+    if not self.movable.alpha then
+        UIManager:setDirty(self, function()
+            return "ui", self.spin_frame.dimen
+        end)
     end
 end