UI tweaks to unlock screen (to be tested under windows)

main/ui/src/main/java/org/cryptomator/ui/controllers/UnlockController.java

@@ -11,7 +11,8 @@ package org.cryptomator.ui.controllers;
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Strings;
 import javafx.application.Application;
-import javafx.beans.Observable;
+import javafx.beans.property.BooleanProperty;
+import javafx.beans.property.SimpleBooleanProperty;
 import javafx.beans.value.ChangeListener;
 import javafx.beans.value.ObservableValue;
 import javafx.fxml.FXML;
@@ -21,12 +22,11 @@ import javafx.scene.control.Button;
 import javafx.scene.control.ButtonType;
 import javafx.scene.control.CheckBox;
 import javafx.scene.control.ChoiceBox;
+import javafx.scene.control.ContentDisplay;
 import javafx.scene.control.Hyperlink;
 import javafx.scene.control.Label;
-import javafx.scene.control.ProgressIndicator;
 import javafx.scene.control.TextField;
 import javafx.scene.input.KeyEvent;
-import javafx.scene.layout.GridPane;
 import javafx.scene.layout.HBox;
 import javafx.scene.layout.VBox;
 import javafx.scene.text.Text;
@@ -85,6 +85,7 @@ public class UnlockController implements ViewController {
 	private Vault vault;
 	private Optional<UnlockListener> listener = Optional.empty();
 	private Subscription vaultSubs = Subscription.EMPTY;
+	private BooleanProperty unlocking = new SimpleBooleanProperty();
 
 	@Inject
 	public UnlockController(Application app, @Named("mainWindow") Stage mainWindow, Localization localization, WindowsDriveLetters driveLetters, Optional<KeychainAccess> keychainAccess, Settings settings, ExecutorService executor) {
@@ -125,7 +126,7 @@ public class UnlockController implements ViewController {
 	private CheckBox revealAfterMount;
 
 	@FXML
-	private Label winDriveLetterLabel;
+	private CheckBox useCustomWinDriveLetter;
 
 	@FXML
 	private ChoiceBox<Character> winDriveLetter;
@@ -139,9 +140,6 @@ public class UnlockController implements ViewController {
 	@FXML
 	private Label customMountPointLabel;
 
-	@FXML
-	private ProgressIndicator progressIndicator;
-
 	@FXML
 	private Hyperlink downloadsPageLink;
 
@@ -160,7 +158,8 @@ public class UnlockController implements ViewController {
 	@Override
 	public void initialize() {
 		advancedOptions.managedProperty().bind(advancedOptions.visibleProperty());
-		unlockButton.disableProperty().bind(passwordField.textProperty().isEmpty());
+		advancedOptions.disableProperty().bind(unlocking);
+		unlockButton.disableProperty().bind(unlocking.or(passwordField.textProperty().isEmpty()));
 		mountName.addEventFilter(KeyEvent.KEY_TYPED, this::filterAlphanumericKeyEvents);
 		mountName.textProperty().addListener(this::mountNameDidChange);
 		useCustomMountFlags.selectedProperty().addListener(this::useCustomMountFlagsDidChange);
@@ -168,14 +167,16 @@ public class UnlockController implements ViewController {
 		mountFlags.textProperty().addListener(this::mountFlagsDidChange);
 		savePassword.setDisable(!keychainAccess.isPresent());
 		unlockAfterStartup.disableProperty().bind(savePassword.disabledProperty().or(savePassword.selectedProperty().not()));
+		downloadsPageLink.visibleProperty().bind(downloadsPageLink.managedProperty());
 
 		customMountPoint.visibleProperty().bind(useCustomMountPoint.selectedProperty());
 		customMountPoint.managedProperty().bind(useCustomMountPoint.selectedProperty());
 		winDriveLetter.setConverter(new WinDriveLetterLabelConverter());
+		winDriveLetter.disableProperty().bind(useCustomWinDriveLetter.selectedProperty().not());
 
 		if (!SystemUtils.IS_OS_WINDOWS) {
-			winDriveLetterLabel.setVisible(false);
-			winDriveLetterLabel.setManaged(false);
+			useCustomWinDriveLetter.setVisible(false);
+			useCustomWinDriveLetter.setManaged(false);
 			winDriveLetter.setVisible(false);
 			winDriveLetter.setManaged(false);
 		}
@@ -205,18 +206,9 @@ public class UnlockController implements ViewController {
 		this.vault = vault;
 		advancedOptions.setVisible(false);
 		advancedOptionsButton.setText(localization.getString("unlock.button.advancedOptions.show"));
-		progressIndicator.setVisible(false);
+		unlockButton.setContentDisplay(ContentDisplay.TEXT_ONLY);
 		state.successMessage().map(localization::getString).ifPresent(messageText::setText);
-		if (SystemUtils.IS_OS_WINDOWS) {
-			winDriveLetter.valueProperty().removeListener(driveLetterChangeListener);
-			winDriveLetter.getItems().clear();
-			winDriveLetter.getItems().add(null);
-			winDriveLetter.getItems().addAll(driveLetters.getAvailableDriveLetters());
-			winDriveLetter.getItems().sort(new WinDriveLetterComparator());
-			winDriveLetter.valueProperty().addListener(driveLetterChangeListener);
-			chooseSelectedDriveLetter();
-		}
-		downloadsPageLink.setVisible(false);
+		downloadsPageLink.setManaged(false);
 		mountName.setText(vault.getMountName());
 		useCustomMountFlags.setSelected(vault.isHavingCustomMountFlags());
 		mountFlags.setText(vault.getMountFlags());
@@ -251,10 +243,6 @@ public class UnlockController implements ViewController {
 
 		// DOKANY-dependent controls:
 		if (VolumeImpl.DOKANY.equals(settings.preferredVolumeImpl().get())) {
-			winDriveLetter.visibleProperty().bind(useCustomMountPoint.selectedProperty().not());
-			winDriveLetter.managedProperty().bind(useCustomMountPoint.selectedProperty().not());
-			winDriveLetterLabel.visibleProperty().bind(useCustomMountPoint.selectedProperty().not());
-			winDriveLetterLabel.managedProperty().bind(useCustomMountPoint.selectedProperty().not());
 			// readonly not yet supported by dokany
 			useReadOnlyMode.setSelected(false);
 			useReadOnlyMode.setVisible(false);
@@ -265,10 +253,18 @@ public class UnlockController implements ViewController {
 
 		// OS-dependent controls:
 		if (SystemUtils.IS_OS_WINDOWS) {
+			winDriveLetter.valueProperty().removeListener(driveLetterChangeListener);
+			winDriveLetter.getItems().clear();
+			winDriveLetter.getItems().add(null);
+			winDriveLetter.getItems().addAll(driveLetters.getAvailableDriveLetters());
+			winDriveLetter.getItems().sort(new WinDriveLetterComparator());
+			winDriveLetter.valueProperty().addListener(driveLetterChangeListener);
+			chooseSelectedDriveLetter();
+			
 			winDriveLetter.visibleProperty().bind(useCustomMountPoint.selectedProperty().not());
 			winDriveLetter.managedProperty().bind(useCustomMountPoint.selectedProperty().not());
-			winDriveLetterLabel.visibleProperty().bind(useCustomMountPoint.selectedProperty().not());
-			winDriveLetterLabel.managedProperty().bind(useCustomMountPoint.selectedProperty().not());
+			useCustomWinDriveLetter.visibleProperty().bind(useCustomMountPoint.selectedProperty().not());
+			useCustomWinDriveLetter.managedProperty().bind(useCustomMountPoint.selectedProperty().not());
 		}
 
 		vaultSubs = vaultSubs.and(EasyBind.subscribe(unlockAfterStartup.selectedProperty(), vaultSettings.unlockAfterStartup()::set));
@@ -305,6 +301,7 @@ public class UnlockController implements ViewController {
 	@FXML
 	private void didClickAdvancedOptionsButton() {
 		messageText.setText(null);
+		downloadsPageLink.setManaged(false);
 		advancedOptions.setVisible(!advancedOptions.isVisible());
 		if (advancedOptions.isVisible()) {
 			advancedOptionsButton.setText(localization.getString("unlock.button.advancedOptions.hide"));
@@ -355,6 +352,13 @@ public class UnlockController implements ViewController {
 		}
 	}
 
+	@FXML
+	public void didClickCustomWinDriveLetterCheckbox() {
+		if (!useCustomWinDriveLetter.isSelected()) {
+			winDriveLetter.setValue(null);
+		}
+	}
+
 	/**
 	 * Converts 'C' to "C:" to translate between model and GUI.
 	 */
@@ -404,7 +408,7 @@ public class UnlockController implements ViewController {
 	private void chooseSelectedDriveLetter() {
 		assert SystemUtils.IS_OS_WINDOWS;
 		// if the vault prefers a drive letter, that is currently occupied, this is our last chance to reset this:
-		if (driveLetters.getOccupiedDriveLetters().contains(vault.getWinDriveLetter())) {
+		if (vault.getWinDriveLetter() != null && driveLetters.getOccupiedDriveLetters().contains(vault.getWinDriveLetter())) {
 			vault.setWinDriveLetter(null);
 		}
 		final Character letter = vault.getWinDriveLetter();
@@ -453,10 +457,9 @@ public class UnlockController implements ViewController {
 
 	@FXML
 	private void didClickUnlockButton() {
-		advancedOptions.setDisable(true);
-		advancedOptions.setVisible(false);
+		unlocking.set(true);
 		advancedOptionsButton.setText(localization.getString("unlock.button.advancedOptions.show"));
-		progressIndicator.setVisible(true);
+		unlockButton.setContentDisplay(ContentDisplay.LEFT);
 
 		CharSequence password = passwordField.getCharacters();
 		Tasks.create(() -> {
@@ -465,22 +468,23 @@ public class UnlockController implements ViewController {
 				keychainAccess.get().storePassphrase(vault.getId(), password);
 			}
 		}).onSuccess(() -> {
-			messageText.setText("");
-			downloadsPageLink.setVisible(false);
+			messageText.setText(null);
+			downloadsPageLink.setManaged(false);
 			listener.ifPresent(lstnr -> lstnr.didUnlock(vault));
 			passwordField.swipe();
 		}).onError(InvalidPassphraseException.class, e -> {
 			messageText.setText(localization.getString("unlock.errorMessage.wrongPassword"));
+			downloadsPageLink.setManaged(false);
 			passwordField.selectAll();
 			passwordField.requestFocus();
 		}).onError(UnsupportedVaultFormatException.class, e -> {
 			if (e.isVaultOlderThanSoftware()) {
 				// whitespace after localized text used as separator between text and hyperlink
 				messageText.setText(localization.getString("unlock.errorMessage.unsupportedVersion.vaultOlderThanSoftware") + " ");
-				downloadsPageLink.setVisible(true);
+				downloadsPageLink.setManaged(true);
 			} else if (e.isSoftwareOlderThanVault()) {
 				messageText.setText(localization.getString("unlock.errorMessage.unsupportedVersion.softwareOlderThanVault") + " ");
-				downloadsPageLink.setVisible(true);
+				downloadsPageLink.setManaged(true);
 			} else if (e.getDetectedVersion() == Integer.MAX_VALUE) {
 				messageText.setText(localization.getString("unlock.errorMessage.unauthenticVersionMac"));
 			}
@@ -488,18 +492,21 @@ public class UnlockController implements ViewController {
 			LOG.error("Unlock failed. Mount point not a directory: {}", e.getMessage());
 			advancedOptions.setVisible(true);
 			messageText.setText(null);
+			downloadsPageLink.setManaged(false);
 			showUnlockFailedErrorDialog("unlock.failedDialog.content.mountPathNonExisting");
 		}).onError(DirectoryNotEmptyException.class, e -> {
 			LOG.error("Unlock failed. Mount point not empty: {}", e.getMessage());
 			advancedOptions.setVisible(true);
 			messageText.setText(null);
+			downloadsPageLink.setManaged(false);
 			showUnlockFailedErrorDialog("unlock.failedDialog.content.mountPathNotEmpty");
 		}).onError(Exception.class, e -> { // including RuntimeExceptions
 			LOG.error("Unlock failed for technical reasons.", e);
 			messageText.setText(localization.getString("unlock.errorMessage.unlockFailed"));
+			downloadsPageLink.setManaged(false);
 		}).andFinally(() -> {
-			advancedOptions.setDisable(false);
-			progressIndicator.setVisible(false);
+			unlocking.set(false);
+			unlockButton.setContentDisplay(ContentDisplay.TEXT_ONLY);
 		}).runOnce(executor);
 	}
 

main/ui/src/main/java/org/cryptomator/ui/model/WindowsDriveLetters.java

@@ -8,6 +8,8 @@ package org.cryptomator.ui.model;
 import org.apache.commons.lang3.CharUtils;
 import org.apache.commons.lang3.SystemUtils;
 import org.cryptomator.common.FxApplicationScoped;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
 import java.nio.file.FileSystems;
@@ -21,6 +23,7 @@ import java.util.stream.StreamSupport;
 @FxApplicationScoped
 public final class WindowsDriveLetters {
 
+	private static final Logger LOG = LoggerFactory.getLogger(WindowsDriveLetters.class);
 	private static final Set<Character> D_TO_Z;
 
 	static {
@@ -35,10 +38,12 @@ public final class WindowsDriveLetters {
 
 	public Set<Character> getOccupiedDriveLetters() {
 		if (!SystemUtils.IS_OS_WINDOWS) {
-			throw new UnsupportedOperationException("This method is only defined for Windows file systems");
+			LOG.warn("Attempted to get occupied drive letters on non-Windows machine.");
+			return Set.of();
+		} else {
+			Iterable<Path> rootDirs = FileSystems.getDefault().getRootDirectories();
+			return StreamSupport.stream(rootDirs.spliterator(), false).map(Path::toString).map(CharUtils::toChar).map(Character::toUpperCase).collect(Collectors.toSet());
 		}
-		Iterable<Path> rootDirs = FileSystems.getDefault().getRootDirectories();
-		return StreamSupport.stream(rootDirs.spliterator(), false).map(Path::toString).map(CharUtils::toChar).map(Character::toUpperCase).collect(Collectors.toSet());
 	}
 
 	public Set<Character> getAvailableDriveLetters() {

main/ui/src/main/resources/fxml/unlock.fxml

@@ -25,7 +25,7 @@
 <VBox fx:controller="org.cryptomator.ui.controllers.UnlockController" fx:id="root" spacing="12" alignment="BOTTOM_CENTER" xmlns:fx="http://javafx.com/fxml" prefWidth="400">
 
 	<padding>
-		<Insets top="24"/>
+		<Insets top="24" right="12" bottom="24" left="12" />
 	</padding>
 
 	<!-- Password Field -->
@@ -35,14 +35,25 @@
 	</HBox>
 
 	<!-- Unlock Button / Advanced Options Button -->
-	<HBox spacing="12.0" alignment="CENTER_RIGHT">
+	<HBox spacing="12" alignment="CENTER_RIGHT">
 		<Button fx:id="advancedOptionsButton" text="%unlock.button.advancedOptions.show" prefWidth="150.0" onAction="#didClickAdvancedOptionsButton"/>
-		<Button fx:id="unlockButton" text="%unlock.button.unlock" defaultButton="true" prefWidth="150.0" onAction="#didClickUnlockButton" disable="true"/>
-		<ProgressIndicator progress="-1" fx:id="progressIndicator"/>
+		<Button fx:id="unlockButton" text="%unlock.button.unlock" defaultButton="true" prefWidth="150.0" onAction="#didClickUnlockButton" disable="true" contentDisplay="TEXT_ONLY">
+				<graphic>
+					<ProgressIndicator progress="-1" prefWidth="12" prefHeight="12"/>
+				</graphic>
+		</Button>
 	</HBox>
 
+	<!-- Status Text -->
+	<TextFlow prefWidth="400" textAlignment="LEFT">
+		<children>
+			<Text fx:id="messageText"/>
+			<Hyperlink fx:id="downloadsPageLink" text="%unlock.label.downloadsPageLink" managed="false" onAction="#didClickDownloadsLink"/>
+		</children>
+	</TextFlow>
+
 	<!-- Advanced Options -->
-	<VBox fx:id="advancedOptions" spacing="6" VBox.vgrow="ALWAYS" visible="false">
+	<VBox fx:id="advancedOptions" spacing="12" VBox.vgrow="ALWAYS" visible="false">
 
 		<Separator/>
 
@@ -81,17 +92,10 @@
 		</HBox>
 
 		<!-- Windows Drive Letter -->
-		<HBox spacing="12">
-			<Label fx:id="winDriveLetterLabel" text="%unlock.label.winDriveLetter"/>
-			<ChoiceBox fx:id="winDriveLetter" maxWidth="Infinity"/>
+		<HBox spacing="12" alignment="BASELINE_LEFT">
+			<CheckBox fx:id="useCustomWinDriveLetter" text="%unlock.label.winDriveLetter" onAction="#didClickCustomWinDriveLetterCheckbox"/>
+			<ChoiceBox fx:id="winDriveLetter" HBox.hgrow="NEVER" maxWidth="Infinity"/>
 		</HBox>
 	</VBox>
 
-	<!-- Status Text -->
-	<TextFlow>
-		<children>
-			<Text fx:id="messageText" visible="true"/>
-			<Hyperlink fx:id="downloadsPageLink" text="%unlock.label.downloadsPageLink" visible="false" onAction="#didClickDownloadsLink"/>
-		</children>
-	</TextFlow>
 </VBox>