Skip to content

Commit

Permalink
Adjust click-to-select implementation
Browse files Browse the repository at this point in the history
This commit contains a number of fixes. The implementation of file
selection modes [0] worked, but there was an interaction with code
that was written _after_ the PR was written but _before_ the PR
was merged. This change adjusts the behaviour to allow that code to
work properly.

A cancel() method was added to file choosers to allow for closing
them reliably. This is not expected to be useful outside of the test
suite, where ensuring that all file choosers are closed between each
test run is desirable.

This also makes a few adjustments to the test suite to reduce some
fragility.

[0]: #27
Affects: #28
  • Loading branch information
io7m committed May 1, 2021
1 parent c411274 commit 7981434
Show file tree
Hide file tree
Showing 13 changed files with 222 additions and 98 deletions.
Expand Up @@ -57,4 +57,11 @@ public interface JWFileChooserType
*/

List<Path> result();

/**
* If the file chooser is open, then hide it and behave as if the user
* cancelled the selection.
*/

void cancel();
}
Expand Up @@ -22,11 +22,9 @@
import com.io7m.jwheatsheaf.api.JWFileChooserType;
import com.io7m.jwheatsheaf.api.JWFileChoosersType;
import com.io7m.jwheatsheaf.ui.JWFileChoosers;
import javafx.application.Platform;
import javafx.scene.input.KeyCode;
import javafx.stage.Stage;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
import org.junit.jupiter.api.extension.ExtendWith;
Expand All @@ -45,6 +43,8 @@
import java.util.List;
import java.util.stream.Collectors;

import static java.util.concurrent.TimeUnit.SECONDS;

@ExtendWith(ApplicationExtension.class)
public final class JWFileChooserActionSaveTest
{
Expand Down Expand Up @@ -82,6 +82,7 @@ public void stop()
throws IOException
{
this.choosers.close();
this.chooser.cancel();
}

/**
Expand All @@ -105,6 +106,7 @@ public void testActionSaveNamed(

FxAssert.verifyThat(okButton, NodeMatchers.isDisabled());
robot.write("GCC.EXE");
robot.sleep(1L, SECONDS);
FxAssert.verifyThat(okButton, NodeMatchers.isEnabled());

robot.type(KeyCode.ENTER);
Expand Down
Expand Up @@ -16,7 +16,11 @@

package com.io7m.jwheatsheaf.tests;

import com.io7m.jwheatsheaf.api.*;
import com.io7m.jwheatsheaf.api.JWFileChooserAction;
import com.io7m.jwheatsheaf.api.JWFileChooserConfiguration;
import com.io7m.jwheatsheaf.api.JWFileChooserEventType;
import com.io7m.jwheatsheaf.api.JWFileChooserType;
import com.io7m.jwheatsheaf.api.JWFileChoosersType;
import com.io7m.jwheatsheaf.ui.JWFileChoosers;
import javafx.stage.Stage;
import org.junit.jupiter.api.AfterEach;
Expand All @@ -39,7 +43,7 @@
import java.util.List;
import java.util.stream.Collectors;

@SuppressWarnings( {"unused", "SameParameterValue"} )
@SuppressWarnings({"unused", "SameParameterValue"})
@ExtendWith(ApplicationExtension.class)
public final class JWFileChooserDefaultModeTest
{
Expand All @@ -55,14 +59,14 @@ public void start(final Stage stage)

final JWTestFilesystems filesystems = JWTestFilesystems.create();
final var systems = filesystems.filesystems();
final FileSystem dosFilesystem = systems.get( "ExampleDOS" );
final FileSystem dosFilesystem = systems.get("ExampleDOS");

final var configuration =
JWFileChooserConfiguration.builder()
.setAllowDirectoryCreation(true)
.setAction(JWFileChooserAction.OPEN_EXISTING_SINGLE)
.setFileSystem(dosFilesystem)
.build();
.setAllowDirectoryCreation(true)
.setAction(JWFileChooserAction.OPEN_EXISTING_SINGLE)
.setFileSystem(dosFilesystem)
.build();

this.choosers = JWFileChoosers.create();
this.chooser = this.choosers.create(stage, configuration);
Expand All @@ -75,10 +79,12 @@ public void stop()
throws IOException
{
this.choosers.close();
this.chooser.cancel();
}

@AfterEach
public void afterEach() {
public void afterEach()
{
Assertions.assertEquals(0, this.events.size());
}

Expand All @@ -91,7 +97,7 @@ public void test_DefaultMode_ClickOpenButton_SelectedItemReturned(
final FxRobot robot,
final TestInfo info)
{
final var delegate = new JWRobotDelegate( robot);
final var delegate = new JWRobotDelegate(robot);
final var okButton = delegate.getOkButton();

FxAssert.verifyThat(okButton, NodeMatchers.isDisabled());
Expand All @@ -104,13 +110,14 @@ public void test_DefaultMode_ClickOpenButton_SelectedItemReturned(
assertSelected("Z:\\USERS\\GROUCH\\DOC");
}

private void assertSelected(final String... selectedItems) {
private void assertSelected(final String... selectedItems)
{
Assertions.assertEquals(
List.of(selectedItems),
this.chooser.result()
.stream()
.map(Path::toString)
.collect(Collectors.toList())
.stream()
.map(Path::toString)
.collect(Collectors.toList())
);
}
}
Expand Up @@ -16,10 +16,17 @@

package com.io7m.jwheatsheaf.tests;

import com.io7m.jwheatsheaf.api.*;
import com.io7m.jwheatsheaf.api.JWFileChooserAction;
import com.io7m.jwheatsheaf.api.JWFileChooserConfiguration;
import com.io7m.jwheatsheaf.api.JWFileChooserEventType;
import com.io7m.jwheatsheaf.api.JWFileChooserType;
import com.io7m.jwheatsheaf.api.JWFileChoosersType;
import com.io7m.jwheatsheaf.ui.JWFileChoosers;
import javafx.stage.Stage;
import org.junit.jupiter.api.*;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
import org.junit.jupiter.api.extension.ExtendWith;
import org.testfx.api.FxAssert;
import org.testfx.api.FxRobot;
Expand All @@ -36,11 +43,13 @@
import java.util.List;
import java.util.stream.Collectors;

import static java.util.concurrent.TimeUnit.SECONDS;

/**
* Verify that DOC, which represents a directory, may be selected.
*/

@SuppressWarnings( {"unused", "SameParameterValue"} )
@SuppressWarnings({"unused", "SameParameterValue"})
@ExtendWith(ApplicationExtension.class)
public final class JWFileChooserDirectoryModeTest
{
Expand All @@ -56,15 +65,15 @@ public void start(final Stage stage)

final JWTestFilesystems filesystems = JWTestFilesystems.create();
final var systems = filesystems.filesystems();
final FileSystem dosFilesystem = systems.get( "ExampleDOS" );
final FileSystem dosFilesystem = systems.get("ExampleDOS");

final var configuration =
JWFileChooserConfiguration.builder()
.setAllowDirectoryCreation(true)
.setAction(JWFileChooserAction.OPEN_EXISTING_SINGLE)
.setFileSystem(dosFilesystem)
.setFileSelectionMode( path -> path.getFileName().toString().equals( "DOC" ) )
.build();
.setAllowDirectoryCreation(true)
.setAction(JWFileChooserAction.OPEN_EXISTING_SINGLE)
.setFileSystem(dosFilesystem)
.setFileSelectionMode(path -> path.getFileName().toString().equals("DOC"))
.build();

this.choosers = JWFileChoosers.create();
this.chooser = this.choosers.create(stage, configuration);
Expand All @@ -77,10 +86,12 @@ public void stop()
throws IOException
{
this.choosers.close();
this.chooser.cancel();
}

@AfterEach
public void afterEach() {
public void afterEach()
{
Assertions.assertEquals(0, this.events.size());
}

Expand All @@ -94,15 +105,21 @@ public void test_DirectoryMode_SingleClickDirectory_CandidateSelected(
final FxRobot robot,
final TestInfo info)
{
JWFileWindowTitles.setTitle(this.chooser, info);

final var delegate = new JWRobotDelegate(robot);
final var okButton = delegate.getOkButton();

FxAssert.verifyThat(okButton, NodeMatchers.isDisabled());
robot.clickOn(delegate.getDirectoryTable());
robot.sleep(1L, SECONDS);
robot.clickOn(delegate.getTableCellFileName("DOC"));
robot.sleep(1L, SECONDS);

FxAssert.verifyThat(okButton, NodeMatchers.isEnabled());
robot.clickOn(okButton);

assertSelected("Z:\\USERS\\GROUCH\\DOC");
this.assertSelected("Z:\\USERS\\GROUCH\\DOC");
}

/**
Expand All @@ -114,15 +131,21 @@ public void test_DirectoryMode_SingleClickFile_NothingSelected(
final FxRobot robot,
final TestInfo info)
{
JWFileWindowTitles.setTitle(this.chooser, info);

final var delegate = new JWRobotDelegate(robot);
final var okButton = delegate.getOkButton();

FxAssert.verifyThat(okButton, NodeMatchers.isDisabled());
robot.clickOn(delegate.getDirectoryTable());
robot.sleep(1L, SECONDS);
robot.clickOn(delegate.getTableCellFileName("DATA.XML"));
robot.sleep(1L, SECONDS);

FxAssert.verifyThat(okButton, NodeMatchers.isDisabled());
robot.clickOn(okButton);

assertSelected();
this.assertSelected();
}

/**
Expand All @@ -134,25 +157,33 @@ public void test_DirectoryMode_DoubleClickDirectory_DirectoryNavigated(
final FxRobot robot,
final TestInfo info)
{
JWFileWindowTitles.setTitle(this.chooser, info);

final var delegate = new JWRobotDelegate(robot);
final var okButton = delegate.getOkButton();

FxAssert.verifyThat(okButton, NodeMatchers.isDisabled());
robot.doubleClickOn(delegate.getTableCellFileName("DOC"));
robot.sleep(1L, SECONDS);

FxAssert.verifyThat(okButton, NodeMatchers.isDisabled());
robot.sleep(1L, SECONDS);
robot.clickOn(delegate.getTableCellFileName("."));
robot.sleep(1L, SECONDS);

robot.clickOn(okButton);

assertSelected("Z:\\USERS\\GROUCH\\DOC");
this.assertSelected("Z:\\USERS\\GROUCH\\DOC");
}

private void assertSelected(final String... selectedItems) {
private void assertSelected(final String... selectedItems)
{
Assertions.assertEquals(
List.of(selectedItems),
this.chooser.result()
.stream()
.map(Path::toString)
.collect(Collectors.toList())
.stream()
.map(Path::toString)
.collect(Collectors.toList())
);
}
}
Expand Up @@ -22,7 +22,6 @@
import com.io7m.jwheatsheaf.api.JWFileChooserType;
import com.io7m.jwheatsheaf.api.JWFileChoosersType;
import com.io7m.jwheatsheaf.ui.JWFileChoosers;
import javafx.scene.input.KeyCode;
import javafx.stage.Stage;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
Expand All @@ -38,14 +37,11 @@

import java.io.IOException;
import java.nio.file.FileSystem;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import static java.util.concurrent.TimeUnit.*;
import static java.util.concurrent.TimeUnit.SECONDS;

@ExtendWith(ApplicationExtension.class)
public final class JWFileChooserFilterDefaultTest
Expand Down Expand Up @@ -89,6 +85,7 @@ public void stop()
throws IOException
{
this.choosers.close();
this.chooser.cancel();
}

/**
Expand Down
Expand Up @@ -84,6 +84,7 @@ public void stop()
throws IOException
{
this.choosers.close();
this.chooser.cancel();
}

/**
Expand Down
Expand Up @@ -84,6 +84,7 @@ public void stop()
throws IOException
{
this.choosers.close();
this.chooser.cancel();
}

/**
Expand Down
Expand Up @@ -101,6 +101,7 @@ public void stop()
throws IOException
{
this.choosers.close();
this.chooser.cancel();
}

/**
Expand Down
Expand Up @@ -92,6 +92,7 @@ public void stop()
throws IOException
{
this.choosers.close();
this.chooser.cancel();
}

/**
Expand Down
Expand Up @@ -20,7 +20,6 @@
import com.io7m.jwheatsheaf.api.JWFileChoosersType;
import com.io7m.jwheatsheaf.api.JWFileKind;
import com.io7m.jwheatsheaf.ui.JWFileChoosers;
import com.io7m.jwheatsheaf.ui.internal.JWFileChooserFilterAllFiles;
import com.io7m.jwheatsheaf.ui.internal.JWFileChooserFilterOnlyDirectories;
import com.io7m.jwheatsheaf.ui.internal.JWFileItem;
import com.io7m.jwheatsheaf.ui.internal.JWFileList;
Expand Down

0 comments on commit 7981434

Please sign in to comment.