On 05/29/2013 10:03 AM, Tomas Jelinek wrote:
..
Daniel have started to review the instance types related patches and has two
concerns about the static header containing DC, Cluster etc.:
1: does not see the point of showing DC, Cluster etc for most of the side tabs
(e.g. console)
2: if there is indeed a point, why not change all the dialogs with side tabs to
have the static header e.g. new cluster. (please note that the currently
reviewed patch implements this static header to all VM related dialogs like
pool,template,vm)
may make sense to review for other dialogs as well. not sure all of them
have the same concept of multiple fields affected by the instance type
in various panes though.
...
having Daniel on our instance types meetings from the beginning, making him the
part of the feature and having this
questions already discussed, so by now he would be perfectly comfortable with
the design, understand the motivation and agreed with it,
so now he would only review the specific implementation (while he would already
know my important code-design decisions and be part of them).
Don't get me wrong Daniel, I find your concerns extremely valid and worth
discussing and thank you for rising them!
since there was a feature page, and some discussions on engine-devel
around this, maybe considering the aspects of the change, would have
been worth while sending to arch@ an invite to review it more thoroughly
at concept level (not all features need this prior to patch level, but
big ones/conceptual ones need them)
----- Original Message -----
From: "Daniel Erez" <de...@redhat.com>
To: "Einav Cohen" <eco...@redhat.com>
Cc: "Malini Rao" <m...@redhat.com>, "Eldan Hildesheim" <i...@eldanet.com>, "Eldan
Hildesheim" <ehild...@redhat.com>,
"Tomas Jelinek" <tjeli...@redhat.com>, "engine-devel" <engine-devel@ovirt.org>
Sent: Wednesday, May 29, 2013 7:36:03 AM
Subject: Re: static header only in VM dialog?
----- Original Message -----
From: "Einav Cohen" <eco...@redhat.com>
To: "Daniel Erez" <de...@redhat.com>, "Malini Rao" <m...@redhat.com>,
"Eldan Hildesheim" <i...@eldanet.com>, "Eldan
Hildesheim" <ehild...@redhat.com>, "Tomas Jelinek" <tjeli...@redhat.com>
Cc: "engine-devel" <engine-devel@ovirt.org>
Sent: Wednesday, May 29, 2013 2:30:09 AM
Subject: Re: static header only in VM dialog?
----- Original Message -----
From: "Daniel Erez" <de...@redhat.com>
Sent: Tuesday, May 28, 2013 6:18:20 PM
----- Original Message -----
From: "Einav Cohen" <eco...@redhat.com>
To: "Malini Rao" <m...@redhat.com>, "Eldan Hildesheim"
<i...@eldanet.com>,
"Eldan Hildesheim" <ehild...@redhat.com>
Cc: "Tomas Jelinek" <tjeli...@redhat.com>, de...@redhat.com,
"engine-devel"
<engine-devel@ovirt.org>
Sent: Wednesday, May 29, 2013 12:53:47 AM
Subject: static header only in VM dialog?
when reviewing the code for the "redesign of vm related dialogs",
Daniel
has
raised an interesting question: Why do we want the static header only
in
VM
dialog?
(I assume that he refers to the top section of the New VM dialog as
seen
in
[1], which
contains the DC/Cluster, Quota, etc information, and is "static", i.e.,
it
is
always
displayed, regardless of the selected side-tab within the dialog)
I agree with what Daniel is implying here: for consistency, we would
probably
want to
introduce this static header to other dialogs, at least to the ones
that
also
contain
side-tabs in which it makes sense to turn the "header" to static
[e.g.
"New Host" (which contains a DC + Cluster "header") - see
http://oi39.tinypic.com/2z84xnp.jpg,
"New Cluster" (which contains a DC "header") - see
http://oi40.tinypic.com/2vmyj2x.jpg]
[I assume, of course, that all the VM-like dialogs (e.g. New/Edit
VM-Pool)
will also have
static headers - I don't know if the patch already takes care of that
or
not]
Thoughts?
[@Daniel - if you had something else in mind, and/or other dialogs in
mind
-
please feel free
to add/amend/etc]
Besides consistency matters, I wanted to understand what's the motivation
of
keeping these widgets static. I.e. is it an essential preparation for the
new
instance type dialog or a new concept for tab based dialogs
(as DC/Cluster values aren't necessarily relevant for each tab in the
dialog)
[maybe Tomas/Eldan/Malini can help here as well]
these static widgets (as well as the type ahead list box [1], for example)
are part of
the instance types feature that had its design details published in [2] and
[3], and the
implementation was done according to this design. I don't see any reason to
not utilize
this newly-introduced concept in other side-tab-based dialogs as well, if
it
makes sense
(ui consistency considerations are sufficient, IMO, but I could be wrong -
maybe it is
relevant/correct to introduce this new concept only in the VM-like
dialogs).
regarding the specific concern about the DC/Cluster values that aren't
necessarily relevant
for each side-tab in the dialog: I agree with that statement, however:
- putting the "Instance Type" drop-down at the top static section is very
useful (see [4]
for explanation), and as the Instance Types list is derived from the
selected
DC, it makes
sense (to me) to put the DC in that top static section as well.
- the DC/Cluster are relevant for some of the tabs in the dialog (Host,
Resource Allocation?)
Only for Host.
Resource Allocation is directly affected by the selected template.
Therefore, it sounds very confusing to me...
Unless we add template to the static header as well?
(which will be odd for the other tabs).
So I still don't get the motivation UX-wise.
E.g. it seems really weird to change the entire DC from Console tab
(or, as a matter of fact, from most other tabs).
In the new instance type dialog, which tabs could be directly affected by
DC/Cluster?
IIUC, only Host? Do we really need a static header just for this tab?
so for consistency-within-the-dialog considerations, it is probably a good
idea to simply
always show these fields within this top static section.
[there is a good chance that I am missing your point here - please correct
me
if necessary]
[1] http://gerrit.ovirt.org/#/c/14936/
[2] http://www.ovirt.org/Features/Instance_Types
[3] http://www.ovirt.org/images/9/9e/Instance_type.pdf
[4] whenever changing the "Instance Type" value, you can automatically see
how these changes
affect the fields in the current tab on which you are standing (e.g. if you
are standing on
the "System" side-tab, you can change the "Instance Type" selected item and
immediately see
the changes within the "System" side-tab contents), and vice-versa: if you
are changing a value
that was originally propagated from the instance-type, you will see the
instance-type automatically
change to "custom"/"not applicable" as a result, so no need to "jump"
between
side-tabs in order
to observe these changes.
----
Thanks,
Einav
[1] http://www.ovirt.org/images/9/9e/Instance_type.pdf
----- Original Message -----
From: de...@redhat.com
To: "Tomas Jelinek" <tjeli...@redhat.com>
Cc: "Vojtech Szocs" <vsz...@redhat.com>, "Einav Cohen"
<eco...@redhat.com>,
"Frank Kobzik" <fkob...@redhat.com>,
"Eldan Hildesheim" <i...@eldanet.com>
Sent: Tuesday, May 28, 2013 5:05:38 PM
Subject: Change in ovirt-engine[master]: userportal,webadmin:
redesign
of
vm related dialogs
Daniel Erez has posted comments on this change.
Change subject: userportal,webadmin: redesign of vm related dialogs
......................................................................
Patch Set 5: (1 inline comment)
Code looks good.
A few questions regarding the design:
1. Why do we want the static header only in VM dialog?
2. DC/Host are really relevant for all tabs?
3. Is it just a preparation for the final instance type dialog?
4. If it's indeed merely preparation, shouldn't it be merged only
once
we
have the full picture of the new dialog?
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/dialog/tab/DialogTabPanel.ui.xml
Line 21:
Line 22: .header {
Line 23: background-color: #D3D3D3;
Line 24: border-bottom: 1px solid #CED8DF;
Line 25: margin-bottom: 15px;
is it supposed to be that large?
Line 26: padding-top: 6px;
Line 27: margin-top: 4px;
Line 28: margin-right: 3px;
Line 29: display: none;
--
To view, visit http://gerrit.ovirt.org/14635
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icad8098e286f821da25fac22fd0a840a42f105c9
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Eldan Hildesheim <i...@eldanet.com>
Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
_______________________________________________
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
