From ad2a9f43c836728c533c98cbbc0a361792199bc9 Mon Sep 17 00:00:00 2001 From: Jurgen Date: Fri, 17 May 2019 14:54:26 +0200 Subject: [PATCH 1/6] Changed page up and down behaviour --- .../fxmisc/richtext/GenericStyledArea.java | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java b/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java index 7e6c5c7ac..d40d839b0 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,29 @@ 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 ); + } + + private void page(int direction, 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( direction * 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 +1258,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; }); } From b410f4818493627410d045e3632f947b52614c56 Mon Sep 17 00:00:00 2001 From: Jurgen Date: Sun, 19 May 2019 12:19:25 +0200 Subject: [PATCH 2/6] Changed page up and down behaviour --- .../main/java/org/fxmisc/richtext/GenericStyledArea.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java b/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java index d40d839b0..614a95355 100644 --- a/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java +++ b/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java @@ -1117,13 +1117,17 @@ public void nextPage(SelectionPolicy selectionPolicy) { page( +1, selectionPolicy ); } - private void page(int direction, SelectionPolicy 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( direction * getViewportHeight() ); + 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() ) From 7bbed73e700a1eca0eeca1e62438d6d2826f019d Mon Sep 17 00:00:00 2001 From: Jurgen Date: Sun, 19 May 2019 12:20:05 +0200 Subject: [PATCH 3/6] Revised tests for new page up/down behaviour --- .../org/fxmisc/richtext/api/HitTests.java | 8 +-- .../richtext/keyboard/PageUpDownTests.java | 49 +++++++++++-------- 2 files changed, 33 insertions(+), 24 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..aa53e20f6 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()+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()-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..3d938cc01 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()); + Platform.runLater(() -> { + 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()); + Platform.runLater(() -> { + 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()); + Platform.runLater(() -> { + 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,10 +94,12 @@ 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()); + Platform.runLater(() -> { + 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()); + }); } } From 1f42153d52fa549b5b9cb71aca9ece6636637ad8 Mon Sep 17 00:00:00 2001 From: Jurgen Date: Sun, 19 May 2019 12:28:26 +0200 Subject: [PATCH 4/6] Changed page up and down behaviour --- .../fxmisc/richtext/GenericStyledArea.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java b/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java index 614a95355..d463571c0 100644 --- a/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java +++ b/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java @@ -1123,19 +1123,19 @@ public void nextPage(SelectionPolicy selectionPolicy) { */ 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(); + // 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 ) ); + 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 ) ) ); + // 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 From fcc835e4726167090c7a5d17079e40d2c35d1960 Mon Sep 17 00:00:00 2001 From: Jurgen Date: Mon, 20 May 2019 12:42:27 +0200 Subject: [PATCH 5/6] Revised tests for new page up/down behaviour --- .../org/fxmisc/richtext/api/HitTests.java | 4 ++-- .../richtext/keyboard/PageUpDownTests.java | 23 +++++++++++++++---- 2 files changed, 21 insertions(+), 6 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 aa53e20f6..d6a5f1b9c 100644 --- a/richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java +++ b/richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java @@ -151,7 +151,7 @@ public void prev_page_leaves_caret_at_bottom_of_page() { }); assertEquals(0, area.getCaretColumn()); - assertEquals(area.lastVisibleParToAllParIndex(), area.getCurrentParagraph()+1); + assertEquals(area.lastVisibleParToAllParIndex(), area.getCurrentParagraph() + (IS_WINDOWS?1:0)); } @Test @@ -165,7 +165,7 @@ public void next_page_leaves_caret_at_top_of_page() { }); assertEquals(0, area.getCaretColumn()); - assertEquals(area.firstVisibleParToAllParIndex(), area.getCurrentParagraph()-1); + assertEquals(area.firstVisibleParToAllParIndex(), area.getCurrentParagraph() - (IS_WINDOWS?1:0)); } } 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 3d938cc01..7943f5d91 100644 --- a/richtextfx/src/integrationTest/java/org/fxmisc/richtext/keyboard/PageUpDownTests.java +++ b/richtextfx/src/integrationTest/java/org/fxmisc/richtext/keyboard/PageUpDownTests.java @@ -37,7 +37,7 @@ public void page_up_leaves_caret_at_bottom_of_viewport() { type(PAGE_UP); - Platform.runLater(() -> { + runLater( 150, () -> { Bounds afterBounds = area.getCaretBounds().get(); assertEquals(8, area.getCaretPosition()); assertTrue(area.getSelectedText().isEmpty()); @@ -56,7 +56,7 @@ public void page_down_leaves_caret_at_top_of_viewport() throws Exception { type(PAGE_DOWN); - Platform.runLater(() -> { + runLater( 150, () -> { Bounds afterBounds = area.getCaretBounds().get(); assertEquals(area.getAbsolutePosition(3, 0), area.getCaretPosition()); assertTrue(area.getSelectedText().isEmpty()); @@ -75,7 +75,7 @@ public void shift_page_up_leaves_caret_at_bottom_of_viewport_and_makes_selection press(SHIFT).type(PAGE_UP).release(SHIFT); - Platform.runLater(() -> { + runLater( 150, () -> { Bounds afterBounds = area.getCaretBounds().get(); assertEquals(8, area.getCaretPosition()); assertEquals(area.getText(0, 0, 5, 0), area.getSelectedText()); @@ -94,7 +94,7 @@ public void shift_page_down_leaves_caret_at_top_of_viewport_and_makes_selection( press(SHIFT).type(PAGE_DOWN).release(SHIFT); - Platform.runLater(() -> { + runLater( 150, () -> { Bounds afterBounds = area.getCaretBounds().get(); assertEquals(area.getAbsolutePosition(3, 0), area.getCaretPosition()); assertEquals(area.getText(0, 0, 3, 0), area.getSelectedText()); @@ -102,5 +102,20 @@ public void shift_page_down_leaves_caret_at_top_of_viewport_and_makes_selection( }); } + // 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(); + } } From 9d27853be499174feb4c1888802ec98dec9a3029 Mon Sep 17 00:00:00 2001 From: Jurgen Date: Mon, 20 May 2019 13:05:31 +0200 Subject: [PATCH 6/6] Revised Mac hit tests for new page up/down behaviour --- .../java/org/fxmisc/richtext/api/HitTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 d6a5f1b9c..3a8e712b1 100644 --- a/richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java +++ b/richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java @@ -151,7 +151,7 @@ public void prev_page_leaves_caret_at_bottom_of_page() { }); assertEquals(0, area.getCaretColumn()); - assertEquals(area.lastVisibleParToAllParIndex(), area.getCurrentParagraph() + (IS_WINDOWS?1:0)); + assertEquals(area.lastVisibleParToAllParIndex(), area.getCurrentParagraph() + (IS_LINUX?0:1)); } @Test @@ -165,7 +165,7 @@ public void next_page_leaves_caret_at_top_of_page() { }); assertEquals(0, area.getCaretColumn()); - assertEquals(area.firstVisibleParToAllParIndex(), area.getCurrentParagraph() - (IS_WINDOWS?1:0)); + assertEquals(area.firstVisibleParToAllParIndex(), area.getCurrentParagraph() - (IS_LINUX?0:1)); } }