anmolbabu has posted comments on this change. Change subject: webadmin: fix NPE and visibility issues in CheckBoxGroup ......................................................................
Patch Set 1: (4 comments) https://gerrit.ovirt.org/#/c/40798/1/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/CheckBoxGroup.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/CheckBoxGroup.java: Line 145: */ Line 146: @Override Line 147: public void setValue(List<T> value, boolean fireEvents) { Line 148: List<T> selectedItems = getValue(); Line 149: if (value == selectedItems why is this required? Line 150: || (selectedItems != null && selectedItems.equals(value))) { Line 151: return; Line 152: } Line 153: clearAllSelections(); Line 152: } Line 153: clearAllSelections(); Line 154: if(value == null){ Line 155: return; Line 156: } please place this as first check Line 157: for (T currentvalue : value) { Line 158: if (checkBoxes.containsKey(currentvalue)) { Line 159: checkBoxes.get(currentvalue).setValue(true); Line 160: } Line 176: * @return void Line 177: */ Line 178: @Override Line 179: public void setAcceptableValues(Collection<List<T>> values) { Line 180: List<T> seletedItems = getValue(); are you sure getValue will always be non-null? is it because in this widget everything is selected by default? but I assume you might be doing clearSelections please verify that it works in snapshot popup also. Line 181: wrapperPanel.clear(); Line 182: checkBoxes.clear(); Line 183: if (values.isEmpty()) { Line 184: throw new IllegalArgumentException("Widget has nothing to do");//$NON-NLS-1$ Line 195: private void showCheckBoxes(List<T> seletedItems) { Line 196: for (Entry<T, CheckBox> currentEntry : checkBoxes.entrySet()) { Line 197: wrapperPanel.add(currentEntry.getValue()); Line 198: if (seletedItems.contains(currentEntry.getKey())) { Line 199: currentEntry.getValue().setValue(true, false); you could simply do currentEntry.getValue().setValue(true) Line 200: } Line 201: } Line 202: } Line 203: -- To view, visit https://gerrit.ovirt.org/40798 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f1d371d6c6cbecad9642641d762e954016105d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ramesh N <rnach...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Ramesh N <rnach...@redhat.com> Gerrit-Reviewer: anmolbabu <anb...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
