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

cre: allow text selection/highlighting across pages #4544

Merged
merged 2 commits into from Feb 3, 2019

Conversation

Projects
None yet
4 participants
@poire-z
Copy link
Contributor

poire-z commented Feb 3, 2019

Follow up on an initial idea and experimentations by @Galunid in #4496 (comment))

Panning to the bottom right corner (or top left corner) switches to scroll mode and scroll the page forward (resp. backward) 1/3rd of the screen.
One has to pan out of the corner to continue selection.
Panning again to that corner scrolls another 1/3rd of screen.
Page mode and initial page are restored when highlighting or dismissing the highlight dialog, and a little marker is shown at where selection was started so one does not get lost after all that scrolling and restoring.

Closes #4496. Closes #1575. Closes #2071. Related #4533.

poire-z added some commits Feb 3, 2019

Adds CreDocument:getScreenPositionFromXPointer()
Factorize same code used by ReaderLink and ReaderRolling into
CreDocument:getScreenPositionFromXPointer().
cre: allow text selection/highlighting across pages
Panning to the bottom right corner (or top left corner) switches
to scroll mode and scroll the page forward (resp. backward) 1/3rd
of the screen.
One has to pan out of the corner to continue selection.
Panning again to that corner scrolls another 1/3rd of screen.
Page mode is restored when highlighting or dismissing the highlight
dialog, and a little marker is shown at where selection was started
so one does not get lost after all that scrolling and restoring.

@poire-z poire-z force-pushed the poire-z:select_scroll branch from cf5dfc2 to b3244fd Feb 3, 2019

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Feb 3, 2019

Looks decent to me. Clever hack.

@Galunid

This comment has been minimized.

Copy link
Contributor

Galunid commented Feb 3, 2019

I tested your recent version on my kindle and it works pretty reliably. While 1/8th of the screen is just right when it comes to width, it feels a little too much when it comes to height. On my PW3 it is 3rd line from the bottom. I wonder if we could add this as a separate setting to defaults.lua, so as to let the user have a bit more control.

@poire-z

This comment has been minimized.

Copy link
Contributor Author

poire-z commented Feb 3, 2019

I don't think it needs anything in defaults.lua. Any other area than top left/bottom right corners would feel unnatural, and the excessive width/height does not prevent anything else from working: it just happens when panning, and you're just switched temporarily into scroll mode and back, like hey, you were indeed selecting text on the 3rd line from last, ok, not on the last, but we thought about scrolling you a bit to make precise selection easier.
I don't know much about fingers aerodynamics :) but I guess it may also depend on people fingers, size, habbits, way to hold the device. I guess you're right handed, and select text by tearing horizontally with your finger horizontal (like I do), so a short height does not matter. But for a left handed person, possibly having a left finger vertical moving horizontally on the last line, a short height may be tedious to trigger it (because of the bottom bezel pushing the vertical contact with screen a bit up), while that left-handed guy wouldn't mind a shorter width.

@poire-z

This comment has been minimized.

Copy link
Contributor Author

poire-z commented Feb 3, 2019

OK, merging this. We'll see how people feels about the zones width & height.

@poire-z poire-z merged commit e802b96 into koreader:master Feb 3, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@poire-z poire-z deleted the poire-z:select_scroll branch Feb 3, 2019

@arooni

This comment has been minimized.

Copy link

arooni commented Feb 7, 2019

hey team!

so I love being able to highlight text across multiple pages on my PW, thanks to the most recent nightly.

but one thing i've noticed is that if i "miss" (i..e highlight the wrong bit of text) the software goes back to previous page position.

it seems like the best user experience would be (of course I have no idea how hard to implement this would be so please take your grain of salt):

  1. do not move the user back to the original page position, but rather leave the user on the most recent page highlighted. why? well this allows the user to fix the highlight it's not correct.
  2. to the first point, it would be useful if i could adjust the start/stop point of the highlight as i make one. i find it a bit frustrating that i have to constantly delete the highlight and create a new one for where i want it to be.

if # 2 needs to be a separate issue to track please let me know and i can open it

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Feb 7, 2019

As a separate issue it would be easier to track, but it may already exist.

@arooni

This comment has been minimized.

Copy link

arooni commented Feb 7, 2019

i did try doing a search to see if someone had suggested it before. but didn't see anything so opened #4555 to track it is a FR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.