> 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" <m...@redhat.com> > To: "Lior Vernia" <lver...@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" <lver...@redhat.com> > > To: "Einav Cohen" <eco...@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" <lver...@redhat.com> > > >> To: "Itamar Heim" <ih...@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" <lver...@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 > > > _______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
