Vojtech Szocs has uploaded a new change for review.
Change subject: webadmin,userportal: Improve FormItem auto placement
......................................................................
webadmin,userportal: Improve FormItem auto placement
1. Modified FormItem constructors
- row + autoPlacementRow consolidated into row field
- created constructor without row argument as the row
value is now treated as non-final (can be assigned
later on)
2. Modified SubTabHostGeneralView to use new FormItem
constructors, no need for explicit withAutoPlacement
invocation
3. Modified AbstractFormPanel to address following edge
cases:
a, add item1 at col=0 -with- auto placement
- item1 row=0, nextAvailableRowForColumn[0]=1
b, add item2 at col=0,row=0 -without- auto placement
(!) edge case: item replacing already assigned row
- item2 row=0, nextAvailableRowForColumn[0]=1
c, add item3 at col=0,row=2 -without- auto placement
(!) edge case: item placed beyond assigned row
- item3 row=2, nextAvailableRowForColumn[0]=1
d, add item4 at col=0 -with- auto placement
- item4 row=1, nextAvailableRowForColumn[0]=3
e, add item5 at col=0 -with- auto placement
- item5 row=3, nextAvailableRowForColumn[0]=4
In short, combining items with vs. without auto placement
should work as expected, i.e. items without auto placement
will override any existing items at given location, items
with auto placement will accomodate next available location,
taking already assigned locations into consideration.
Change-Id: I226b449b608d54c106c84229598461ca09a837f8
Signed-off-by: Vojtech Szocs <[email protected]>
---
M
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/AbstractFormPanel.java
M
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/FormItem.java
M
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostGeneralView.java
3 files changed, 91 insertions(+), 60 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/06/22306/1
diff --git
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/AbstractFormPanel.java
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/AbstractFormPanel.java
index 7f8c3f2..53a77b8 100644
---
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/AbstractFormPanel.java
+++
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/AbstractFormPanel.java
@@ -2,8 +2,10 @@
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import com.google.gwt.uibinder.client.UiField;
import com.google.gwt.user.client.ui.Composite;
@@ -23,19 +25,46 @@
// List of detail views, each one representing a column of form items
private final List<Grid> detailViews = new ArrayList<Grid>();
- // Used with form item auto placement feature
- private Map<Integer, Integer> nextAvailableRowForColumn = new
HashMap<Integer, Integer>();
+ // Maps columns to next available row
+ private final Map<Integer, Integer> nextAvailableRowForColumn = new
HashMap<Integer, Integer>();
+
+ // Maps columns to all assigned rows
+ private final Map<Integer, Set<Integer>> assignedRowForColumn = new
HashMap<Integer, Set<Integer>>();
public int getNextAvailableRow(int column) {
- if(!nextAvailableRowForColumn.containsKey(column)) {
+ if (!nextAvailableRowForColumn.containsKey(column)) {
nextAvailableRowForColumn.put(column, 0);
}
return nextAvailableRowForColumn.get(column);
}
- void incNextAvailableRow(int column) {
- int curRow = getNextAvailableRow(column);
- nextAvailableRowForColumn.put(column, curRow + 1);
+ void updateAutoPlacementData(FormItem item) {
+ int itemRow = item.getRow();
+ int itemColumn = item.getColumn();
+
+ // Update assigned rows mapping
+ if (!assignedRowForColumn.containsKey(itemColumn)) {
+ assignedRowForColumn.put(itemColumn, new HashSet<Integer>());
+ }
+ assignedRowForColumn.get(itemColumn).add(itemRow);
+
+ // Get next available row
+ int nextRow = getNextAvailableRow(itemColumn);
+
+ // Increment next available row only if the item is to be
+ // added to that same row, this is to exclude two edge cases:
+ // a, item replacing already assigned row, i.e. itemRow < nextRow
+ // b, item placed beyond assigned row, i.e. itemRow > nextRow
+ if (itemRow == nextRow) {
+ nextRow++;
+
+ // Increment next available row if already assigned
+ while (assignedRowForColumn.get(itemColumn).contains(nextRow)) {
+ nextRow++;
+ }
+
+ nextAvailableRowForColumn.put(itemColumn, nextRow);
+ }
}
/**
@@ -67,7 +96,7 @@
updateFormItem(item);
// Update auto placement data
- incNextAvailableRow(item.getColumn());
+ updateAutoPlacementData(item);
}
/**
diff --git
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/FormItem.java
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/FormItem.java
index 82f8788..ceb8aec 100644
---
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/FormItem.java
+++
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/FormItem.java
@@ -22,7 +22,7 @@
private AbstractFormPanel formPanel;
- private int row;
+ private int row = UNASSIGNED_ROW;
private final int column;
private final String isAvailablePropertyName;
@@ -35,20 +35,27 @@
private DefaultValueCondition defaultValueCondition;
private boolean autoPlacement = false;
- private int autoPlacementRow = UNASSIGNED_ROW;
+ /**
+ * Creates a form item with {@linkplain #withAutoPlacement auto placement}.
+ */
+ public FormItem(String name, Widget valueWidget, int column, boolean
isAvailable) {
+ this(name, valueWidget, column, null, isAvailable);
+ withAutoPlacement();
+ }
+
+ /**
+ * Creates a form item with {@linkplain #withAutoPlacement auto placement}.
+ */
+ public FormItem(String name, Widget valueWidget, int column) {
+ this(name, valueWidget, column, true);
+ }
+
+ /**
+ * Creates an empty form item.
+ */
public FormItem(int row, int column) {
this("", new TextBoxLabel(), row, column); //$NON-NLS-1$
- }
-
- public FormItem(String name, Widget valueWidget, int column) {
- this(name, valueWidget, 0 , column, null, true);
- withAutoPlacement();
- }
-
- public FormItem(String name, Widget valueWidget, int column, boolean
isAvailable) {
- this(name, valueWidget, 0 , column, null, isAvailable);
- withAutoPlacement();
}
public FormItem(String name, Widget valueWidget, int row, int column) {
@@ -63,13 +70,8 @@
this(name, valueWidget, row, column, isAvailablePropertyName, true);
}
- public FormItem(String name,
- Widget valueWidget,
- int row,
- int column,
- String isAvailablePropertyName,
- boolean isAvailable) {
- this.row = row;
+ public FormItem(String name, Widget valueWidget, int column,
+ String isAvailablePropertyName, boolean isAvailable) {
this.column = column;
this.isAvailablePropertyName = isAvailablePropertyName;
this.valueWidget = valueWidget;
@@ -81,6 +83,12 @@
} else {
this.name = name;
}
+ }
+
+ public FormItem(String name, Widget valueWidget, int row, int column,
+ String isAvailablePropertyName, boolean isAvailable) {
+ this(name, valueWidget, column, isAvailablePropertyName, isAvailable);
+ this.row = row;
}
/**
@@ -95,10 +103,8 @@
/**
* Automatically place this form item into next available row for the
given column.
* <p>
- * This works only when the form item is {@linkplain #getIsAvailable
available} when being added to the
- * {@link AbstractFormPanel}. Otherwise, the form item is marked as dead
and will be discarded by the form panel.
- *
- * @see #setFormPanel(AbstractFormPanel)
+ * This works only when the form item is {@linkplain #getIsAvailable
available} when being added to the form panel.
+ * Otherwise, the form item will be discarded by the form panel.
*/
public FormItem withAutoPlacement() {
this.autoPlacement = true;
@@ -108,21 +114,17 @@
public void setFormPanel(AbstractFormPanel formPanel) {
this.formPanel = formPanel;
- // Compute autoPlacementRow if this item is available
+ // Compute auto placement row if this item is available
if (autoPlacement && getIsAvailable()) {
- this.autoPlacementRow = formPanel.getNextAvailableRow(column);
+ this.row = formPanel.getNextAvailableRow(column);
}
}
/**
- * If {@code false}, this form item shouldn't be processed by the {@link
AbstractFormPanel}.
+ * If {@code false}, this form item should be discarded by the form panel.
*/
public boolean isValid() {
- if (autoPlacement && autoPlacementRow == UNASSIGNED_ROW) {
- // Failed to assign autoPlacementRow
- return false;
- }
- return true;
+ return row != UNASSIGNED_ROW;
}
public void update() {
@@ -131,7 +133,7 @@
}
public int getRow() {
- return (autoPlacement && autoPlacementRow != UNASSIGNED_ROW) ?
autoPlacementRow : row;
+ return row;
}
public int getColumn() {
diff --git
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostGeneralView.java
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostGeneralView.java
index 0441a13..5f6fc2a 100644
---
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostGeneralView.java
+++
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostGeneralView.java
@@ -136,29 +136,29 @@
// Build a form using the FormBuilder
formBuilder = new FormBuilder(formPanel, 3, 7);
- formBuilder.addFormItem(new FormItem(constants.osVersionHostGeneral(),
oS, 0).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.kernelVersionHostGeneral(), kernelVersion,
0).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.kvmVersionHostGeneral(), kvmVersion, 0,
virtSupported).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.libvirtVersionHostGeneral(), libvirtVersion, 0,
virtSupported).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.vdsmVersionHostGeneral(), vdsmVersion,
0).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.spiceVersionHostGeneral(), spiceVersion, 0,
virtSupported).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.glusterVersionHostGeneral(), glusterVersion, 0,
glusterSupported).withAutoPlacement());
+ formBuilder.addFormItem(new FormItem(constants.osVersionHostGeneral(),
oS, 0));
+ formBuilder.addFormItem(new
FormItem(constants.kernelVersionHostGeneral(), kernelVersion, 0));
+ formBuilder.addFormItem(new
FormItem(constants.kvmVersionHostGeneral(), kvmVersion, 0, virtSupported));
+ formBuilder.addFormItem(new
FormItem(constants.libvirtVersionHostGeneral(), libvirtVersion, 0,
virtSupported));
+ formBuilder.addFormItem(new
FormItem(constants.vdsmVersionHostGeneral(), vdsmVersion, 0));
+ formBuilder.addFormItem(new
FormItem(constants.spiceVersionHostGeneral(), spiceVersion, 0, virtSupported));
+ formBuilder.addFormItem(new
FormItem(constants.glusterVersionHostGeneral(), glusterVersion, 0,
glusterSupported));
- formBuilder.addFormItem(new FormItem(constants.spmPriority(),
spmPriority, 0, 1, virtSupported).withAutoPlacement());
- formBuilder.addFormItem(new FormItem(constants.activeVmsHostGeneral(),
activeVms, 1, virtSupported).withAutoPlacement());
- formBuilder.addFormItem(new FormItem(constants.cpuNameHostGeneral(),
cpuName, 1, virtSupported).withAutoPlacement());
- formBuilder.addFormItem(new FormItem(constants.cpuTypeHostGeneral(),
cpuType, 1).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.numOfSocketsHostGeneral(), numberOfSockets,
1).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.numOfCoresPerSocketHostGeneral(), coresPerSocket,
1).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.numOfThreadsPerCoreHostGeneral(), threadsPerCore,
1).withAutoPlacement());
+ formBuilder.addFormItem(new FormItem(constants.spmPriority(),
spmPriority, 1, virtSupported));
+ formBuilder.addFormItem(new FormItem(constants.activeVmsHostGeneral(),
activeVms, 1, virtSupported));
+ formBuilder.addFormItem(new FormItem(constants.cpuNameHostGeneral(),
cpuName, 1, virtSupported));
+ formBuilder.addFormItem(new FormItem(constants.cpuTypeHostGeneral(),
cpuType, 1));
+ formBuilder.addFormItem(new
FormItem(constants.numOfSocketsHostGeneral(), numberOfSockets, 1));
+ formBuilder.addFormItem(new
FormItem(constants.numOfCoresPerSocketHostGeneral(), coresPerSocket, 1));
+ formBuilder.addFormItem(new
FormItem(constants.numOfThreadsPerCoreHostGeneral(), threadsPerCore, 1));
- formBuilder.addFormItem(new FormItem(constants.physMemHostGeneral(),
physicalMemoryDetails, 2).withAutoPlacement());
- formBuilder.addFormItem(new FormItem(constants.swapSizeHostGeneral(),
swapSizeDetails, 2).withAutoPlacement());
- formBuilder.addFormItem(new FormItem(constants.sharedMemHostGeneral(),
sharedMemory, 2).withAutoPlacement());
- formBuilder.addFormItem(new FormItem(constants.maxSchedulingMemory(),
maxSchedulingMemory, 2, virtSupported).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.memPageSharingHostGeneral(), memoryPageSharing,
2).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.autoLargePagesHostGeneral(), automaticLargePage,
2).withAutoPlacement());
- formBuilder.addFormItem(new
FormItem(constants.isciInitNameHostGeneral(), iScsiInitiatorName, 2,
virtSupported).withAutoPlacement());
+ formBuilder.addFormItem(new FormItem(constants.physMemHostGeneral(),
physicalMemoryDetails, 2));
+ formBuilder.addFormItem(new FormItem(constants.swapSizeHostGeneral(),
swapSizeDetails, 2));
+ formBuilder.addFormItem(new FormItem(constants.sharedMemHostGeneral(),
sharedMemory, 2));
+ formBuilder.addFormItem(new FormItem(constants.maxSchedulingMemory(),
maxSchedulingMemory, 2, virtSupported));
+ formBuilder.addFormItem(new
FormItem(constants.memPageSharingHostGeneral(), memoryPageSharing, 2));
+ formBuilder.addFormItem(new
FormItem(constants.autoLargePagesHostGeneral(), automaticLargePage, 2));
+ formBuilder.addFormItem(new
FormItem(constants.isciInitNameHostGeneral(), iScsiInitiatorName, 2,
virtSupported));
}
void initMemorySizeLabels() {
--
To view, visit http://gerrit.ovirt.org/22306
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I226b449b608d54c106c84229598461ca09a837f8
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
