If all the attached examples are orderless, why do 3 of them have the + button on every row?
+1. it seems that all widgets should contain the "+" button only in the last row.
I think it will be a good idea to lock down the behavior for ordered and order-less rows as Einav mentioned for consistency.
assuming everyone are OK with adjusting all existing widgets (which are all orderless, as was mentioned here) so that they will contain the "+" button only in the last row, then we currently don't even need to introduce the "ordered" flavor to AddRemoveRowWidget.
I understand that the row content can be different based on the use case but we need to have some basic commonalities - For e.g, in the VNic Profile example, we should have a label for the first text field as it is not really clear what that starting empty text field is for - I can deduce it is for the Profile name but can't be sure.
+1. We should think of a consistent way of "labeling" the different parts within a row / provide an explanation for a row (for all of the row types that we currently have). Today there is a bit of a mess: In the vNIC assignment, there is a top explanation of "VM has x network interfaces. Assign profiles to them". In vNIC profiles, there isn't a top explanation, but the check-box and drop-down are labeled, however the text-box, as you mentioned, is not. In the Profiles/Custom-Properties, there are no explanations and no labels at all. ----- Original Message -----
From: "Malini Rao" <mrao@redhat.com> To: "Lior Vernia" <lvernia@redhat.com> Cc: "engine-devel" <engine-devel@ovirt.org> Sent: Friday, October 25, 2013 4:01:13 PM Subject: Re: [Engine-devel] GUI widget for adding/removing entries
----- Original Message -----
From: "Lior Vernia" <lvernia@redhat.com> To: "Einav Cohen" <ecohen@redhat.com> Cc: "engine-devel" <engine-devel@ovirt.org> Sent: Sunday, October 13, 2013 10:03:30 AM Subject: Re: [Engine-devel] GUI widget for adding/removing entries
On 10/10/13 16:01, Einav Cohen wrote:
see attached: AFAIK, there are three types of adding-and-removing widget in the application:
(1) the one that exists in the customs properties section as well as the cluster policy. in this widget: - the "+" and "-" buttons appear on every row. - row is "identified" by a selected item from a drop down. - input controls in row: drop-down and text box (that appears upon selection of a non-empty value in the drop-down)
(2) the one that exists in the vNICs assignment part of the General section in the New VM dialog. in this widget: - the "+" appears only in the last row, "-" appears in all lines but the last row. - row is "identified" by (what seems to be a) read-only label (I assume that this widget is built to be initially loaded to already contain a number of rows, as opposed to (1), which typically starts with no rows / initial "empty" row. - input controls in row: drop-down
(3) the one that exists in the vNIC profiles section in the Add/Edit Network dialog in this widget: - the "+" and "-" buttons appear on every row. - row is "identified" by a free-text string. - input controls in row: text-box, check-box and drop-down.
so there are differences between the widgets other than the "+" and "-" buttons. however, from ux perspective, it is important to keep the look-and-feel of all of them consistent.
technically (code-wise), I am not sure how easy it is to merge the three, due to the differences. we can maybe think of creating a general adding-and-removing-entries widget, which can support "ordered" and "non-ordered" flavors (which will affect the "+"/"-" buttons appearance / exact behavior), and it will contain a collection of "abstract" row-widgets (and we will have several implementations of row-widgets for each needed functionality [(1), (2), (3)] with exact appearance / input controls / behavior/etc.), which may need to support a certain api (e.g. isRowEmpty(), get/set Identifier(), etc.) in order to communicate appropriately with its "parent" adding-and-removing-entries widget.
thoughts?
What you described is pretty much how the abstract AddRemoveRowWidget works, so it should be fairly easy to merge the three (it was designed to be so). The widget doesn't care about the exact nature of the content widget to the left of the plus/minus button, that information is given when overriding the aforementioned abstract methods. The only difference is that there's only one flavor at the moment, which is orderless, since none of these current examples is mindful of ordering.
If all the attached examples are orderless, why do 3 of them have the + button on every row? I think it will be a good idea to lock down the behavior for ordered and order-less rows as Einav mentioned for consistency. I understand that the row content can be different based on the use case but we need to have some basic commonalities - For e.g, in the VNic Profile example, we should have a label for the first text field as it is not really clear what that starting empty text field is for - I can deduce it is for the Profile name but can't be sure.
----- Original Message -----
From: "Lior Vernia" <lvernia@redhat.com> To: "Itamar Heim" <iheim@redhat.com> Cc: "engine-devel" <engine-devel@ovirt.org> Sent: Thursday, October 10, 2013 4:44:47 AM Subject: Re: [Engine-devel] GUI widget for adding/removing entries
To my knowledge, such a widget existed only in two other places: custom properties and vNIC profiles in add/edit network dialog. In both of them the order wasn't important, in which case the new widget is probably preferable. If it is indeed preferable (Einav? Malini?), I could do some refactoring to have both of them use it.
On 10/10/13 09:02, Itamar Heim wrote:
On 10/10/2013 10:59 AM, Lior Vernia wrote:
On 09/10/13 23:34, Itamar Heim wrote: > On 10/09/2013 03:32 PM, Lior Vernia wrote: >> Of course, my bad. Attached is a screenshot of the add/edit VM >> dialog, >> note the vNIC part on the bottom half of the dialog. > > how is it different from the custom properties one? >
Design-wise, there are a couple of small differences. There's only one button next to each row, plus if it's the last row or minus otherwise (so items can only be added at the end, as I replied to Malini order hasn't been important so far). A row appears as disabled until it is edited, and a disabled row is ignored when the view is flushed back to the model (e.g. when the user presses OK in the dialog).
Code-wise, it's constructed to be reusable, which the custom properties widget wasn't :)
could we converge on one of them though?
>> >> On 09/10/13 13:24, Einav Cohen wrote: >>> Hi Lior - can you please provide a screen-shot, so we will know >>> which >>> widget >>> you are referring to? >>> will make it easier for people to decide if and where to use this >>> widget. >>> >>> Many thanks! >>> >>> ---- >>> Regards, >>> Einav >>> >>> ----- Original Message ----- >>>> From: "Lior Vernia" <lvernia@redhat.com> >>>> To: "engine-devel" <engine-devel@ovirt.org> >>>> Sent: Wednesday, October 9, 2013 4:34:29 AM >>>> Subject: [Engine-devel] GUI widget for adding/removing entries >>>> >>>> Hello, >>>> >>>> Lately a patch has been merged that introduces a widget for >>>> adding/removing entries (e.g. network interfaces when >>>> creating/editing a >>>> VM): >>>> >>>> http://gerrit.ovirt.org/#/c/19530/ >>>> >>>> This kind of widgets is becoming common in oVirt, so the idea is >>>> to >>>> make >>>> adding one easy rather than copying & pasting code. >>>> AddRemoveRowWidget >>>> takes care of the plus/minus button logic, disabling an entry that >>>> hasn't been edited, and the arranging in rows. >>>> >>>> In order to use it, one is required to override a couple of >>>> abstract >>>> methods that are dependent upon the specific entry implementation. >>>> An >>>> example may be found in ProfilesInstanceTypeEditor, which handles >>>> adding/removing network interfaces in the new/edit VM dialog. >>>> >>>> Yours, Lior. >>>> _______________________________________________ >>>> Engine-devel mailing list >>>> Engine-devel@ovirt.org >>>> http://lists.ovirt.org/mailman/listinfo/engine-devel >>>> >>>> >>>> >>>> _______________________________________________ >>>> Engine-devel mailing list >>>> Engine-devel@ovirt.org >>>> http://lists.ovirt.org/mailman/listinfo/engine-devel >
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel