Skip to content
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

Reader: Allow scrolling small offsets in continuous mode. #11148

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aero31aero
Copy link

@aero31aero aero31aero commented Nov 23, 2023

Using the drag gestures is cumbersome and finicky, especially when you want to do it all the time. Thus, we add a new option here to modify the default tapping behavior.


Relates to #11139 made yesterday, based on @poire-z 's suggestion. Although that one-liner didn't work at all (it was merely being used in a >0 or <0 comparison, whether to go forward or backward), it was somewhat trivial to wrap the functionality that the mouse-wheel scroll implemented.

image

This merely brings consistency to the experience; previously, (and now, in the "legacy" setting), you'd notice the screen would scroll a little bit on each tap until you reached the end of an image, whereupon it'd completely transition to the new image.

I had no experience with Lua till yesterday, and I still wasn't able to get a toolchain working to compile this for my Kindle, so I've only tested on Linux so far.


This change is Reviewable

Using the drag gestures is cumbersome and finicky, especially when
you want to do it all the time. Thus, we add a new option here to
modify the default tapping behavior.
@poire-z
Copy link
Contributor

poire-z commented Nov 23, 2023

I had no experience with Lua till yesterday, and I still wasn't able to get a toolchain working to compile this for my Kindle

You don't need to build or compile anything: you just touched the .lua files, you can just copy these 3 files in their respective directories on your Kindle (the files are there as-is, they are not packaged/massaged/transformed/compiled in any way).

@poire-z
Copy link
Contributor

poire-z commented Nov 23, 2023

Not being a PDF (nor comics) reader, I can't say thoughts about that feature and if it is worth having.

So, some comments non-feature-wise:

We managed to somehow normalize the bottom panels to be 4-items tall, so not fond of having this one grow :) Although the previous panel can already grow to 5 items depending on the "Zoom to" item selected.
(So, may be it could only appear on some conditions, if there are any, see below?)

You probably just tested your new setting with the other settings in the mode you are using. You'll need to test how it behave in all combinations of settings.

A tap will go into your new function no matter the other settings, and previously, via onGotoViewRel, it reached either onScrollPageRel or onGotoPageRel, which do many different things depending on the other settings, than you will probably not have done anymore.
So, you'd probably want more conditionals to go only then thru your new stuff.
You have an enabled_func expecting page_scroll==1 in your koptptions, so if that's not enough, you'll need to update the checks here too so it's reflected in that bottom menu when this has effects or not.

self:onGotoPosRel(self.page_scroll_amount * diff) with diff = 1/-1 , and page_scroll_amount being 1, 2 or 3, this goes thru self:onPanningRel(100*that). So, with 3, how much do we move, 300px ? If yes, the pan will move by 100px, 200px or 300px.
Shouldn't this be a factor of the screen height? (I think that normally, this gets the distance of the finger pan, so it reflects the reality). Small, medium and large should may be relate to 1/4 , 1/2, 3/4 of the screen height (random numbers, I have no idea what's really usable and expected by users that will chose from the 3 names we propose).

Also, how does this interact with Page gap?

@Frenzie
Copy link
Member

Frenzie commented Nov 23, 2023

I think this is trying to reimplement the overlap pixels setting, but it's still not really clear what combination of settings leads to the behavior you're describing.

koreader/defaults.lua

Lines 46 to 47 in bf03f40

-- page overlap pixels
DOVERLAPPIXELS = 30,

@aero31aero
Copy link
Author

aero31aero commented Nov 24, 2023

I think this is trying to reimplement the overlap pixels setting,

I didn't notice there was already something like this, will take a look.

If yes, the pan will move by 100px, 200px or 300px.

Yeah, I couldn't figure it out and wanted to get a piece of concept hacked together quickly, but I wanted to make the UI for this like the line-height or font-size ones, where you could just enter a number you like as well. %age of page makes a lot of sense.

Wouldn't mind keeping the setting hidden away deep in the menus somewhere if the visual consistency is important, though.

Also, how does this interact with Page gap?

The scroll distance here also applies on the page gap pixels, so its behaving as expected. This code is already in KOReader, I'm just finding a new way to invoke it.


I think I can do another thing here that'd keep things as they are for people yet provide this option. Offer this as a custom gesture that can be setup, so then one could double-tap to invoke it etc. Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants