Skip to content

Commit 6f8a666

Browse files
mfl28mfl28
authored
Merge pull request #78 from mfl28/bugfix/fix-partial-navigation-key-combination-release-handling
Fix partial navigation key-combination release handling
2 parents 0fcad1e + 75371ee commit 6f8a666

File tree

4 files changed

+110
-15
lines changed

4 files changed

+110
-15
lines changed

‎src/main/java/com/github/mfl28/boundingboxeditor/controller/Controller.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ public void onRegisterSceneKeyPressed(KeyEvent event) {
509509
}
510510

511511
keyCombinationHandlers.stream()
512-
.filter(keyCombinationHandler -> keyCombinationHandler.getKeyCombination().match(event) && keyCombinationHandler.hasOnPressedHandler())
512+
.filter(keyCombinationHandler -> keyCombinationHandler.handlesPressed(event))
513513
.findFirst()
514514
.ifPresent(keyCombinationEventHandler -> keyCombinationEventHandler.onPressed(event));
515515
}
@@ -525,7 +525,7 @@ public void onRegisterSceneKeyReleased(KeyEvent event) {
525525
}
526526

527527
keyCombinationHandlers.stream()
528-
.filter(keyCombinationHandler -> keyCombinationHandler.getKeyCombination().match(event) && keyCombinationHandler.hasOnReleasedHandler())
528+
.filter(keyCombinationHandler -> keyCombinationHandler.handlesReleased(event))
529529
.findFirst()
530530
.ifPresent(keyCombinationEventHandler -> keyCombinationEventHandler.onReleased(event));
531531
}
@@ -674,7 +674,7 @@ public void onRegisterAboutAction() {
674674
"About " + PROGRAM_NAME,
675675
PROGRAM_NAME,
676676
"Version: " + PROGRAM_VERSION +
677-
"\nLicense: " + PROGRAM_LICENSE,
677+
"\nLicense: " + PROGRAM_LICENSE,
678678
stage);
679679
}
680680

@@ -739,10 +739,16 @@ void initiateAnnotationExport(File destination, ImageAnnotationSaveStrategy.Type
739739

740740
private List<KeyCombinationEventHandler> createKeyCombinationHandlers() {
741741
return List.of(
742+
new KeyCombinationEventHandler(KeyCombination.NO_MATCH, null,
743+
event -> {
744+
navigatePreviousKeyPressed.set(false);
745+
navigateNextKeyPressed.set(false);
746+
},
747+
event -> KeyCombinations.navigationReleaseKeyCodes.contains(event.getCode())),
742748
new KeyCombinationEventHandler(KeyCombinations.navigateNext,
743-
event -> handleNavigateNextKeyPressed(), event -> navigateNextKeyPressed.set(false)),
749+
event -> handleNavigateNextKeyPressed(), null),
744750
new KeyCombinationEventHandler(KeyCombinations.navigatePrevious,
745-
event -> handleNavigatePreviousKeyPressed(), event -> navigatePreviousKeyPressed.set(false)),
751+
event -> handleNavigatePreviousKeyPressed(), null),
746752
new SingleFireKeyCombinationEventHandler(KeyCombinations.deleteSelectedBoundingShape,
747753
event -> view.removeSelectedTreeItemAndChildren(), null),
748754
new SingleFireKeyCombinationEventHandler(KeyCombinations.removeEditingVerticesWhenBoundingPolygonSelected,
@@ -1329,7 +1335,7 @@ private void updateViewImageFiles() {
13291335

13301336
private void updateStageTitle() {
13311337
ImageMetaData currentImageMetaData = model.getCurrentImageMetaData();
1332-
stage.setTitle(PROGRAM_IDENTIFIER + PROGRAM_NAME_EXTENSION_SEPARATOR
1338+
stage.setTitle(PROGRAM_IDENTIFIER + PROGRAM_NAME_EXTENSION_SEPARATOR
13331339
+ model.getCurrentImageFilePath() + " " + currentImageMetaData.getDimensionsString());
13341340
}
13351341

@@ -1589,6 +1595,8 @@ public static class KeyCombinations {
15891595
KeyCombination.SHORTCUT_DOWN);
15901596
public static final KeyCombination navigatePrevious = new KeyCodeCombination(KeyCode.A,
15911597
KeyCombination.SHORTCUT_DOWN);
1598+
1599+
public static final List<KeyCode> navigationReleaseKeyCodes = List.of(KeyCode.A, KeyCode.D, KeyCode.CONTROL, KeyCode.META);
15921600
public static final KeyCombination showAllBoundingShapes =
15931601
new KeyCodeCombination(KeyCode.V, KeyCombination.SHORTCUT_DOWN, KeyCombination.ALT_DOWN);
15941602
public static final KeyCombination hideAllBoundingShapes =

‎src/main/java/com/github/mfl28/boundingboxeditor/controller/utils/KeyCombinationEventHandler.java‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,38 @@
2222
import javafx.scene.input.KeyCombination;
2323
import javafx.scene.input.KeyEvent;
2424

25+
import java.util.function.Function;
26+
2527
public class KeyCombinationEventHandler {
2628
private final KeyCombination keyCombination;
2729
private final EventHandler<KeyEvent> onPressedHandler;
2830
private final EventHandler<KeyEvent> onReleasedHandler;
31+
private final Function<KeyEvent, Boolean> releaseMatcher;
2932

30-
public KeyCombinationEventHandler(KeyCombination keyCombination, EventHandler<KeyEvent> onPressedHandler, EventHandler<KeyEvent> onReleasedHandler) {
33+
public KeyCombinationEventHandler(
34+
KeyCombination keyCombination,
35+
EventHandler<KeyEvent> onPressedHandler,
36+
EventHandler<KeyEvent> onReleasedHandler,
37+
Function<KeyEvent, Boolean> releaseMatcher) {
3138
this.keyCombination = keyCombination;
3239
this.onPressedHandler = onPressedHandler;
3340
this.onReleasedHandler = onReleasedHandler;
41+
this.releaseMatcher = releaseMatcher;
42+
}
43+
44+
public KeyCombinationEventHandler(
45+
KeyCombination keyCombination,
46+
EventHandler<KeyEvent> onPressedHandler,
47+
EventHandler<KeyEvent> onReleasedHandler) {
48+
this(
49+
keyCombination,
50+
onPressedHandler,
51+
onReleasedHandler,
52+
null
53+
);
3454
}
3555

56+
3657
public KeyCombination getKeyCombination() {
3758
return keyCombination;
3859
}
@@ -56,4 +77,24 @@ public void onReleased(KeyEvent event) {
5677
this.onReleasedHandler.handle(event);
5778
}
5879
}
80+
81+
public boolean matchPressed(KeyEvent event) {
82+
return keyCombination.match(event);
83+
}
84+
85+
public boolean matchReleased(KeyEvent event) {
86+
if(releaseMatcher != null) {
87+
return releaseMatcher.apply(event);
88+
} else {
89+
return keyCombination.match(event);
90+
}
91+
}
92+
93+
public boolean handlesPressed(KeyEvent event) {
94+
return hasOnPressedHandler() && matchPressed(event);
95+
}
96+
97+
public boolean handlesReleased(KeyEvent event) {
98+
return hasOnReleasedHandler() && matchReleased(event);
99+
}
59100
}

‎src/test/java/com/github/mfl28/boundingboxeditor/controller/SceneKeyShortcutTests.java‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import javafx.application.Platform;
2525
import javafx.geometry.Point2D;
2626
import javafx.scene.input.KeyCode;
27+
import javafx.scene.input.KeyCombination;
2728
import javafx.scene.input.KeyEvent;
2829
import javafx.scene.input.MouseButton;
2930
import javafx.stage.Stage;
@@ -57,6 +58,7 @@ void onSceneKeyPressed_ShouldPerformCorrectAction(TestInfo testinfo, FxRobot rob
5758

5859
verifyThat(controller.keyCombinationHandlers.stream().map(KeyCombinationEventHandler::getKeyCombination).toList(),
5960
Matchers.containsInAnyOrder(
61+
KeyCombination.NO_MATCH,
6062
Controller.KeyCombinations.navigateNext, Controller.KeyCombinations.navigatePrevious,
6163
Controller.KeyCombinations.showAllBoundingShapes, Controller.KeyCombinations.hideAllBoundingShapes,
6264
Controller.KeyCombinations.showSelectedBoundingShape, Controller.KeyCombinations.hideSelectedBoundingShape,
@@ -69,8 +71,12 @@ void onSceneKeyPressed_ShouldPerformCorrectAction(TestInfo testinfo, FxRobot rob
6971
Controller.KeyCombinations.hideNonSelectedBoundingShapes, Controller.KeyCombinations.simplifyPolygon
7072
));
7173

72-
testNavigateNextKeyEvent(testinfo);
73-
testNavigatePreviousKeyEvent(testinfo);
74+
testNavigateNextKeyEvent(testinfo, true, true, "wexor-tmg-L-2p8fapOA8-unsplash.jpg");
75+
testNavigatePreviousKeyEvent(testinfo, true, true, "rachel-hisko-rEM3cK8F1pk-unsplash.jpg");
76+
testNavigateNextKeyEvent(testinfo, false, true, "wexor-tmg-L-2p8fapOA8-unsplash.jpg");
77+
testNavigatePreviousKeyEvent(testinfo, false, true, "rachel-hisko-rEM3cK8F1pk-unsplash.jpg");
78+
testNavigateNextKeyEvent(testinfo, true, false, "wexor-tmg-L-2p8fapOA8-unsplash.jpg");
79+
testNavigatePreviousKeyEvent(testinfo, true, false, "rachel-hisko-rEM3cK8F1pk-unsplash.jpg");
7480
testSelectFreehandDrawingModeKeyEvent();
7581
testSelectRectangleModeKeyEvent();
7682
testFocusCategorySearchFieldKeyEvent();
@@ -307,34 +313,38 @@ private void testSelectFreehandDrawingModeKeyEvent() {
307313
verifyThat(controller.getView().getEditor().getEditorToolBar().getFreehandModeButton().isSelected(), Matchers.is(true));
308314
}
309315

310-
private void testNavigatePreviousKeyEvent(TestInfo testinfo) {
316+
private void testNavigatePreviousKeyEvent(TestInfo testinfo, boolean keyReleased, boolean ctrlReleased, String expectedTargetImageName) {
311317
KeyEvent navigatePreviousPressedEvent = new KeyEvent(KeyEvent.KEY_PRESSED, "", "", KeyCode.A, false, true, false, false);
312318

313319
Platform.runLater(() -> controller.onRegisterSceneKeyPressed(navigatePreviousPressedEvent));
314320
WaitForAsyncUtils.waitForFxEvents();
315321

316-
KeyEvent navigatePreviousReleasedEvent = new KeyEvent(KeyEvent.KEY_RELEASED, "", "", KeyCode.A, false, true, false, false);
322+
KeyEvent navigatePreviousReleasedEvent = new KeyEvent(KeyEvent.KEY_RELEASED, "", "",
323+
keyReleased ? KeyCode.A : KeyCode.CONTROL, false, ctrlReleased, false, false);
317324
Platform.runLater(() -> controller.onRegisterSceneKeyReleased(navigatePreviousReleasedEvent));
318325
WaitForAsyncUtils.waitForFxEvents();
319326

320327
waitUntilCurrentImageIsLoaded(testinfo);
321328

322-
verifyThat(model.getCurrentImageFile().getName(), Matchers.equalTo("rachel-hisko-rEM3cK8F1pk-unsplash.jpg"));
329+
verifyThat(model.getCurrentImageFile().getName(), Matchers.equalTo(
330+
expectedTargetImageName));
323331
}
324332

325-
private void testNavigateNextKeyEvent(TestInfo testinfo) {
333+
private void testNavigateNextKeyEvent(TestInfo testinfo, boolean keyReleased, boolean ctrlReleased, String expectedTargetImageName) {
326334
KeyEvent navigateNextPressedEvent = new KeyEvent(KeyEvent.KEY_PRESSED, "", "", KeyCode.D, false, true, false, false);
327335

328336
Platform.runLater(() -> controller.onRegisterSceneKeyPressed(navigateNextPressedEvent));
329337
WaitForAsyncUtils.waitForFxEvents();
330338

331-
KeyEvent navigateNextReleasedEvent = new KeyEvent(KeyEvent.KEY_RELEASED, "", "", KeyCode.D, false, true, false, false);
339+
KeyEvent navigateNextReleasedEvent = new KeyEvent(KeyEvent.KEY_RELEASED, "", "",
340+
keyReleased ? KeyCode.D : KeyCode.CONTROL, false, ctrlReleased, false, false);
332341
Platform.runLater(() -> controller.onRegisterSceneKeyReleased(navigateNextReleasedEvent));
333342
WaitForAsyncUtils.waitForFxEvents();
334343

335344
waitUntilCurrentImageIsLoaded(testinfo);
336345

337-
verifyThat(model.getCurrentImageFile().getName(), Matchers.equalTo("wexor-tmg-L-2p8fapOA8-unsplash.jpg"));
346+
verifyThat(model.getCurrentImageFile().getName(), Matchers.equalTo(
347+
expectedTargetImageName));
338348
}
339349

340350
}

‎src/test/java/com/github/mfl28/boundingboxeditor/controller/utils/KeyCombinationEventHandlerTests.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import javafx.scene.input.KeyCode;
2222
import javafx.scene.input.KeyCodeCombination;
2323
import javafx.scene.input.KeyCombination;
24+
import javafx.scene.input.KeyEvent;
2425
import org.junit.jupiter.api.Assertions;
2526
import org.junit.jupiter.api.Tag;
2627
import org.junit.jupiter.api.Test;
@@ -32,6 +33,8 @@ class KeyCombinationEventHandlerTests {
3233
@Test
3334
void checkKeyCombinationEventHandlerWithNonNullHandlers() {
3435
KeyCodeCombination testCombination = new KeyCodeCombination(KeyCode.A, KeyCombination.SHIFT_DOWN);
36+
KeyEvent trueEvent = new KeyEvent(KeyEvent.KEY_PRESSED, "", "", KeyCode.A, true, false, false, false);
37+
KeyEvent falseEvent = new KeyEvent(KeyEvent.KEY_PRESSED, "", "", KeyCode.B, true, false, false, false);
3538

3639
AtomicInteger numPressedHandled = new AtomicInteger();
3740
AtomicInteger numReleasedHandled = new AtomicInteger();
@@ -45,6 +48,10 @@ void checkKeyCombinationEventHandlerWithNonNullHandlers() {
4548
Assertions.assertTrue(keyCombinationEventHandler.hasOnPressedHandler());
4649
Assertions.assertTrue(keyCombinationEventHandler.hasOnReleasedHandler());
4750
Assertions.assertEquals(keyCombinationEventHandler.getKeyCombination(), testCombination);
51+
Assertions.assertTrue(keyCombinationEventHandler.handlesPressed(trueEvent));
52+
Assertions.assertTrue(keyCombinationEventHandler.handlesReleased(trueEvent));
53+
Assertions.assertFalse(keyCombinationEventHandler.handlesPressed(falseEvent));
54+
Assertions.assertFalse(keyCombinationEventHandler.handlesReleased(falseEvent));
4855

4956
keyCombinationEventHandler.onPressed(null);
5057
Assertions.assertEquals(1, numPressedHandled.get());
@@ -59,6 +66,8 @@ void checkKeyCombinationEventHandlerWithNonNullHandlers() {
5966
@Test
6067
void checkKeyCombinationEventHandlerWithNullHandlers() {
6168
KeyCodeCombination testCombination = new KeyCodeCombination(KeyCode.A, KeyCombination.SHIFT_DOWN);
69+
KeyEvent trueEvent = new KeyEvent(KeyEvent.KEY_PRESSED, "", "", KeyCode.A, true, false, false, false);
70+
KeyEvent falseEvent = new KeyEvent(KeyEvent.KEY_PRESSED, "", "", KeyCode.B, true, false, false, false);
6271

6372
KeyCombinationEventHandler keyCombinationEventHandler = new KeyCombinationEventHandler(
6473
testCombination, null, null
@@ -67,11 +76,38 @@ void checkKeyCombinationEventHandlerWithNullHandlers() {
6776
Assertions.assertFalse(keyCombinationEventHandler.hasOnPressedHandler());
6877
Assertions.assertFalse(keyCombinationEventHandler.hasOnReleasedHandler());
6978
Assertions.assertEquals(keyCombinationEventHandler.getKeyCombination(), testCombination);
79+
Assertions.assertFalse(keyCombinationEventHandler.handlesPressed(trueEvent));
80+
Assertions.assertFalse(keyCombinationEventHandler.handlesReleased(trueEvent));
81+
Assertions.assertFalse(keyCombinationEventHandler.handlesPressed(falseEvent));
82+
Assertions.assertFalse(keyCombinationEventHandler.handlesReleased(falseEvent));
83+
7084

7185
keyCombinationEventHandler.onPressed(null);
7286
keyCombinationEventHandler.onReleased(null);
7387
}
7488

89+
@Test
90+
void checkKeyCombinationEventHandlerWithNonNullReleaseMatcher() {
91+
KeyCodeCombination testCombination = new KeyCodeCombination(KeyCode.A, KeyCombination.SHIFT_DOWN);
92+
KeyEvent trueEvent = new KeyEvent(KeyEvent.KEY_PRESSED, "", "", KeyCode.A, true, false, false, false);
93+
KeyEvent falseEvent = new KeyEvent(KeyEvent.KEY_PRESSED, "", "", KeyCode.B, true, false, false, false);
94+
95+
AtomicInteger numPressedHandled = new AtomicInteger();
96+
AtomicInteger numReleasedHandled = new AtomicInteger();
97+
98+
KeyCombinationEventHandler keyCombinationEventHandler = new KeyCombinationEventHandler(
99+
testCombination,
100+
event -> numPressedHandled.addAndGet(1),
101+
event -> numReleasedHandled.addAndGet(1),
102+
event -> event.getCode() == KeyCode.B
103+
);
104+
105+
Assertions.assertTrue(keyCombinationEventHandler.handlesPressed(trueEvent));
106+
Assertions.assertFalse(keyCombinationEventHandler.handlesReleased(trueEvent));
107+
Assertions.assertFalse(keyCombinationEventHandler.handlesPressed(falseEvent));
108+
Assertions.assertTrue(keyCombinationEventHandler.handlesReleased(falseEvent));
109+
}
110+
75111
@Test
76112
void checkSingleFireKeyCombinationEventHandlerWithNonNullHandlers() {
77113
KeyCodeCombination testCombination = new KeyCodeCombination(KeyCode.A, KeyCombination.SHIFT_DOWN);

0 commit comments

Comments
 (0)