Add Page Up and Page Down #1114
Conversation
There was a problem hiding this comment.
Thank you for taking this on!
At the moment the core doesn't actually make the frontend scroll to the new position. So for now nothing is happening. I added some inline comments as to why that is.
I think the easiest way to test the functionality for now is to implement the page_up and page_down in the frontend your are using (or maybe they are already implemented?) and just manually test and play around with it. After that unit tests can be added to ensure that selections and the visible line ranges are correct.
One important thing that should be mentioned here is the distinction between PageUp/PageDown and ScrollPageUp/ScrollPageDown. In the case of scroll page the cursor position is not changed whereas for page up and page down the cursor position actually changes. To make this a bit more clear here some videos:
The following shows the case for ScrollPageUp and ScrollPageDown. Note that the cursor remains on scrollView the entire time while scrolling up and down.
The following is what you are going to implement in this PR, PageUp and PageDown. Note that the cursor actually changes while moving pages up and down.
So what you are currently missing is that the cursor needs to be changed. Another thing is that selection can get collapsed.
It might be interesting to see how different editors implement this behaviour. I looked at XCode and Sublime and there are some minor differences between them.
rust/core-lib/src/view.rs
Outdated
| let scroll_height: i64 = self.scroll_height() as i64; | ||
| match movement { | ||
| Movement::DownPage => { | ||
| self.set_scroll(first_line - scroll_height, first_line); |
There was a problem hiding this comment.
Right now this only updates first_line and height in View but does not tell the frontend to actually scroll to that position. So for now nothing happens.
To make it scroll you have to set self.scroll_to = Some(<position>);
There are some functions in View where this is already used, for example in scroll_to_cursor, which might be helpful.
|
But doesn't xi-editor already implement that? PageUp and PageDown are working on the
|
|
Oh, I see. It seems that the xi-mac frontend doesn't implement it correctly. Yeah, then it makes sense to not change the selections when doing |
|
Yes, so shall I go with implementing this properly ? Like without changing the selection. |
|
Yeah sure, go ahead. |
|
I need some help with getting the cursor to not move. I tried messing around with selections and carets, but couldn't get much out of it. |
|
@scholtzan do you have any pointers for this? |
Addresses #698
I am not sure how to test this. Can someone guide me?