Skip to content

Commit 113643f

Browse files
JugenJugen
authored
RFE #688 Changed page up and down behaviour (#821)
Changed page up and down behavior and revised tests accordingly
1 parent f3e89ff commit 113643f

File tree

3 files changed

+75
-33
lines changed

3 files changed

+75
-33
lines changed

‎richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/HitTests.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ public void clicking_character_should_move_caret_to_that_position()
140140
}
141141

142142
@Test
143-
public void prev_page_moves_caret_to_top_of_page() {
143+
public void prev_page_leaves_caret_at_bottom_of_page() {
144144
area.showParagraphAtBottom(area.getParagraphs().size() - 1);
145145
// move to last line, column 0
146146
area.moveTo(area.getParagraphs().size() - 1, 0);
@@ -151,11 +151,11 @@ public void prev_page_moves_caret_to_top_of_page() {
151151
});
152152

153153
assertEquals(0, area.getCaretColumn());
154-
assertEquals(area.firstVisibleParToAllParIndex(), area.getCurrentParagraph());
154+
assertEquals(area.lastVisibleParToAllParIndex(), area.getCurrentParagraph() + (IS_LINUX?0:1));
155155
}
156156

157157
@Test
158-
public void next_page_moves_caret_to_bottom_of_page() {
158+
public void next_page_leaves_caret_at_top_of_page() {
159159
area.showParagraphAtTop(0);
160160

161161
interact(() -> {
@@ -165,7 +165,7 @@ public void next_page_moves_caret_to_bottom_of_page() {
165165
});
166166

167167
assertEquals(0, area.getCaretColumn());
168-
assertEquals(area.lastVisibleParToAllParIndex(), area.getCurrentParagraph());
168+
assertEquals(area.firstVisibleParToAllParIndex(), area.getCurrentParagraph() - (IS_LINUX?0:1));
169169
}
170170

171171
}

‎richtextfx/src/integrationTest/java/org/fxmisc/richtext/keyboard/PageUpDownTests.java

+44-20
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.fxmisc.richtext.keyboard;
22

3+
import javafx.application.Platform;
34
import javafx.geometry.Bounds;
45
import javafx.stage.Stage;
56
import org.fxmisc.richtext.InlineCssTextAreaAppTest;
@@ -26,7 +27,7 @@ public void start(Stage stage) throws Exception {
2627
}
2728

2829
@Test
29-
public void page_up_moves_caret_to_top_of_viewport() {
30+
public void page_up_leaves_caret_at_bottom_of_viewport() {
3031
interact(() -> {
3132
area.moveTo(5, 0);
3233
area.requestFollowCaret();
@@ -36,14 +37,16 @@ public void page_up_moves_caret_to_top_of_viewport() {
3637

3738
type(PAGE_UP);
3839

39-
Bounds afterBounds = area.getCaretBounds().get();
40-
assertEquals(0, area.getCaretPosition());
41-
assertTrue(area.getSelectedText().isEmpty());
42-
assertTrue(beforeBounds.getMinY() > afterBounds.getMinY());
40+
runLater( 150, () -> {
41+
Bounds afterBounds = area.getCaretBounds().get();
42+
assertEquals(8, area.getCaretPosition());
43+
assertTrue(area.getSelectedText().isEmpty());
44+
assertTrue(beforeBounds.getMinY() > afterBounds.getMinY());
45+
});
4346
}
4447

4548
@Test
46-
public void page_down_moves_caret_to_bottom_of_viewport() throws Exception {
49+
public void page_down_leaves_caret_at_top_of_viewport() throws Exception {
4750
interact(() -> {
4851
area.moveTo(0);
4952
area.requestFollowCaret();
@@ -53,14 +56,16 @@ public void page_down_moves_caret_to_bottom_of_viewport() throws Exception {
5356

5457
type(PAGE_DOWN);
5558

56-
Bounds afterBounds = area.getCaretBounds().get();
57-
assertEquals(area.getAbsolutePosition(5, 0), area.getCaretPosition());
58-
assertTrue(area.getSelectedText().isEmpty());
59-
assertTrue(beforeBounds.getMinY() < afterBounds.getMinY());
59+
runLater( 150, () -> {
60+
Bounds afterBounds = area.getCaretBounds().get();
61+
assertEquals(area.getAbsolutePosition(3, 0), area.getCaretPosition());
62+
assertTrue(area.getSelectedText().isEmpty());
63+
assertTrue(beforeBounds.getMinY() < afterBounds.getMinY());
64+
});
6065
}
6166

6267
@Test
63-
public void shift_page_up_moves_caret_to_top_of_viewport_and_makes_selection() {
68+
public void shift_page_up_leaves_caret_at_bottom_of_viewport_and_makes_selection() {
6469
interact(() -> {
6570
area.moveTo(5, 0);
6671
area.requestFollowCaret();
@@ -70,14 +75,16 @@ public void shift_page_up_moves_caret_to_top_of_viewport_and_makes_selection() {
7075

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

73-
Bounds afterBounds = area.getCaretBounds().get();
74-
assertEquals(0, area.getCaretPosition());
75-
assertEquals(area.getText(0, 0, 5, 0), area.getSelectedText());
76-
assertTrue(beforeBounds.getMinY() > afterBounds.getMinY());
78+
runLater( 150, () -> {
79+
Bounds afterBounds = area.getCaretBounds().get();
80+
assertEquals(8, area.getCaretPosition());
81+
assertEquals(area.getText(0, 0, 5, 0), area.getSelectedText());
82+
assertTrue(beforeBounds.getMinY() > afterBounds.getMinY());
83+
});
7784
}
7885

7986
@Test
80-
public void shift_page_down_moves_caret_to_bottom_of_viewport_and_makes_selection() {
87+
public void shift_page_down_leaves_caret_at_top_of_viewport_and_makes_selection() {
8188
interact(() -> {
8289
area.moveTo(0);
8390
area.requestFollowCaret();
@@ -87,11 +94,28 @@ public void shift_page_down_moves_caret_to_bottom_of_viewport_and_makes_selectio
8794

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

90-
Bounds afterBounds = area.getCaretBounds().get();
91-
assertEquals(area.getAbsolutePosition(5, 0), area.getCaretPosition());
92-
assertEquals(area.getText(0, 0, 5, 0), area.getSelectedText());
93-
assertTrue(beforeBounds.getMinY() < afterBounds.getMinY());
97+
runLater( 150, () -> {
98+
Bounds afterBounds = area.getCaretBounds().get();
99+
assertEquals(area.getAbsolutePosition(3, 0), area.getCaretPosition());
100+
assertEquals(area.getText(0, 0, 3, 0), area.getSelectedText());
101+
assertTrue(beforeBounds.getMinY() < afterBounds.getMinY());
102+
});
94103
}
95104

105+
// Can't use sleep( t ) as that seems to delay the key press & release actions as well,
106+
// defeating the purpose of it. So here a new thread is created for the delay ...
107+
private void runLater( final long time, final Runnable runFX )
108+
{
109+
new Thread( () -> {
110+
long t0 = System.currentTimeMillis();
111+
long t1 = t0 + time;
112+
113+
while ( t0 < t1 ) try { Thread.sleep( t1 - t0 ); } catch ( Exception e ) {}
114+
finally { t0 = System.currentTimeMillis(); }
115+
116+
Platform.runLater( runFX );
117+
118+
} ).start();
119+
}
96120
}
97121

‎richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java

+27-9
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ public final boolean removeSelection(Selection<PS, SEG, S> selection) {
557557
// paragraphs and on the lower level are lines within a paragraph
558558
private final TwoLevelNavigator paragraphLineNavigator;
559559

560-
private boolean followCaretRequested = false;
560+
private boolean paging, followCaretRequested = false;
561561

562562
/* ********************************************************************** *
563563
* *
@@ -1109,16 +1109,33 @@ public void lineEnd(SelectionPolicy policy) {
11091109

11101110
@Override
11111111
public void prevPage(SelectionPolicy selectionPolicy) {
1112-
showCaretAtBottom();
1113-
CharacterHit hit = hit(getTargetCaretOffset(), 1.0);
1114-
moveTo(hit.getInsertionIndex(), selectionPolicy);
1112+
page( -1, selectionPolicy );
11151113
}
11161114

11171115
@Override
11181116
public void nextPage(SelectionPolicy selectionPolicy) {
1119-
showCaretAtTop();
1120-
CharacterHit hit = hit(getTargetCaretOffset(), getViewportHeight() - 1.0);
1121-
moveTo(hit.getInsertionIndex(), selectionPolicy);
1117+
page( +1, selectionPolicy );
1118+
}
1119+
1120+
/**
1121+
* @param pgCount the number of pages to page up/down.
1122+
* <br>Negative numbers for paging up and positive for down.
1123+
*/
1124+
private void page(int pgCount, SelectionPolicy selectionPolicy)
1125+
{
1126+
// Use underlying caret to get the same behaviour as navigating up/down a line where the x position is sticky
1127+
Optional<Bounds> cb = caretSelectionBind.getUnderlyingCaret().getCaretBounds();
1128+
1129+
paging = true; // Prevent scroll from reverting back to the current caret position
1130+
scrollYBy( pgCount * getViewportHeight() );
1131+
1132+
cb.map( this::screenToLocal ) // Place caret near the same on screen position as before
1133+
.map( b -> hit( b.getMinX(), b.getMinY()+b.getHeight()/2.0 ).getInsertionIndex() )
1134+
.ifPresent( i -> caretSelectionBind.moveTo( i, selectionPolicy ) );
1135+
1136+
// Adjust scroll by a few pixels to get the caret at the exact on screen location as before
1137+
cb.ifPresent( prev -> getCaretBounds().map( newB -> newB.getMinY() - prev.getMinY() )
1138+
.filter( delta -> delta != 0.0 ).ifPresent( delta -> scrollYBy( delta ) ) );
11221139
}
11231140

11241141
@Override
@@ -1245,12 +1262,13 @@ protected void layoutChildren() {
12451262
getWidth() - ins.getLeft() - ins.getRight(),
12461263
getHeight() - ins.getTop() - ins.getBottom());
12471264

1248-
if(followCaretRequested) {
1249-
followCaretRequested = false;
1265+
if(followCaretRequested && ! paging) {
12501266
try (Guard g = viewportDirty.suspend()) {
12511267
followCaret();
12521268
}
12531269
}
1270+
followCaretRequested = false;
1271+
paging = false;
12541272
});
12551273
}
12561274

0 commit comments

Comments
 (0)