-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FR: Better D-pad support on K4/5 #11295
Comments
Thinking about this, would it be possible to re-map the left/right to next chapter (when inside a book), and up/down to trigger this feature from #8877 using a user patch? navigation on non-touch is quite difficult the next/previous chapter would really help a lot. |
If any of those are currently mapped to any button events it might be simpler still. I don't have time to investigate right now, but what you're asking should probably be relatively straightforward (although you never know when something's strongly dependent on something else). |
if you find some time to look into it, it would be amazing, as I don't know how to implement it. |
You'll have to figure out with detailed logs what the output for those buttons is. |
I have the logs, I think... Logs
|
Okay, so simply Left, Right, Up, Down, and Press. For the reader we can probably simply remove the not hasFewKeys from these lines and add some new ones for the chapter stuff. koreader/frontend/apps/reader/modules/readerpaging.lua Lines 55 to 64 in d301157
The rest of what you said I don't have time to think about right this moment. ;-) |
I've removed the self.key_events.GotoNextPage = {
{ "RPgFwd", "LPgFwd" },
event = "GotoViewRel",
args = 1,
}
self.key_events.GotoPrevPage = {
{ "RPgBack", "LPgBack" },
event = "GotoViewRel",
args = -1,
}
-- self.key_events.GotoNextChapter = {
-- { "Right" },
-- event = "",
-- args = 1,
-- }
-- self.key_events.GotoPrevChapter = {
-- { "Left" },
-- event = "",
-- args = -1,
-- } |
Ah sorry, that was slightly more of a note to self, but you're probably testing ReaderRolling: koreader/frontend/apps/reader/modules/readerrolling.lua Lines 119 to 130 in d301157
It's broadly speaking PDF/DjVu/CBZ/etc. ("paged") vs EPUB/FB2/etc (er, "rolling" in the code but something like "dynamic" might be more descriptive). |
PS I think GotoNextChapter should be correct. |
no worries, there is still the up/down that triggers "start content selection". But that would require writing a new function and I can't do that one. should I make a PR for the left/right stuff? |
Annoyingly, I don't really see a way out of that without |
this is what I did: if Device:hasKeys() then
self.key_events.GotoNextView = {
{ { "RPgFwd", "LPgFwd" } },
event = "GotoViewRel",
args = 1,
}
self.key_events.GotoPrevView = {
{ { "RPgBack", "LPgBack" } },
event = "GotoViewRel",
args = -1,
}
end
if Device:hasDPad() then
self.key_events.MoveUp = {
{ "Up" },
event = "Panning",
args = {0, -1},
}
self.key_events.MoveDown = {
{ "Down" },
event = "Panning",
args = {0, 1},
}
self.key_events.GotoNextChapter = {
{ "Right" },
event = "GotoNextChapter",
args = 1,
}
self.key_events.GotoPrevChapter = {
{ "Left" },
event = "GotoPrevChapter",
args = -1,
}
end |
Kindles definitely have back buttons, and a d-pad |
It's not Kindles I'm talking about. My point is we don't want to make something else unusable where the left/right thing isn't redundant but actively necessary. |
What about applying the changes only to So even this #11295 (comment) could affect someone using a random unknown non-touch device? |
That's the bad version of |
But isn’t |
Currently it's merely the outline of a potential concept. But I think it is (or rather, would be) its own independent thing. It's only the combination of |
But to explain a bit more, something like koreader/frontend/device/generic/device.lua Lines 26 to 139 in 87c85bf
|
I see, so a little more complex than expected. What would I need to do to re-map locally (user patch) the up/down buttons to 'start content selection' and the keyboard button to do something? For now at least. |
Screenshots, that too. Capturing them i mean. |
Wait a second, but the |
That is likely an oversight on my end. Quoting from #6195:
I'm building the program right now, but on this older laptop it'll take a while. So on a normal device, left/right and up/down are already in use for page turning vs relative position changing. The former is somewhat similar in purpose to what you'd prefer to be chapters instead. (A PDF page is typically taller than the screen, meaning it'll take two page turns to get through fully.) |
another thing, for some reason the top menu does not respect the hierarchy of the 'Home' button. When you go deep into the menus the 'home' button doesn't do anything, when it should take you back to the file browser. |
Home button? |
I don't think that's wrong per se, but it does harm greppability. |
not sure what it means, but it sounds terrible. In that case they could both be now it is time to have that conversation, my recent commit does now break panning when on continuous mode. what to do? I tried adding a nested |
It means that if you search for "this and that" to see if it occurs anywhere you'll miss "that and this". grep in its various forms is the standard text searching program. So yes, make them the same please.
That wouldn't work; the event handlers are registered on load. How bothered are we about continuous mode there @poire-z, if at all? |
Haven't followed what's all this all about (happily not having a non-touch device, so not bothering), and not sure what you're asking me :/ If I got it right and you want to fix that, I guess you should send new events in your new code for ArrowUp/Down, and in the new handler(s), dispatch new events depending on the mode. I'm not really familiar with the non-touch stuff, and I don't really know the differences between hasKeys/hasDpad - and what the emulator "has". And it seems it's all again different with PDF (no content selection?) :/ Again, not sure if I got it right and if it changes that much stuff, would be nice to ping some other Kindle NT users to discuss the changes and agree on some new behaviour if it's that much. |
Yes, me too. I think it's the evidently correct behavior but that might differ per device. |
exactly that, only this change should target kindles exclusively, in the words of Frenzie |
What I said was closer to calling that code smell. But the fact is: a. we like the current behavior |
with all due respect, you might do, but you don't use non-touch kindles.
yes, but there was no "content selection" at that time. why anyone thought having four sets of buttons doing the same was a great idea is beyond me. #4329 here is another example a somewhat similar suggestion btw all this is doing is simply somewhat replicating stock firmware behaviour, which is superior in this case. having said that, it is possibly less urgent to have both (one is probably enough) up/down calling content selection (btw this is dict lookup and highlights) here but for now can't think of another thing to add there. |
Exactly, that's my point. :-) Everything has to be carefully considered, not just non-touch Kindles. And on non-touch Kindles you also have to consider everything, not just one particular document type in one particular view mode. But we'll get there. |
This might do the trick: if not Device:hasKeyboard() and Device:hasDPad() and Device:hasPageUpDownKeys() then
|
It might be preferable to abstract that away in a different manner to make it explicit rather than fully emergent. That would probably make it more obvious to future readers why it's doing these things. Something along these lines for example: diff --git a/frontend/device/generic/device.lua b/frontend/device/generic/device.lua
index a2f24426c..0e83dfe9b 100644
--- a/frontend/device/generic/device.lua
+++ b/frontend/device/generic/device.lua
@@ -43,6 +43,7 @@ local Device = {
hasKeys = no,
canKeyRepeat = no,
hasDPad = no,
+ hasDPadWeaponSelect = no,
hasExitOptions = yes,
hasFewKeys = no,
hasWifiToggle = yes,
@@ -231,6 +232,10 @@ function Device:init()
end
end
+ if not Device:hasKeyboard() and Device:hasDPad() and Device:hasPageUpDownKeys() then
+ self.hasDPadWeaponSelect = yes
+ end
+
if self:hasGSensor() then
-- Setup our standard gyro event handler (EV_MSC:MSC_GYRO)
if G_reader_settings:nilOrFalse("input_ignore_gsensor") then |
Is that the device ? http://www.btobey.com/review/kindle-4-buttons.jpg |
But this is fragmenting everything even more, and i'll be the devil's advocate this time, that does not seem good. I might as well say "model == Kindle4" or whatever it is. I think it is better to cover all non-touch equally in terms of dPad options. How about adding an option under "cog > navigation > physical buttons" where you choose either panning or content selection, even more options could be added (and force a restart like when activating/deactivating plugins). But that would require some effort from your end as i doubt i could pull that off. Edit: should i make a PR now so we can all commit stuff? edit 2: I want to make extremely clear, content selection isn't just highlights. It is probably 80% dictionary lookups, 15% full text searches, 4% highlights and 1% others (yes I made up the numbers, but largely the idea holds true). |
People who like to use the d-pad would beg to differ. But I never said that adapting to the situation on the ground was bad, only that
I suppose so. The defaults should still be as stated above or much like it regardless though.
At least as far as code goes that's easier to talk about, yes. |
Yes, it is. There was a picture in an earlier #11295 (comment)
Yes, on both sides for right or left hand holding.
Yes, although I personally prefer the page-turn-button style on the previous kindle 3 (keyboard), the device is light and quite easy to hold.
I can't argue with that, preferences are indeed a personal thing.
That seems to be where we all have finally landed. yay all of us. #11749 is now open, so if you find yourself in need of helping someone out, feel free to drop by. |
Unlike Little Timmy, I'm quite disciplined: it's written "Non-touch", so non-touching it :) |
Hello all, So I am trying to bring screenshots to non-touch devices but I was found wanting. for reference this is our file and init() koreader/frontend/ui/widget/screenshoter.lua Lines 16 to 34 in ca8e935
and I am adding something like this to it (i.e if not Device:isTouchDevice() then
if Device:hasKeyboard() then
-- self.key_events = {
-- KeyPressShoot = {
-- Screenshot = { { "Shift" and "S" } },
-- },
-- }
elseif Device:hasKeys() then
self.key_events.KeyPressShoot = { { { "Press", "Menu" } }, event = "KeyPressShoot", args = 1 }
end
end and this function of course: function Screenshoter:onKeyPressShoot()
return self:onScreenshot()
end but it is not working as expected. With that code, |
Try to remove one pair of curly brackets. |
I added the extra one |
Would be nice if there was a way to map the unused keyboard key on Kindle 4 to the selection mode |
there is a PR #11749 (comment) that will add better support, including selection mode tied to a press key, maybe not the ScreenKB key as it would not be consistent with the older kindles with keyboards. |
trying to run this now self.key_events.KeyPressShoot = { {Key:match({ "ScreenKB", "Menu" })}, event = "KeyPressShoot", args = 1, } ps. I changed from "Press" to "ScreenKB",
edit: I believe this does not work because neither of those keys are modifiers, hmmm. any advice? |
Note to self: koreader/frontend/ui/widget/touchmenu.lua Line 513 in e94550a
|
Hi KoReader team,
I am a new KoR user, I am using a Black Kindle 4/5 (B023, K4) and running version 2023.10.
My issue concerns the D-pad, currently the D-pad is set to individual page turns (either forwards or backwards) for both up/down and left/right, however the kindle already has two sets of turn page buttons which makes it a total of FOUR sets of buttons doing exactly the same job.
I was wondering if you guys could make the D-pad behave the same way the stock firmware does, i.e left/right moves you to previous/next chapter and up/down allows you to select words for dictionary lookup or highlights.
This leads me to another issue and much in the same spirit as before, the only way of selecting words is using 'content selection' which is at best annoying on non-touch kindles. so once again, I am wondering whether 'content selection' could be added via d-pad instead in much the same way stock firmware does it (up/down).
Also, the keyboard button does not do anything whatsoever, it would be nice if you could map it to either 'full text search' or to give the user the option to select what they'd like to do. For example turn wifi on/off.
thank you for your time and consideration.
commodore64user
The text was updated successfully, but these errors were encountered: