Please don't tack on random button swaps.

Please don't tack on random button swaps.

Yes, it has some kind of logic: #7511 (comment)

It is not random, when you edit a string, say "reluctantly" to "reluctant" you need to press twice as many keys to perform the new dic lookup. Having it like this reduces the presses from 4 to 2, (from Back - down - down - right to just Back - down)

[       reluctant      ]
[dictionary] [wikipedia]
[cancel]     [translate]

This is a seemingly innocent PR that randomly tacks on unrelated changes. I borderline merged it before noticing this unwelcome stowaway.

This is a seemingly innocent PR that randomly tacks on unrelated changes. I borderline merged it before noticing this unwelcome stowaway.

Are we still talking about the swap? Is not unrelated, didn't i just explain the reasoning?

You did, but in the attempt to make it easier for NT users, you make it unnatural and less easy for right handed touch devices users.
If you want to make it easier for NT users, benefiting from our layout choices (most useful buttons on the right, dangerous (because they enable wifi) ones on the left), you could tweak the dedicated self.layout stuff, for this widget or even internally reversing self.layout = self.buttons_layout - or some other trick - in buttontable.lua so it starts on the right column.

I can revert it if a patch is offered in return... deal or no deal?

I can revert it if a patch is offered in return... deal or no deal?

No deal.

Feel free to submit another PR that doesn't hijack the menu order.

Wtf man? Why do you have to act in such a 'tyrannical' way when i just said i am going to put it back (later when i get time to sit in front of my computer), only i am not allowed to ask @poire-z directly for help with patches so i have to make it open to all, playful and silly.

I put some boundaries. That's always received as 'tyrannical'. I have no problem with that!

when i just said i am going to put it back (later when i get time to sit in front of my computer)

That's explicit. You could have used something like:

"Alright, I'll change this to the original button placement!. Is there a way to swap them using a patch?"

But your answers don't imply that ¿?

"Are we still talking about the swap? Is not unrelated, didn't i just explain the reasoning?"
"I can revert it if a patch is offered in return... deal or no deal?"

I have to make it open to all, playful and silly.

I usually have no problem with playful and silly comments. Yours is something else. You're addressing reviewers that asked you to change your PR. "Deal or no deal?" I addressed that, with no deal.

only i am not allowed to ask @poire-z directly for help

I don't think you aren't :) (I remember I mentionned once I would not help you as long as you discussed the thing in the horrible GitHub Discussions, but I had since helped you.)

To prove you wrong:

diff --git a/frontend/ui/widget/buttontable.lua b/frontend/ui/widget/buttontable.lua
index 4b50795b..4f1131ae 100644
--- a/frontend/ui/widget/buttontable.lua
+++ b/frontend/ui/widget/buttontable.lua
@@ -45,7 +45,10 @@ function ButtonTable:init()
     local table_min_needed_width = -1
     for i = 1, row_cnt do
         self:addVerticalSpan()
-        local buttons_layout_line = {}
+        -- All (not all?) button tables layout are aimed at right handed people, with most useful
+        -- and less risky buttons on the right. Make this available to Non-Touch devices too by
+        -- entering vertically merged buttontables items on the right
+        local buttons_layout_line = { gravity = "right" }
         local horizontal_group = HorizontalGroup:new{}
         local row = self.buttons[i]
         local column_cnt = #row
diff --git a/frontend/ui/widget/dictquicklookup.lua b/frontend/ui/widget/dictquicklookup.lua
index c5f8ad53..11b5c604 100644
--- a/frontend/ui/widget/dictquicklookup.lua
+++ b/frontend/ui/widget/dictquicklookup.lua
@@ -723,6 +723,7 @@ function DictQuickLookup:init()
     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 });
+        self.button_table:refocusWidget(false, true)
     end

     -- We're a new window
diff --git a/frontend/ui/widget/focusmanager.lua b/frontend/ui/widget/focusmanager.lua
index abb33089..343dadce 100644
--- a/frontend/ui/widget/focusmanager.lua
+++ b/frontend/ui/widget/focusmanager.lua
@@ -223,6 +223,7 @@ function FocusManager:onFocusMove(args)
         return true
     end
     local current_item = self.layout[self.selected.y][self.selected.x]
+    local current_gravity = self.layout[self.selected.y].gravity
     while true do
         if not self.layout[self.selected.y + dy] then
             --horizontal border, try to wraparound
@@ -244,6 +245,12 @@ function FocusManager:onFocusMove(args)
             self.selected.x = self.selected.x + dx
         end
         logger.dbg("FocusManager cursor position is:", self.selected.x, ",", self.selected.y)
+        if self.layout[self.selected.y].gravity ~= current_gravity and dx == 0 then
+            if self.layout[self.selected.y].gravity == "right" then
+                -- We're moving up/down, and meeting a row with gravity=right: focus on the right most item
+                self.selected.x = #self.layout[self.selected.y]
+            end
+        end

         if self.layout[self.selected.y][self.selected.x] ~= current_item
         or not self.layout[self.selected.y][self.selected.x].is_inactive then
@@ -472,8 +479,18 @@ 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, force_reset)
     if not self._parent then
+        if force_reset then
+            self.selected.x = 1
+            self.selected.y = 1
+            self._gravity_refocused = nil
+        end
+        if not self._gravity_refocused and self.layout and self.layout[self.selected.y].gravity == "right" then
+            -- This is used by ButtonTable: initially focus on the right most item
+            self.selected.x = #self.layout[self.selected.y]
+            self._gravity_refocused = true
+        end
         if not nextTick then
             self:moveFocusTo(self.selected.x, self.selected.y)
         else

This was my initial idea:

• I have no idea how usable it is, and how it behave in other contexts, I just tested it on the text selection popup (focusing on the right), on the DictLookup results, and on its pen icon's InputDialog.
• it's not finished - I think it needs some similar tweaks to _wrapAroundY, _verticalStep...
• it was already quite painful to just get there :/
• I used the terms gravity = right, I think i had read that in some UI toolkit doc, possibly Android - there may be some better words and logic, as we will only use it with right
• on the emulator, the initially focused button is not highlighted (no "reverse video"), only when we move we get the highlighting on the second button. Dunno how it behave on real NT devices, and if you will see the top right button highlighted/focused.
• FocusManager is really hard to grasp (it's quite simple though, a 2-dimensional array self.layout we navigater with up/down/left/right arrows, but there are many special handling, halfMove, handling holes in the array, etc... I suggest you get familiar with it, because it's only of interest to NT users :)

No man, you could have easily requested changes via a code review and that would have been "setting boundaries". Also, even thought i indirectly indicated that i would change them back, you didn't give me enough time (and notice) to do so. I have been busy and have not had time to use a computer all day. Perhaps my message was bad at conveying that, but your actions were a lot worse.

One more thing "deal or no deal" is a British show based on a famous dutch one, which i thought perhaps @Frenzie would have appreciated if he had gotten the reference (long shot though)

I didn't catch the reference, no. But @pazos did make me smile. Don't serve it if you can't dish it. ^_^

The same, "simple" place swap (translate for dictionary) but on patch form should be enough really. No need to change how anything else functions.

I appreciate your help @poire-z, i really do.

You mean/meant "user patch" all along ?! :/
(I thought you still had a little faith in helping humanity and wanted to make something generic - not just solve your own most used little case.)

Well, there's no real possibility to hijack the function and just swap them at the end, but you can just replace the whole function with a copy of the original with the buttons swapped, it's not a function we change often.
2-swap-some-dict-buttons.lua

local DictQuickLookup = require("ui/widget/dictquicklookup")
-- all the other local xyz=require for all the things used in the function, ie.
local InputDialog = require("ui/widget/inputdialog")
local UIManager = require("ui/uimanager")

DictQuickLookup.lookupInputWord = function(self, hint)
    -- copy of the original one, and modify it as you wish
end

You mean/meant "user patch" all along ?! :/
(I thought you still had a little faith in helping humanity and wanted to make something generic - not just solve your own most used little case.)

yes, i just don’t think that changing how the focus normally works is a better approach. It is okay though, i just might be the only person that cares anyway and unlike what some people stated, it was never intended as a hijack. If i had known it would bring the worst in some people i would not have committed it.

anyway, back to bedlam now. (This is a joke btw)

thought about adding to kindle something like

if key["Shift"] then
    if key["."] then
          self:addChars(",")
    end
end

so if one hides the keyboard, one can still type [regular] sentences without needing to bring up the VK, but I end up adding ,. instead. I think somewhere Shift + . is already set to . what would be the best way of dealing with that?

what gives?

If you don't try, or give feedback about, my suggestions, I guess I won't bother investigating that one with you.

what gives?

If you don't try, or give feedback about, my suggestions, I guess I won't bother investigating that one with you.

apologies, I think I must have misread your message cause I thought you were saying find the other bit and add it there

elseif key["."] then
    self:addChars(",")
    return true

that works fine

Unrelated to this PR as it happens with or without, just adding a note to remember:

When in some text box (ie. adding a note for a long highlight), hitting the Home key keep moving to the start of the previous line (dunno if that is the behaviour we expected ? feels like we shouldn't move anymore when we are at the start of line), when reaching the first line and hitting Home again, it crashes with:

./luajit: frontend/ui/widget/textboxwidget.lua:1888: attempt to index a nil value
stack traceback:
        frontend/ui/widget/textboxwidget.lua:1888: in function 'moveCursorHome'
        frontend/ui/widget/scrolltextwidget.lua:237: in function 'moveCursorHome'
        frontend/ui/widget/inputtext.lua:934: in function 'goToHome'
        frontend/ui/widget/inputtext.lua:614: in function 'handleEvent'

Let's not add that here but, do we know why the "Alt" thing won't work then... #12078 (comment)

is this it?:

Instead of copy/paste existing code in comments, and needing us to figure out where it comes from to get more context, you can use:

and paste the link, which gives us:

do we know why the "Alt" thing won't work then

Dunno :) This snippet is later, so probably not.
But may be you also forgot a return true or handled=true so to not go through that later snippet.

First and foremost, this is done here so, do your thing @Frenzie ...

you can use: and paste the link, which gives us:

I am familiar with that, it was just too late and could not be bothered. Easier to copy and paste. my bad.

But may be you also forgot a return true or handled=true so to not go through that later snippet.

even mr fancy pants over here does not work:

if Device:hasSymKey() then
    if not handled and key["Alt"] then
        handled = true
        if key["Q"] then
            self:addChars("1")
        elseif key["W"] then
            self:addChars("2")
        elseif key["E"] then
            self:addChars("3")
        elseif key["R"] then
            self:addChars("4")
        elseif key["T"] then
            self:addChars("5")
        elseif key["Y"] then
            self:addChars("6")
        elseif key["U"] then
            self:addChars("7")
        elseif key["I"] then
            self:addChars("8")
        elseif key["O"] then
            self:addChars("9")
        elseif key["P"] then
            self:addChars("0")
        else
            handled = false
        end
    end
end

so there is something blocking it "somewhere only we [don't] know" (any Keane fans?)

You say it's not working yet?

You say it's not working yet?

The PR as is right now works fine, what doesn't work is the "alt" + "top row keys" to get numerical values. But that is not included here [yet] and could be add it later.