Skip to content

Commit

Permalink
Fix a silently-failing OK button
Browse files Browse the repository at this point in the history
This fixes an issue where the "are all files filtered" check was
being carried out in the OK button method. This caused the button to
silently fail in CREATE mode when the name of a nonexistent file was
typed in and an "is regular file" check was provided for the file
selection mode.

Fix: #34
  • Loading branch information
io7m committed Feb 13, 2022
1 parent 694b3d4 commit c79188d
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 20 deletions.
Expand Up @@ -39,6 +39,7 @@
import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
import java.time.temporal.ChronoUnit;
Expand All @@ -49,6 +50,7 @@
import java.util.Objects;
import java.util.ResourceBundle;
import java.util.concurrent.Executors;
import java.util.function.Function;
import java.util.stream.Collectors;

/**
Expand Down Expand Up @@ -88,6 +90,8 @@ public final class ExampleViewController implements Initializable
@FXML
private CheckBox unusualStrings;
@FXML
private CheckBox onlyFiles;
@FXML
private ChoiceBox<JWFileChooserAction> action;
@FXML
private TextField title;
Expand Down Expand Up @@ -219,6 +223,13 @@ private void onOpenSelected()
stringOverrides = JWFileChooserStringOverridesEmpty.get();
}

final Function<Path, Boolean> fileSelectionMode;
if (this.onlyFiles.isSelected()) {
fileSelectionMode = path -> Boolean.valueOf(Files.isRegularFile(path));
} else {
fileSelectionMode = path -> Boolean.TRUE;
}

configurationBuilder
.setAllowDirectoryCreation(this.allowDirectoryCreation.isSelected())
.setShowParentDirectory(this.parentDirectory.isSelected())
Expand All @@ -230,6 +241,7 @@ private void onOpenSelected()
.setStringOverrides(stringOverrides)
.addFileFilters(new ExampleFilterRejectAll())
.addFileFilters(new ExampleFilterXML())
.setFileSelectionMode(fileSelectionMode)
.addAllRecentFiles(recents);

if (this.homeDirectory.isSelected()) {
Expand Down
Expand Up @@ -4,6 +4,7 @@
<?import javafx.scene.control.Button?>
<?import javafx.scene.control.CheckBox?>
<?import javafx.scene.control.ChoiceBox?>
<?import javafx.scene.control.ComboBox?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.TextArea?>
<?import javafx.scene.control.TextField?>
Expand All @@ -14,7 +15,6 @@
<?import javafx.scene.layout.RowConstraints?>
<?import javafx.scene.layout.VBox?>

<?import javafx.scene.control.ComboBox?>
<VBox fx:id="main" prefHeight="600.0" prefWidth="800.0" stylesheets="@example.css" xmlns="http://javafx.com/javafx/11.0.1" xmlns:fx="http://javafx.com/fxml/1" fx:controller="com.io7m.jwheatsheaf.examples.ExampleViewController">
<children>
<HBox layoutX="18.0" layoutY="18.0">
Expand Down Expand Up @@ -51,6 +51,7 @@
<CheckBox fx:id="parentDirectory" maxHeight="-Infinity" maxWidth="1.7976931348623157E308" minHeight="-Infinity" mnemonicParsing="false" onAction="#onParentDirectoryChanged" prefHeight="32.0" text="Parent directory" GridPane.columnIndex="1" GridPane.rowIndex="3" />
<CheckBox fx:id="confirmSelection" maxHeight="-Infinity" maxWidth="1.7976931348623157E308" minHeight="-Infinity" mnemonicParsing="false" onAction="#onConfirmSelectionChanged" prefHeight="32.0" text="Confirm selection" GridPane.columnIndex="1" GridPane.rowIndex="4" />
<CheckBox fx:id="unusualStrings" mnemonicParsing="false" text="Unusual strings" GridPane.columnIndex="2" />
<CheckBox fx:id="onlyFiles" mnemonicParsing="false" text="Only allow files" GridPane.columnIndex="2" GridPane.rowIndex="1" />
</children>
</GridPane>
<Region layoutX="18.0" layoutY="250.0" maxHeight="-Infinity" minHeight="-Infinity" prefHeight="8.0" />
Expand All @@ -71,8 +72,7 @@
<HBox layoutX="18.0" layoutY="295.0">
<children>
<Label maxHeight="-Infinity" minHeight="-Infinity" prefHeight="32.0" prefWidth="160.0" text="Locale" />
<ComboBox fx:id="localeSelection" maxHeight="-Infinity" maxWidth="1.7976931348623157E308" minHeight="-Infinity"
prefHeight="32.0" HBox.hgrow="ALWAYS" />
<ComboBox fx:id="localeSelection" maxHeight="-Infinity" maxWidth="1.7976931348623157E308" minHeight="-Infinity" prefHeight="32.0" HBox.hgrow="ALWAYS" />
</children>
</HBox>
<Region layoutX="18.0" layoutY="327.0" maxHeight="-Infinity" minHeight="-Infinity" prefHeight="8.0" />
Expand Down
@@ -0,0 +1,132 @@
/*
* Copyright © 2020 Mark Raynsford <code@io7m.com> http://io7m.com
*
* Permission to use, copy, modify, and/or distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
* copyright notice and this permission notice appear in all copies.
*
* THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
* SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR
* IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/

package com.io7m.jwheatsheaf.tests;

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.scene.input.KeyCode;
import javafx.stage.Stage;
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;
import org.testfx.framework.junit5.ApplicationExtension;
import org.testfx.framework.junit5.Start;
import org.testfx.framework.junit5.Stop;
import org.testfx.matcher.base.NodeMatchers;

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

@ExtendWith(ApplicationExtension.class)
public final class JWFileChooserBug34
{
private JWTestFilesystems filesystems;
private FileSystem dosFilesystem;
private JWFileChooserType chooser;
private List<JWFileChooserEventType> events;
private JWFileChoosersType choosers;

@Start
public void start(final Stage stage)
throws Exception
{
this.events = Collections.synchronizedList(new ArrayList<>());

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

final var configuration =
JWFileChooserConfiguration.builder()
.setAction(JWFileChooserAction.CREATE)
.setFileSystem(this.dosFilesystem)
.setFileSelectionMode(path -> {
return Boolean.valueOf(Files.isRegularFile(path));
})
.build();

this.choosers = JWFileChoosers.create();
this.chooser = this.choosers.create(stage, configuration);
this.chooser.setEventListener(event -> this.events.add(event));
this.chooser.show();
}

@Stop
public void stop()
throws IOException
{
this.choosers.close();
this.chooser.cancel();
}

/**
* In CREATE mode, specifying a nonexistent file if using a filter that
* checks if files are regular files, does not silently fail when the user
* clicks OK.
*
* @param robot The FX test robot
*/

@Test
public void test_NameField_NonexistentFileDoesNotSilentlyFail(
final FxRobot robot,
final TestInfo info)
{
JWFileWindowTitles.setTitle(this.chooser, info);

final var delegate = new JWRobotDelegate(robot);

final var okButton =
robot.lookup("#fileChooserOKButton")
.queryButton();

robot.clickOn("#fileChooserNameField");

FxAssert.verifyThat(okButton, NodeMatchers.isDisabled());
robot.write("THIS_DOES_NOT_EXIST.TXT");
delegate.pauseBriefly();
FxAssert.verifyThat(okButton, NodeMatchers.isEnabled());

robot.type(KeyCode.ENTER);
FxAssert.verifyThat(okButton, NodeMatchers.isEnabled());

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

Assertions.assertEquals(
List.of("Z:\\USERS\\GROUCH\\THIS_DOES_NOT_EXIST.TXT"),
this.chooser.result()
.stream()
.map(Path::toString)
.collect(Collectors.toList())
);
Assertions.assertEquals(0, this.events.size());
}
}
Expand Up @@ -650,6 +650,8 @@ private void onCreateDirectoryButton()
@FXML
private void onOKSelected()
{
LOG.debug("ok: selected");

this.result = List.of();

var resultTarget = List.<Path>of();
Expand All @@ -669,25 +671,23 @@ private void onOKSelected()
break;
}

resultTarget =
resultTarget.stream()
.filter(this::filterSelectionMode)
.collect(Collectors.toList());

if (!resultTarget.isEmpty()) {
final boolean confirmed;
if (this.isFileSelectionConfirmationRequired()) {
confirmed = this.confirmFileSelection(resultTarget);
} else {
confirmed = true;
}
final boolean confirmed;
if (this.isFileSelectionConfirmationRequired()) {
confirmed = this.confirmFileSelection(resultTarget);
} else {
confirmed = true;
}

if (confirmed) {
this.result = resultTarget;
final var window = this.mainContent.getScene().getWindow();
window.hide();
}
if (!confirmed) {
LOG.trace("ok: confirmation failed");
return;
}

this.result = resultTarget;
final var window = this.mainContent.getScene().getWindow();

LOG.trace("ok: hiding window");
window.hide();
}

/**
Expand Down

0 comments on commit c79188d

Please sign in to comment.