On Tue, 31 Oct 2023 19:20:48 GMT, brunesto <d...@openjdk.org> wrote:
> The fix prevents the DatePicker from losing focus if the date is not parsable.
Just preliminary comments, will do a full review once OCA process is completed.
tests/system/src/test/java/test/robot/javafx/scene/DatePickerOnFocusLostTest.java
line 2:
> 1: /*
> 2: * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights
> reserved.
it's a brand new file, I wonder if it should have 2023 only
tests/system/src/test/java/test/robot/javafx/scene/DatePickerOnFocusLostTest.java
line 36:
> 34: import org.junit.Before;
> 35: import org.junit.BeforeClass;
> 36: import org.junit.Test;
could we switch to junit5 please?
tests/system/src/test/java/test/robot/javafx/scene/DatePickerOnFocusLostTest.java
line 98:
> 96: // 3. Click on button to grab the focus and hence attempt to
> datePicker.commitValue()
> 97: // 4. Verify that in case of typo, the value was reverted, as well as
> the editor's text
> 98: public void testDatePickerCommit(boolean typo) throws Exception {
thank you for explaining what the test cases attempt to do!
tests/system/src/test/java/test/robot/javafx/scene/DatePickerOnFocusLostTest.java
line 141:
> 139: // 6 check if onChangeListener was called (already working as
> expected before the patch)
> 140: if (typo)
> 141: Assert.assertEquals(onChangeListenerCalled, 0);
please always use { } in `if` statements
tests/system/src/test/java/test/robot/javafx/scene/DatePickerOnFocusLostTest.java
line 185:
> 183: button = new Button("...");
> 184: root.getChildren().add(button);
> 185:
extra newline (here and elsewhere)
-------------
PR Review: https://git.openjdk.org/jfx/pull/1274#pullrequestreview-1708442005
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1378943077
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1378939425
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1378940060
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1378938085
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1378938680
