[client] added user feedback & reworked how we reactToInput() in ImageDetailsWindow

TODO: make the error message not shift all the fields upwards...

2 files changed, 78 insertions, 59 deletions

diff --git a/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/ImageDetailsWindow.java b/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/ImageDetailsWindow.java
index d53daf95..821f13cf 100644
--- a/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/ImageDetailsWindow.java
+++ b/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/ImageDetailsWindow.java
@@ -144,9 +144,10 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 					@Override
 					public void userAdded(UserInfo user, UserListWindow window) {
 						window.dispose();
-						if (Gui.showMessageBox(me, "Sind Sie sicher, dass sie die Besitzerrechte an "
-								+ "einen anderen Account übertragen wollen?", MessageType.QUESTION_YESNO,
-								LOGGER, null))
+						if (Gui.showMessageBox(me,
+								"Sind Sie sicher, dass sie die Besitzerrechte an "
+										+ "einen anderen Account übertragen wollen?",
+								MessageType.QUESTION_YESNO, LOGGER, null))
 							setImageOwner(user);
 					}
 
@@ -159,9 +160,9 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 				// open the custom permission window and save returned default/custom permissions
 				DefaultCustomPerms<ImagePermissions> combined = ImagePermissionWindow.open(me,
 						customPermissions, image.defaultPermissions, image.ownerId);
-				// since the window above gets references of the default/custom permission object
-				// there is no need to further save the return value ...
-				// THAT or we do work with copies in ImagePermissionWindow ...
+						// since the window above gets references of the default/custom permission object
+						// there is no need to further save the return value ...
+						// THAT or we do work with copies in ImagePermissionWindow ...
 
 				// for now let's stay with refs and just reactToChange() where we check if
 				// the permissions stuff changed since the last call of fill()
@@ -216,12 +217,12 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 					// highlight the row and popup the menu
 					versionTable.setRowSelectionInterval(r, r);
 					if (e.isPopupTrigger()) {
-						popupItemNew.setEnabled(versionTable.getSelectedItem().isValid
-								&& ImagePerms.canLink(image));
-						popupItemDownload.setEnabled(versionTable.getSelectedItem().isValid
-								&& ImagePerms.canDownload(image));
-						popupItemDelete.setEnabled(versionTable.getSelectedItem().isValid
-								&& ImagePerms.canAdmin(image));
+						popupItemNew.setEnabled(
+								versionTable.getSelectedItem().isValid && ImagePerms.canLink(image));
+						popupItemDownload.setEnabled(
+								versionTable.getSelectedItem().isValid && ImagePerms.canDownload(image));
+						popupItemDelete.setEnabled(
+								versionTable.getSelectedItem().isValid && ImagePerms.canAdmin(image));
 						pop.show(e.getComponent(), e.getX(), e.getY());
 					}
 				} else {
@@ -251,8 +252,10 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 	 * 
 	 ********************************************************************************/
 	/**
-	 * Sets the image to the given imageBaseId. This will also trigger fill() which 
-	 * will set the image details fields to the values represented by this image.
+	 * Sets the image to the given imageBaseId. This will also trigger fill()
+	 * which
+	 * will set the image details fields to the values represented by this
+	 * image.
 	 *
 	 * @param imageBaseId the id of the image to be displayed
 	 */
@@ -288,7 +291,8 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 	 * @param user UserInfo to set the owner to
 	 */
 	private void setImageOwner(final UserInfo user) {
-		if (!ThriftActions.setImageOwner(JOptionPane.getFrameForComponent(me), image.getImageBaseId(), user)) {
+		if (!ThriftActions.setImageOwner(JOptionPane.getFrameForComponent(me), image.getImageBaseId(),
+				user)) {
 			return;
 		}
 		Gui.showMessageBox(me, "Besitzrechte übertragen an " + FormatHelper.userName(user), MessageType.INFO,
@@ -304,9 +308,9 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 		// first build the ImageBaseWrite from the GUI fields
 		final ImageBaseWrite ibw = new ImageBaseWrite(txtTitle.getText(), txtDescription.getText(),
 				cboOperatingSystem.getItemAt(cboOperatingSystem.getSelectedIndex()).osId, image.virtId,
-				chkIsTemplate.isSelected(), new ImagePermissions(image.defaultPermissions.link,
-						image.defaultPermissions.download, image.defaultPermissions.edit,
-						image.defaultPermissions.admin),
+				chkIsTemplate.isSelected(),
+				new ImagePermissions(image.defaultPermissions.link, image.defaultPermissions.download,
+						image.defaultPermissions.edit, image.defaultPermissions.admin),
 				cboShareMode.getItemAt(cboShareMode.getSelectedIndex()));
 		// now trigger the actual action
 		if (!ThriftActions.updateImageBase(JOptionPane.getFrameForComponent(me), image.getImageBaseId(), ibw))
@@ -499,35 +503,46 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 	/**
 	 * Checks whether the user changed any fields of the image details and
 	 * enables the save button if so
-	 * TODO: give user feedback why save isn't enabled if he changed something
-	 * but some other field is not valid....
-	 * e.g. no OS preset, change some checkbox, still can't save since OS is not selected
-	 * and the user is currently not notified that 1) we saw he changed something 2) what
-	 * field is invalid (in this example, OS)
+	 * TODO TAGS!
 	 */
 	private boolean reactToChange() {
+		// disable save by default here, we will enable it at the end if all checks passed
+		btnSaveChanges.setEnabled(false);
+		boolean changed = false;
+		// Image name
+		if (txtTitle.getText().isEmpty()) {
+			lblError.setText("Kein Imagename!");
+			return false;
+		}
+		// Image description
+		if (txtDescription.getText().isEmpty()) {
+			lblError.setText("Keine Beschreibung!");
+			return false;
+		}
+
+		// Operating system
 		OperatingSystem newOs = cboOperatingSystem.getItemAt(cboOperatingSystem.getSelectedIndex());
+		if (newOs == null) {
+			lblError.setText("Kein Betriebssystem ausgewählt!");
+			return false;
+		}
+
+		// Share mode
 		ShareMode newShareMode = cboShareMode.getItemAt(cboShareMode.getSelectedIndex());
-		boolean changed = false;
-		if (newOs != null) {
-			if (!txtTitle.getText().isEmpty() && !txtTitle.getText().equals(image.getImageName())) {
-				changed = true;
-			} else if (!txtDescription.getText().isEmpty()
-					&& !txtDescription.getText().equals(image.getDescription())) {
-				changed = true;
-			} else if (newOs.getOsId() != image.getOsId()) {
-				changed = true;
-			} else if (newShareMode != null && !newShareMode.equals(image.shareMode)) {
-				changed = true;
-			} else if (chkIsTemplate.isSelected() != image.isTemplate) {
-				changed = true;
-			} else if (!image.defaultPermissions.equals(originalDefaultPermissions)) {
-				changed = true;
-			} else if (PermsHelper.hasChanged(originalCustomPermissions, customPermissions)) {
-				changed = true;
-			}
+		if (newShareMode == null) {
+			lblError.setText("Kein Share-Modus ausgewählt!");
+			return false;
 		}
-		// TODO TAGS
+		// Template, default permissions and custom permissions:
+		// no sanity checks here only check if they changed
+		changed = !txtTitle.getText().equals(image.getImageName())
+				|| !txtDescription.getText().equals(image.getDescription())
+				|| newOs.getOsId() != image.getOsId() || !newShareMode.equals(image.shareMode)
+				|| chkIsTemplate.isSelected() != image.isTemplate
+				|| !image.defaultPermissions.equals(originalDefaultPermissions)
+				|| PermsHelper.hasChanged(originalCustomPermissions, customPermissions);
+		if (changed)
+			lblError.setText(null);
 		btnSaveChanges.setEnabled(changed);
 		return changed;
 	}
diff --git a/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/layout/ImageDetailsWindowLayout.java b/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/layout/ImageDetailsWindowLayout.java
index 5c836a15..b81474f2 100644
--- a/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/layout/ImageDetailsWindowLayout.java
+++ b/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/layout/ImageDetailsWindowLayout.java
@@ -1,6 +1,7 @@
 package org.openslx.dozmod.gui.window.layout;
 
 import java.awt.BorderLayout;
+import java.awt.Color;
 import java.awt.Font;
 import java.awt.Frame;
 import java.awt.GridBagConstraints;
@@ -36,6 +37,7 @@ public abstract class ImageDetailsWindowLayout extends JDialog {
 	protected final JTextField txtTitle;
 	protected final JTextArea txtDescription;
 
+	protected QLabel lblError;
 	protected final PersonLabel lblOwner;
 	protected final JButton btnChangeOwner;
 	protected final QLabel lblCreateTime;
@@ -51,9 +53,8 @@ public abstract class ImageDetailsWindowLayout extends JDialog {
 	protected final JTextField txtId;
 	protected final JTextField txtVersion;
 
-	
 	protected final JButton btnPermissions;
-	
+
 	protected final JButton btnSaveChanges;
 	protected final JButton btnUpdateImage;
 	protected final JButton btnClose;
@@ -88,11 +89,8 @@ public abstract class ImageDetailsWindowLayout extends JDialog {
 		txtDescription.setWrapStyleWord(true);
 		txtDescription.setMinimumSize(Gui.getScaledDimension(0, 100));
 		grid.add(new QLabel("Beschreibung")).anchor = GridBagConstraints.FIRST_LINE_START;
-		grid.add(
-				new JScrollPane(txtDescription, JScrollPane.VERTICAL_SCROLLBAR_AS_NEEDED,
-						JScrollPane.HORIZONTAL_SCROLLBAR_NEVER), 2)
-				.expand(true, true)
-				.fill(true, true);
+		grid.add(new JScrollPane(txtDescription, JScrollPane.VERTICAL_SCROLLBAR_AS_NEEDED,
+				JScrollPane.HORIZONTAL_SCROLLBAR_NEVER), 2).expand(true, true).fill(true, true);
 		grid.nextRow();
 
 		// owner
@@ -137,12 +135,6 @@ public abstract class ImageDetailsWindowLayout extends JDialog {
 		grid.add(cboOperatingSystem, 2).expand(true, false).fill(true, false);
 		grid.nextRow();
 
-		// virtualizer
-		lblVirtualizer = new QLabel();
-		grid.add(new QLabel("Virtualisierer"));
-		grid.add(lblVirtualizer, 2);
-		grid.nextRow();
-
 		// tags
 		txtTags = new JTextField();
 		/* TODO
@@ -175,12 +167,24 @@ public abstract class ImageDetailsWindowLayout extends JDialog {
 		grid.add(new QLabel("Image-UUID"));
 		grid.add(txtId, 2).expand(true, false).fill(true, false);
 		grid.nextRow();
-		
-		
+
+		// virtualizer
+		lblVirtualizer = new QLabel();
+		grid.add(new QLabel("Virtualisierer"));
+		grid.add(lblVirtualizer, 2);
+		grid.nextRow();
+
 		btnPermissions = new JButton("Berechtigungen");
-		grid.add(btnPermissions);
-		grid.skip(2);
+		grid.skip();
+		grid.add(btnPermissions, 2);
+		grid.nextRow();
+		grid.add(Box.createVerticalStrut(10), 3);
 		grid.nextRow();
+		// user feedback slot
+		lblError = new QLabel("");
+		lblError.setForeground(Color.RED);
+		grid.skip();
+		grid.add(lblError, 2);
 		grid.finish(true);
 
 		infoPanel.setPreferredSize(Gui.getScaledDimension(500, 400));