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.
----- Original Message -----
> From: "Lior Vernia" <lvernia(a)redhat.com>
> To: "Itamar Heim" <iheim(a)redhat.com>
> Cc: "engine-devel" <engine-devel(a)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(a)redhat.com>
>>>>>>> To: "engine-devel" <engine-devel(a)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(a)ovirt.org
>>>>>>>
http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Engine-devel mailing list
>>>>>>> Engine-devel(a)ovirt.org
>>>>>>>
http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>>
>>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
>
http://lists.ovirt.org/mailman/listinfo/engine-devel
>
>