[FocusManager] correct cursor keys in RTL

[Commodore64user]

what's new

• frontend/ui/widget/focusmanager.lua: Added logic in the onFocusMove function to flip the horizontal direction when RTL mode is enabled.
• frontend/apps/reader/modules/readerpaging.lua: Introduced nextKey and prevKey variables that adjust key events based on the layout direction.
• frontend/apps/reader/modules/readerrolling.lua: Similar changes to readerpaging.lua, adjusting key events based on the layout direction using nextKey and prevKey.

reported #12579 (comment)

okay @poire-z, this should solve the cursor key problem. But there is still another one. The focus underline sticks to the left side of the screen (touchmenu)... when it should move along with the text to the right

---

This change is 

[poire-z]

But there is still another one. The focus underline sticks to the left side of the screen (touchmenu)... when it should move along with the text to the right

Indeed... How all these widget are set up is a bit messy.
But before trying to fix it, a question: you, as a NT user, have you been fine with the underline taking only the width of the text above it ? Or do you think it should take the whole width ?
(In KeyValuePage (a rectangle now) or Menu/Filebrowser in classic mode, it takes the whole width, but they have some stuff on the right.)

[Commodore64user]

you, as a NT user, have you been fine with the underline taking only the width of the text above it ? Or do you think it should take the whole width ?

I don't mind it the way it is. As you said, unlike in the other places, there is nothing going on at the opposite end so it doesn't quite make sense to make it full width. What I have thought sometimes—and this is perhaps less so with touch menu (but still very much) and more with list and classic views—is having a thicker underline but last time NiLuJe tried that he quickly backtracked because of reasons...

[NiLuJe]

Those reasons being it eats into the content bbox, leading to (vertical) text alignment issues (i.e., it shifts the baseline)

[Commodore64user]

Those reasons being it eats into the content bbox, leading to (vertical) text alignment issues (i.e., it shifts the baseline)

hey, what are you doing here? ;). We have been waiting for you at #13328.

[Commodore64user]

Anyway, this issue was caused by not inverting self.layout when on RTL mode, so are you satisfied with the solution @poire-z ?

[poire-z]

I'd prefer seeing the math stuff only once instead of duplicated just to get it -.

dx = math.floor(#row / 2)
dx = BD.mirroredUILayout() and -dx or dx
-- or just the straighforward:
if BD.mirroredUILayout()  then dx = -dx end

[Commodore64user]

okay

[poire-z]

Aren't you reverting your dx = BD.mirroredUILayout() and math.floor(#row / 2) or -math.floor(#row / 2) here ?

[Commodore64user]

in most cases, yes, but when we fall in any if/elseif for "left" or "right", then no.

[poire-z]

It still reads odd (and may feel like a bug to an outsider - I don't know what this half-move is all about, and don't want to know).
If there's no obvious/simpler way, please add a comment so our later selves know it's on purpose and not a bug.

[Commodore64user]

btw, the idea is that we have to unfix the value so when it gets to onFocusMove it is "fixed" again

[poire-z]

But there is still another one. The focus underline sticks to the left side of the screen (touchmenu)... when it should move along with the text to the right

Indeed... How all these widgets are set up is a bit messy.

"a bit" was an understatement, it's a BIG mess. There's so many inconsistencies in how our widgets work:

• :getSize() sometimes using a provided self.dimen (WidgetContainer), sometimes not (FrameContainer)
• so having hacks in FrameContainer accepting a width=, but using for it half the things
• (and we must have calling sites that use :getSize() or self.dimen - whatever worked for the developper at the time, including me - that really makes it hard to "fix" things)
• UnderlineContainer using none of these to get its width
• then in TouchMenuItem with its menu item flashing, we use that width= to hack the area to flash

I spent a few hours trying to make UnderlineContainer either behave like a proper WidgetContainer, or behave like FrameContainer, but there were too many tweaks to do elsewhere, with the risks of side effects in the few other widget using it.
So, for RTL & NT users, may be we can go with ANOTHER hack added to the pile of adhoc ugly ones ?:

--- a/frontend/ui/widget/container/underlinecontainer.lua
+++ b/frontend/ui/widget/container/underlinecontainer.lua
@@ -6,2 +6,3 @@ a line under its child node.

+local BD = require("ui/bidi")
 local Blitbuffer = require("ffi/blitbuffer")
@@ -17,2 +18,3 @@ local UnderlineContainer = WidgetContainer:extend{
     vertical_align = "top",
+    line_width = nil, -- (Don't use this, it's there because of the complex and ugly layout in TouchMenuItem)
 }
@@ -39,2 +41,9 @@ function UnderlineContainer:paintTo(bb, x, y)
     end
+
+    local line_width = self.line_width or self.dimen.w
+    local line_x = x
+    if BD.mirroredUILayout() then
+        line_x = line_x + self.dimen.w - line_width
+    end
+
     local content_size = self[1]:getSize()
@@ -47,4 +56,4 @@ function UnderlineContainer:paintTo(bb, x, y)
     self[1]:paintTo(bb, x, p_y)
-    bb:paintRect(x, y + container_size.h - self.linesize,
-        container_size.w, self.linesize, self.color)
+    bb:paintRect(line_x, y + container_size.h - self.linesize,
+        line_width, self.linesize, self.color)
 end
diff --git a/frontend/ui/widget/touchmenu.lua b/frontend/ui/widget/touchmenu.lua
index 0b3bd3ac2..fbece57e0 100644
--- a/frontend/ui/widget/touchmenu.lua
+++ b/frontend/ui/widget/touchmenu.lua
@@ -148,2 +148,3 @@ function TouchMenuItem:init()
         dimen = self.dimen:copy(),
+        line_width = self.item_frame:getSize().w, -- we'll draw a shorter line
         self.item_frame,

[NiLuJe]

• :getSize() sometimes using a provided self.dimen (WidgetContainer), sometimes not (FrameContainer)

I have repressed memories about that insanity :D.

(And, yeah, I refrained from touching too much of that mess when I looked at the whole "WTF is happening with dimen" a few years ago; way too easy to break wonky assumptions elsewhere :/).

So, yeah, I'm fine with a hack if nobody has the bandwidth and time to deal with the fallout (and, hell, the actual minutiae needed to actually fix the mess in the first place) ;).

[Commodore64user]

What’s the deal here? Should i close this?