Lior Vernia has posted comments on this change.
Change subject: webadmin: allow to extend KeyValueModel
......................................................................
Patch Set 2:
(6 comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/key_value/KeyValueWidget.java
Line 82: model.updateKeys();
Line 83: widgets.remove(widget);
Line 84: }
Line 85:
Line 86: public void updateRowWidth(String rowWidth) {
There's something strange about this method, it has to be called before edit()
is called to affect all line widgets. I would create another constructor that
also accepts the width as argument and use it as part of the affinity group
feature (I would leave the no-arg constructor in order to allow people to use
the widget without having to state @UiField(provided = true)).
Line 87: this.rowWidth = rowWidth;
Line 88: }
Line 89:
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/key_value/BaseKeyModel.java
Line 27: protected void init(Set<String> allKeys, Set<String> usedKeys) {
Line 28: this.allKeys = new HashSet<String>(allKeys);
Line 29: this.usedKeys = new HashSet<String>(usedKeys);
Line 30: List<KeyValueLineModel> list = new
ArrayList<KeyValueLineModel>();
Line 31: KeyValueLineModel lineModel;
No reason for this to be outside of the loop, you can inline it with
createNewLineModel(key).
Line 32: disableEvent = true;
Line 33: for (String key : usedKeys) {
Line 34: lineModel = createNewLineModel(key);
Line 35: lineModel.getKeys().setSelectedItem(key);
Line 41: }
Line 42:
Line 43: protected abstract void initLineModel(KeyValueLineModel lineModel,
String key);
Line 44:
Line 45: protected abstract void fillLineModel(KeyValueLineModel lineModel,
String key);
The name doesn't explain well what this does, I would name the method
setValueByKey() - assuming I understood its function correctly.
Line 46:
Line 47: public final IEventListener keyChangedListener = new
IEventListener() {
Line 48:
Line 49: @Override
Line 80: public boolean isKeyValid(String key) {
Line 81: return !(key == null || key.equals(selectKey) ||
key.equals(noKeys));
Line 82: }
Line 83:
Line 84: public List<String> getAvailableKeys(String key) {
This can be private.
Line 85: List<String> list = getAvailableKeys();
Line 86: boolean realKey = isKeyValid(key);
Line 87: if (realKey && !list.contains(key)) {
Line 88: list.add(0, key);
Line 101:
Line 102: return list;
Line 103: }
Line 104:
Line 105: public List<String> getAvailableKeys() {
This too.
Line 106: List<String> list =
Line 107: (allKeys == null) ? new LinkedList<String>() : new
LinkedList<String>(allKeys);
Line 108: list.removeAll(getUsedKeys());
Line 109: if (list.size() > 0) {
Line 122: return new ArrayList<String>(usedKeys);
Line 123: }
Line 124: }
Line 125:
Line 126: public int possibleKeysCount() {
This can be removed altogether.
Line 127: return allKeys == null ? 0 : allKeys.size();
Line 128: }
Line 129:
Line 130: public void updateKeys() {
--
To view, visit http://gerrit.ovirt.org/22926
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I92d49a9e3a6099e0e082939bee61ccf484545263
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
