From 113643fa20da842a4e9f202fad633fb03c6b7933 Mon Sep 17 00:00:00 2001 From: Jurgen Date: Mon, 20 May 2019 13:32:17 +0200 Subject: [PATCH] RFE #688 Changed page up and down behaviour (#821) Changed page up and down behavior and revised tests accordingly --- .../org/fxmisc/richtext/api/HitTests.java | 8 +-- .../richtext/keyboard/PageUpDownTests.java | 64 +++++++++++++------ .../fxmisc/richtext/GenericStyledArea.java | 36 ++++++++--- 3 files changed, 75 insertions(+), 33 deletions(-) diff --git a/richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java b/richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java index 3ba4c2a89..3a8e712b1 100644 --- a/richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java +++ b/richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java @@ -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); @@ -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(() -> { @@ -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)); } } diff --git a/richtextfx/src/integrationTest/java/org/fxmisc/richtext/keyboard/PageUpDownTests.java b/richtextfx/src/integrationTest/java/org/fxmisc/richtext/keyboard/PageUpDownTests.java index 263a8bfa0..7943f5d91 100644 --- a/richtextfx/src/integrationTest/java/org/fxmisc/richtext/keyboard/PageUpDownTests.java +++ b/richtextfx/src/integrationTest/java/org/fxmisc/richtext/keyboard/PageUpDownTests.java @@ -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; @@ -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(); @@ -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(); @@ -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(); @@ -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(); @@ -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(); + } } diff --git a/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java b/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java index 7e6c5c7ac..d463571c0 100644 --- a/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java +++ b/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java @@ -557,7 +557,7 @@ public final boolean removeSelection(Selection 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; /* ********************************************************************** * * * @@ -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. + *
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 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 @@ -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; }); }