Skip to content

Commit

Permalink
RFE #688 Changed page up and down behaviour (#821)
Browse files Browse the repository at this point in the history 
Changed page up and down behavior and revised tests accordingly
  • Loading branch information
@Jugen
Jugen authored May 20, 2019
1 parent f3e89ff commit 113643f
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 33 deletions.
8 changes: 4 additions & 4 deletions richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java
View file
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
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
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

0 comments on commit 113643f

Please sign in to comment.