Skip to content

RFE #688 Changed page up and down behaviour #821

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.

Sign up for GitHub

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

Merged
Jugen merged 6 commits into from
May 20, 2019
Merged

RFE #688 Changed page up and down behaviour #821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ad2a9f4
Changed page up and down behaviour
May 17, 2019
b410f48
Changed page up and down behaviour
May 19, 2019
7bbed73
Revised tests for new page up/down behaviour
May 19, 2019
1f42153
Changed page up and down behaviour
May 19, 2019
fcc835e
Revised tests for new page up/down behaviour
May 20, 2019
9d27853
Revised Mac hit tests for new page up/down behaviour
May 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void clicking_character_should_move_caret_to_that_position()
}

@Test
public void prev_page_moves_caret_to_top_of_page() {
public void prev_page_leaves_caret_at_bottom_of_page() {
area.showParagraphAtBottom(area.getParagraphs().size() - 1);
// move to last line, column 0
area.moveTo(area.getParagraphs().size() - 1, 0);
Expand All @@ -151,11 +151,11 @@ public void prev_page_moves_caret_to_top_of_page() {
});

assertEquals(0, area.getCaretColumn());
assertEquals(area.firstVisibleParToAllParIndex(), area.getCurrentParagraph());
assertEquals(area.lastVisibleParToAllParIndex(), area.getCurrentParagraph() + (IS_LINUX?0:1));
}

@Test
public void next_page_moves_caret_to_bottom_of_page() {
public void next_page_leaves_caret_at_top_of_page() {
area.showParagraphAtTop(0);

interact(() -> {
Expand All @@ -165,7 +165,7 @@ public void next_page_moves_caret_to_bottom_of_page() {
});

assertEquals(0, area.getCaretColumn());
assertEquals(area.lastVisibleParToAllParIndex(), area.getCurrentParagraph());
assertEquals(area.firstVisibleParToAllParIndex(), area.getCurrentParagraph() - (IS_LINUX?0:1));
}

}
Expand Down
64 changes: 44 additions & 20 deletions richtextfx/src/integrationTest/java/org/fxmisc/richtext/keyboard/PageUpDownTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.fxmisc.richtext.keyboard;

import javafx.application.Platform;
import javafx.geometry.Bounds;
import javafx.stage.Stage;
import org.fxmisc.richtext.InlineCssTextAreaAppTest;
Expand All @@ -26,7 +27,7 @@ public void start(Stage stage) throws Exception {
}

@Test
public void page_up_moves_caret_to_top_of_viewport() {
public void page_up_leaves_caret_at_bottom_of_viewport() {
interact(() -> {
area.moveTo(5, 0);
area.requestFollowCaret();
Expand All @@ -36,14 +37,16 @@ public void page_up_moves_caret_to_top_of_viewport() {

type(PAGE_UP);

Bounds afterBounds = area.getCaretBounds().get();
assertEquals(0, area.getCaretPosition());
assertTrue(area.getSelectedText().isEmpty());
assertTrue(beforeBounds.getMinY() > afterBounds.getMinY());
runLater( 150, () -> {
Bounds afterBounds = area.getCaretBounds().get();
assertEquals(8, area.getCaretPosition());
assertTrue(area.getSelectedText().isEmpty());
assertTrue(beforeBounds.getMinY() > afterBounds.getMinY());
});
}

@Test
public void page_down_moves_caret_to_bottom_of_viewport() throws Exception {
public void page_down_leaves_caret_at_top_of_viewport() throws Exception {
interact(() -> {
area.moveTo(0);
area.requestFollowCaret();
Expand All @@ -53,14 +56,16 @@ public void page_down_moves_caret_to_bottom_of_viewport() throws Exception {

type(PAGE_DOWN);

Bounds afterBounds = area.getCaretBounds().get();
assertEquals(area.getAbsolutePosition(5, 0), area.getCaretPosition());
assertTrue(area.getSelectedText().isEmpty());
assertTrue(beforeBounds.getMinY() < afterBounds.getMinY());
runLater( 150, () -> {
Bounds afterBounds = area.getCaretBounds().get();
assertEquals(area.getAbsolutePosition(3, 0), area.getCaretPosition());
assertTrue(area.getSelectedText().isEmpty());
assertTrue(beforeBounds.getMinY() < afterBounds.getMinY());
});
}

@Test
public void shift_page_up_moves_caret_to_top_of_viewport_and_makes_selection() {
public void shift_page_up_leaves_caret_at_bottom_of_viewport_and_makes_selection() {
interact(() -> {
area.moveTo(5, 0);
area.requestFollowCaret();
Expand All @@ -70,14 +75,16 @@ public void shift_page_up_moves_caret_to_top_of_viewport_and_makes_selection() {

press(SHIFT).type(PAGE_UP).release(SHIFT);

Bounds afterBounds = area.getCaretBounds().get();
assertEquals(0, area.getCaretPosition());
assertEquals(area.getText(0, 0, 5, 0), area.getSelectedText());
assertTrue(beforeBounds.getMinY() > afterBounds.getMinY());
runLater( 150, () -> {
Bounds afterBounds = area.getCaretBounds().get();
assertEquals(8, area.getCaretPosition());
assertEquals(area.getText(0, 0, 5, 0), area.getSelectedText());
assertTrue(beforeBounds.getMinY() > afterBounds.getMinY());
});
}

@Test
public void shift_page_down_moves_caret_to_bottom_of_viewport_and_makes_selection() {
public void shift_page_down_leaves_caret_at_top_of_viewport_and_makes_selection() {
interact(() -> {
area.moveTo(0);
area.requestFollowCaret();
Expand All @@ -87,11 +94,28 @@ public void shift_page_down_moves_caret_to_bottom_of_viewport_and_makes_selectio

press(SHIFT).type(PAGE_DOWN).release(SHIFT);

Bounds afterBounds = area.getCaretBounds().get();
assertEquals(area.getAbsolutePosition(5, 0), area.getCaretPosition());
assertEquals(area.getText(0, 0, 5, 0), area.getSelectedText());
assertTrue(beforeBounds.getMinY() < afterBounds.getMinY());
runLater( 150, () -> {
Bounds afterBounds = area.getCaretBounds().get();
assertEquals(area.getAbsolutePosition(3, 0), area.getCaretPosition());
assertEquals(area.getText(0, 0, 3, 0), area.getSelectedText());
assertTrue(beforeBounds.getMinY() < afterBounds.getMinY());
});
}

// Can't use sleep( t ) as that seems to delay the key press & release actions as well,
// defeating the purpose of it. So here a new thread is created for the delay ...
private void runLater( final long time, final Runnable runFX )
{
new Thread( () -> {
long t0 = System.currentTimeMillis();
long t1 = t0 + time;

while ( t0 < t1 ) try { Thread.sleep( t1 - t0 ); } catch ( Exception e ) {}
finally { t0 = System.currentTimeMillis(); }

Platform.runLater( runFX );

} ).start();
}
}

36 changes: 27 additions & 9 deletions richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ public final boolean removeSelection(Selection<PS, SEG, S> selection) {
// paragraphs and on the lower level are lines within a paragraph
private final TwoLevelNavigator paragraphLineNavigator;

private boolean followCaretRequested = false;
private boolean paging, followCaretRequested = false;

/* ********************************************************************** *
* *
Expand Down Expand Up @@ -1109,16 +1109,33 @@ public void lineEnd(SelectionPolicy policy) {

@Override
public void prevPage(SelectionPolicy selectionPolicy) {
showCaretAtBottom();
CharacterHit hit = hit(getTargetCaretOffset(), 1.0);
moveTo(hit.getInsertionIndex(), selectionPolicy);
page( -1, selectionPolicy );
}

@Override
public void nextPage(SelectionPolicy selectionPolicy) {
showCaretAtTop();
CharacterHit hit = hit(getTargetCaretOffset(), getViewportHeight() - 1.0);
moveTo(hit.getInsertionIndex(), selectionPolicy);
page( +1, selectionPolicy );
}

/**
* @param pgCount the number of pages to page up/down.
* <br>Negative numbers for paging up and positive for down.
*/
private void page(int pgCount, SelectionPolicy selectionPolicy)
{
// Use underlying caret to get the same behaviour as navigating up/down a line where the x position is sticky
Optional<Bounds> cb = caretSelectionBind.getUnderlyingCaret().getCaretBounds();

paging = true; // Prevent scroll from reverting back to the current caret position
scrollYBy( pgCount * getViewportHeight() );

cb.map( this::screenToLocal ) // Place caret near the same on screen position as before
.map( b -> hit( b.getMinX(), b.getMinY()+b.getHeight()/2.0 ).getInsertionIndex() )
.ifPresent( i -> caretSelectionBind.moveTo( i, selectionPolicy ) );

// Adjust scroll by a few pixels to get the caret at the exact on screen location as before
cb.ifPresent( prev -> getCaretBounds().map( newB -> newB.getMinY() - prev.getMinY() )
.filter( delta -> delta != 0.0 ).ifPresent( delta -> scrollYBy( delta ) ) );
}

@Override
Expand Down Expand Up @@ -1245,12 +1262,13 @@ protected void layoutChildren() {
getWidth() - ins.getLeft() - ins.getRight(),
getHeight() - ins.getTop() - ins.getBottom());

if(followCaretRequested) {
followCaretRequested = false;
if(followCaretRequested && ! paging) {
try (Guard g = viewportDirty.suspend()) {
followCaret();
}
}
followCaretRequested = false;
paging = false;
});
}

Expand Down
⚡