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

Support scrolling with page up and page down. #267

Open
wants to merge 2 commits into
base: mac
Choose a base branch
from

Conversation

Lukas-Stuehrk
Copy link

Not deleteToEndOfParagraph: yet ;)

This change implements scrolling via the page up and page down keys. The behavior is the same like in standard text views on macOS: it only scrolls the contents, but doesn't move the cursor.

Copy link
Owner

@simonbs simonbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lukas-Stuehrk Thanks for taking the time to create this PR.

Unfortunately, implementing support for scrolling a page up and down is a bit more elaborate than what's initially suggested in this PR.

Runestone will only typeset and layout the lines that are within the viewport unless it is explicitly asked to layout more lines. The content size is not correct until all lines have been laid out. A consequence of this is that we need to explicitly layout the lines within the rect to scroll to using the LayoutManager.

The issue is illustrated in the video below. Notice that when attempting to scroll a page down, the text is sometimes only scrolled a few hundred pixels.

video.mov

@Lukas-Stuehrk
Copy link
Author

@simonbs Sorry, my bad. I was kind of expecting this, but when I tested it, I only tested with edited files and did not catch it.

I updated the implementation and it should work now. There's one thing which is rather inelegant:

if newViewPort.maxY > scrollView.contentSize.height {
newViewPort = CGRect(origin: CGPoint(x: newOffset.x, y: scrollView.contentSize.height - frame.size.height), size: frame.size)

I'm reading the scrollview's content size instead of using the ContentSizeService. But the scroll view is updated based on the contents, so it works for now.

@simonbs
Copy link
Owner

simonbs commented Feb 16, 2023

@Lukas-Stuehrk Thanks for revisiting these changes.

I'm afraid that there are still some issues though. See the attached video. From seconds 7 to 8, I attempt to scroll a page down but it only scrolls 2/3 of a page down or so.

I'm afraid that the changes needed are quite elaborate, however, I have an idea as to how it can be implemented. I'll be happy to have a look at this a bit further down the road when a few more of the basics in the Mac app are in place 😊

video.mov

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.

2 participants