From 1ea9115dafa8bd0d1a949693caca9a18caa1487e Mon Sep 17 00:00:00 2001 From: Mark Raynsford Date: Sat, 1 May 2021 14:39:43 +0000 Subject: [PATCH] Improve "select directly" behaviour If the path entered into the "select directly" dialog is a file, then set the current directory to the parent of the file, and select the file. If the path entered into the "select directly" dialog is a directory, then set the current directory to the path, and select ".". Fix: https://github.com/io7m/jwheatsheaf/issues/29 --- README-CHANGES.xml | 9 +- .../tests/JWFileChooserActionSaveTest.java | 102 +++++++++++++++++- .../jwheatsheaf/tests/JWTestFilesystems.java | 2 +- .../internal/JWFileChooserViewController.java | 80 ++++++++++---- 4 files changed, 168 insertions(+), 25 deletions(-) diff --git a/README-CHANGES.xml b/README-CHANGES.xml index ff83777..d38d613 100644 --- a/README-CHANGES.xml +++ b/README-CHANGES.xml @@ -39,7 +39,7 @@ - + @@ -86,11 +86,16 @@ - + + + + + + diff --git a/com.io7m.jwheatsheaf.tests/src/test/java/com/io7m/jwheatsheaf/tests/JWFileChooserActionSaveTest.java b/com.io7m.jwheatsheaf.tests/src/test/java/com/io7m/jwheatsheaf/tests/JWFileChooserActionSaveTest.java index 7932f66..4a9d46a 100644 --- a/com.io7m.jwheatsheaf.tests/src/test/java/com/io7m/jwheatsheaf/tests/JWFileChooserActionSaveTest.java +++ b/com.io7m.jwheatsheaf.tests/src/test/java/com/io7m/jwheatsheaf/tests/JWFileChooserActionSaveTest.java @@ -90,7 +90,7 @@ public void stop() */ @Test - public void testActionSaveNamed( + public void test_NameField_ExplicitlyTypedName_CandidateSelected( final FxRobot robot, final TestInfo info) { @@ -126,13 +126,15 @@ public void testActionSaveNamed( } /** - * The enter directly dialog works. + * If the path entered into the "select direct" dialog is not a directory, + * then the parent of that path's file name is set as the current directory, + * and the file name is selected. * * @param robot The FX test robot */ @Test - public void testDirectorySelectDirect( + public void test_SelectDirectly_TargetIsNotFile_TargetSelected( final FxRobot robot, final TestInfo info) { @@ -167,4 +169,98 @@ public void testDirectorySelectDirect( ); Assertions.assertEquals(0, this.events.size()); } + + /** + * If the path entered into the "select direct" dialog is not a directory, + * then the parent of that path's file name is set as the current directory, + * and the file name is selected. + * + * @param robot The FX test robot + */ + + @Test + public void test_SelectDirectly_TargetIsFile_TargetSelected( + final FxRobot robot, + final TestInfo info) + { + JWFileWindowTitles.setTitle(this.chooser, info); + + final var okButton = + robot.lookup("#fileChooserOKButton") + .queryButton(); + + final var selectButton = + robot.lookup("#fileChooserSelectDirectButton") + .queryButton(); + + FxAssert.verifyThat(okButton, NodeMatchers.isDisabled()); + robot.clickOn(selectButton); + + FxAssert.verifyThat(okButton, NodeMatchers.isDisabled()); + robot.write("Z:\\USERS\\GROUCH\\PHOTO.JPG"); + + FxAssert.verifyThat(okButton, NodeMatchers.isDisabled()); + robot.type(KeyCode.ENTER); + + FxAssert.verifyThat(okButton, NodeMatchers.isEnabled()); + robot.clickOn(okButton); + + Assertions.assertEquals( + List.of("Z:\\USERS\\GROUCH\\PHOTO.JPG"), + this.chooser.result() + .stream() + .map(Path::toString) + .collect(Collectors.toList()) + ); + Assertions.assertEquals(0, this.events.size()); + } + + /** + * If the path entered into the "select direct" dialog is a directory, + * then that path becomes the new current directory, and "." is selected. + * + * @param robot The FX test robot + */ + + @Test + public void test_SelectDirectly_TargetIsDirectory_TargetSelected( + final FxRobot robot, + final TestInfo info) + throws Exception + { + JWFileWindowTitles.setTitle(this.chooser, info); + + final var delegate = new JWRobotDelegate(robot); + + final var okButton = + robot.lookup("#fileChooserOKButton") + .queryButton(); + + final var selectButton = + robot.lookup("#fileChooserSelectDirectButton") + .queryButton(); + + FxAssert.verifyThat(okButton, NodeMatchers.isDisabled()); + robot.clickOn(selectButton); + + FxAssert.verifyThat(okButton, NodeMatchers.isDisabled()); + robot.write("Z:\\USERS\\"); + + FxAssert.verifyThat(okButton, NodeMatchers.isDisabled()); + robot.type(KeyCode.ENTER); + + delegate.waitUntil(() -> !okButton.isDisabled()); + + FxAssert.verifyThat(okButton, NodeMatchers.isEnabled()); + robot.clickOn(okButton); + + Assertions.assertEquals( + List.of("Z:\\USERS"), + this.chooser.result() + .stream() + .map(Path::toString) + .collect(Collectors.toList()) + ); + Assertions.assertEquals(0, this.events.size()); + } } diff --git a/com.io7m.jwheatsheaf.tests/src/test/java/com/io7m/jwheatsheaf/tests/JWTestFilesystems.java b/com.io7m.jwheatsheaf.tests/src/test/java/com/io7m/jwheatsheaf/tests/JWTestFilesystems.java index d520882..1154ec4 100644 --- a/com.io7m.jwheatsheaf.tests/src/test/java/com/io7m/jwheatsheaf/tests/JWTestFilesystems.java +++ b/com.io7m.jwheatsheaf.tests/src/test/java/com/io7m/jwheatsheaf/tests/JWTestFilesystems.java @@ -180,7 +180,7 @@ public int getRegexFlags() .build(); Files.createDirectories(filesystem.getPath("Z:\\HOME")); - Files.writeString(filesystem.getPath("Z:\\HOME","FILE.TXT"), "FILE!"); + Files.writeString(filesystem.getPath("Z:\\HOME", "FILE.TXT"), "FILE!"); Files.createDirectories(filesystem.getPath("DOC")); Files.writeString(filesystem.getPath("README.TXT"), "HELLO!"); Files.writeString(filesystem.getPath("DATA.XML"), "Some data."); diff --git a/com.io7m.jwheatsheaf.ui/src/main/java/com/io7m/jwheatsheaf/ui/internal/JWFileChooserViewController.java b/com.io7m.jwheatsheaf.ui/src/main/java/com/io7m/jwheatsheaf/ui/internal/JWFileChooserViewController.java index f03a807..ec95d28 100644 --- a/com.io7m.jwheatsheaf.ui/src/main/java/com/io7m/jwheatsheaf/ui/internal/JWFileChooserViewController.java +++ b/com.io7m.jwheatsheaf.ui/src/main/java/com/io7m/jwheatsheaf/ui/internal/JWFileChooserViewController.java @@ -78,6 +78,10 @@ public final class JWFileChooserViewController private static final Logger LOG = LoggerFactory.getLogger(JWFileChooserViewController.class); + private static final Runnable AND_THEN_DO_NOTHING = () -> { + + }; + private final AtomicReference> eventReceiver; private final BlockingDeque initialFilename; private final ChangeListener listener; @@ -241,7 +245,7 @@ public void setConfiguration( this.upDirectoryButton ); - this.setCurrentDirectory(startDirectory); + this.setCurrentDirectory(startDirectory, AND_THEN_DO_NOTHING); } private void configureSearch() @@ -340,11 +344,12 @@ protected void updateItem( */ private void setCurrentDirectory( - final Path path) + final Path path, + final Runnable andThen) { this.currentDirectory = Objects.requireNonNull(path, "path"); this.rebuildPathMenu(path); - this.populateDirectoryTable(path); + this.populateDirectoryTable(path, andThen); } private void rebuildPathMenu( @@ -359,17 +364,20 @@ private void rebuildPathMenu( } private void populateDirectoryTable( - final Path directory) + final Path directory, + final Runnable andThen) { this.populateDirectoryTableWith( () -> JWFileItems.listDirectory( directory, - this.configuration.showParentDirectory()) + this.configuration.showParentDirectory()), + andThen ); } private void populateDirectoryTableWith( - final JWFileListingRetrieverType itemRetriever) + final JWFileListingRetrieverType itemRetriever, + final Runnable andThen) { this.directoryTable.setItems(this.fileListing.items()); this.ioLockUI(); @@ -389,6 +397,8 @@ private void populateDirectoryTableWith( } catch (final NoSuchElementException e) { // Most of the time, there's no initial filename. } + + andThen.run(); }); } catch (final Exception e) { LOG.error("exception during directory listing: ", e); @@ -513,7 +523,7 @@ private void onPathMenuItemSelected( final Path oldValue, final Path newValue) { - this.setCurrentDirectory(newValue); + this.setCurrentDirectory(newValue, AND_THEN_DO_NOTHING); } @FXML @@ -545,12 +555,40 @@ private void onSelectDirectButton() final var nameOpt = dialog.showAndWait(); if (nameOpt.isPresent()) { final var name = nameOpt.get(); - final var path = this.configuration.fileSystem().getPath(name); - final var parent = path.getParent(); + final var fileSystem = this.configuration.fileSystem(); + final var targetPath = fileSystem.getPath(name); + + /* + * If the specified path is a directory, then simply set the current + * directory to that path. + */ + + if (Files.isDirectory(targetPath)) { + this.setCurrentDirectory(targetPath, () -> { + this.trySelectDirectoryItem(this.directoryTable.getItems(), "."); + }); + return; + } + + /* + * Otherwise, set the current directory to the parent of the target + * path, and attempt to select the file with the path's file name. + */ + + final var targetFileNameOpt = + Optional.ofNullable(targetPath.getFileName()) + .map(Path::toString); + + final var parent = targetPath.getParent(); if (parent != null) { - this.setCurrentDirectory(parent); + this.setCurrentDirectory(parent, () -> { + targetFileNameOpt.ifPresent( + targetName -> this.fileName.setText(targetName)); + }); + } else { + targetFileNameOpt.ifPresent( + targetName -> this.fileName.setText(targetName)); } - this.fileName.setText(path.getFileName().toString()); } } @@ -559,7 +597,7 @@ private void onUpDirectoryButton() { final var parent = this.currentDirectory.getParent(); if (parent != null) { - this.setCurrentDirectory(parent); + this.setCurrentDirectory(parent, AND_THEN_DO_NOTHING); } } @@ -592,7 +630,7 @@ private void onCreateDirectoryButton() LOG.error("exception raised by event receiver: ", ex); } } - this.setCurrentDirectory(this.currentDirectory); + this.setCurrentDirectory(this.currentDirectory, AND_THEN_DO_NOTHING); } } @@ -640,8 +678,10 @@ private void onCancelSelected() @FXML private void onHomeSelected() { - final var homeDirectoryOpt = this.configuration.homeDirectory(); - homeDirectoryOpt.ifPresent(this::setCurrentDirectory); + final var homeDirectoryOpt = + this.configuration.homeDirectory(); + homeDirectoryOpt + .ifPresent(path -> this.setCurrentDirectory(path, AND_THEN_DO_NOTHING)); } @FXML @@ -849,7 +889,7 @@ private void onTableRowClicked( if (item != null) { final var directory = item.path(); if (Files.isDirectory(directory)) { - this.setCurrentDirectory(directory); + this.setCurrentDirectory(directory, AND_THEN_DO_NOTHING); } } } @@ -859,9 +899,11 @@ private void onSourceItemDoubleClicked( final MouseEvent event) { if (event.getClickCount() == 2) { - final var item = this.sourcesList.getSelectionModel().getSelectedItem(); - item.path().ifPresent(this::setCurrentDirectory); - this.populateDirectoryTableWith(item); + final var item = + this.sourcesList.getSelectionModel().getSelectedItem(); + item.path() + .ifPresent(path -> this.setCurrentDirectory(path, AND_THEN_DO_NOTHING)); + this.populateDirectoryTableWith(item, AND_THEN_DO_NOTHING); } }