Lior Vernia has posted comments on this change.

Change subject: userportal,webadmin: type ahead list box
......................................................................


Patch Set 8:

I assure you I did not misunderstand anything. That is exactly what I figured 
out this widget does, and that's why it's not like a ListModelSuggestBox. The 
whole point of ListModelSuggestBox is that you can input a text that's not part 
of the predefined constrained values. This was the sole purpose of 
ListModelSuggestBox and the only reason we needed to add it.

If, as you said, the widget "does not create business entities but uses exactly 
the same principle as the e.g. ListModelListBox", then you should perhaps be 
inheriting from ListModelListBox and using a SuggestBox member (there's no 
necessity to inherit from SuggestBox, you can just have it as a member and 
delegate to it!) or even just a SuggestOracle. What you're describing is not 
similar to the behaviour of ListModelSuggestBox.

So again, I am not okay with how it's implemented at the moment, because as I 
said all along, it has nothing in common with ListModelSuggestBox other than 
that they both need a SuggestOracle. I don't understand what's gained from this 
forced inheritance other than saving a couple of key handlers (and I strongly 
disagree with Michal that this is a good case for inheritance when the widgets 
behave differently).

And again, besides the design, it seems to me like this breaks 
ListModelSuggestBox (if someone tries to use it with any class other than 
Object) as well as HostBondPopupView (I think handler isn't set and would cause 
a null pointer exception). Did you try to add a new bond and saw that it worked?

-- 
To view, visit http://gerrit.ovirt.org/14936
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I956af3c675894c850a1a104a81cec49f4bd62011
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to