Hello Shmuel Melamud, I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/40782 to review the following change. Change subject: ui: Incorrect selection reset in EntityModelCellTable ...................................................................... ui: Incorrect selection reset in EntityModelCellTable When ListModel is used together with EntityModelCellTable, change of selection in the ListModel causes corresponding change of selection in EntityModelCellTable. Selection change handler in EntityModelCellTable sets selection in the ListModel to null, planning to assign the new selection afterwards. This causes another loop of events. As the result, selection in EntityModelCellTable is set to null before it is read by the event handler. To fix this, the event handler is changed to get the selection before resetting the ListModel. Also, additional checks are added to avoid resetting the ListModel when it is already set to the desired value. This eliminates extra events and side effects of their handlers. Change-Id: I8c6fd184c567aee993737112bcb45135c83dc1cb Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1220142 Signed-off-by: Shmuel Melamud <smela...@redhat.com> --- M frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java 1 file changed, 29 insertions(+), 12 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/82/40782/1 diff --git a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java index a418331..8f8dacb 100644 --- a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java +++ b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java @@ -3,6 +3,7 @@ import java.util.ArrayList; import java.util.List; +import org.ovirt.engine.core.common.utils.ListUtils; import org.ovirt.engine.ui.common.CommonApplicationConstants; import org.ovirt.engine.ui.common.PopupTableResources; import org.ovirt.engine.ui.common.gin.AssetProvider; @@ -223,34 +224,50 @@ return; } - getListModel().setSelectedItems(null); - getListModel().setSelectedItem(null); - // Clear "IsSelected" - for (EntityModel entity : (List<EntityModel>) getListModel().getItems()) { - entity.setIsSelected(false); - } - // Set "IsSelected" SelectionModel<? super EntityModel> selectionModel = EntityModelCellTable.this.getSelectionModel(); if (selectionModel instanceof SingleSelectionModel) { EntityModel selectedObject = ((SingleSelectionModel<EntityModel>) selectionModel).getSelectedObject(); - if (selectedObject != null) { - selectedObject.setIsSelected(true); - getListModel().setSelectedItem(selectedObject); + if (getListModel().getSelectedItem() != selectedObject) { + resetListModelSelection(); + if (selectedObject != null) { + selectedObject.setIsSelected(true); + getListModel().setSelectedItem(selectedObject); + } } } else if (selectionModel instanceof MultiSelectionModel) { List<EntityModel> selectedItems = new ArrayList<EntityModel>(); for (EntityModel entity : ((MultiSelectionModel<EntityModel>) selectionModel).getSelectedSet()) { - entity.setIsSelected(true); selectedItems.add(entity); } - getListModel().setSelectedItems(selectedItems); + List<EntityModel> prevSelectedItems = getListModel().getSelectedItems(); + if (prevSelectedItems == null && selectedItems.size() != 0 || + prevSelectedItems != null && !ListUtils.listsEqual(prevSelectedItems, selectedItems)) { + resetListModelSelection(); + getListModel().setSelectedItems(selectedItems); + for (EntityModel entity : selectedItems) { + entity.setIsSelected(true); + } + } } } }); } + private void resetListModelSelection() { + if (getListModel() == null || getListModel().getItems() == null) { + return; + } + + getListModel().setSelectedItems(null); + getListModel().setSelectedItem(null); + // Clear "IsSelected" + for (EntityModel entity : (List<EntityModel>) getListModel().getItems()) { + entity.setIsSelected(false); + } + } + private void addCheckBoxColumn(boolean hideCheckbox, boolean showSelectAllCheckbox) { if (!hideCheckbox) { -- To view, visit https://gerrit.ovirt.org/40782 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8c6fd184c567aee993737112bcb45135c83dc1cb Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shmuel Leib Melamud <smela...@redhat.com> Gerrit-Reviewer: Shmuel Melamud <smela...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches