Hi,

> I suggest that we will proceed with the patches that were already written as 
> they are, and 
> also write the upcoming patches in the same manner (i.e. explicitly 
> "enableColumnResizing()" 
> each one of the widget instances + set the widths), verify that everything is 
> working/looking OK, 
> and only then - create the patch that replaces the "enableColumnResizing()" 
> method with the 
> "disbaleColumnResizing()" method (and removes all calls to 
> "enableColumnResizing()"), which will 
> actually be only a code-change/refactoring patch (i.e. will have no affect on 
> the application behavior).

ack

[I'd suggest to verify that addColumn() is always called with explicit width 
first, before changing enable -> disable API]

Vojtech


----- Original Message -----
From: "Einav Cohen" <eco...@redhat.com>
To: "Daniel Erez" <de...@redhat.com>, "Vojtech Szocs" <vsz...@redhat.com>, 
"Gilad Chaplik" <gchap...@redhat.com>
Cc: "engine-devel" <engine-devel@ovirt.org>
Sent: Thursday, April 25, 2013 12:33:06 PM
Subject: Re: [Engine-devel] enable column resizing on all sub-tabs?

> ----- Original Message -----
> From: "Daniel Erez" <de...@redhat.com>
> Sent: Thursday, April 25, 2013 6:22:02 AM
> 
> 
> ----- Original Message -----
> > From: "Einav Cohen" <eco...@redhat.com>
> > To: "Vojtech Szocs" <vsz...@redhat.com>
> > Cc: "Daniel Erez" <de...@redhat.com>, "Alona Kaplan" <alkap...@redhat.com>,
> > "Tal Nisan" <tni...@redhat.com>,
> > gchap...@redhat.com, "engine-devel" <engine-devel@ovirt.org>
> > Sent: Thursday, April 25, 2013 10:30:48 AM
> > Subject: enable column resizing on all sub-tabs?
> > 
> > Vojtech[/others] - what do you think about Gilad's comment below?
> > would it be possible / more correct to activate column resizing within the
> > base grid class itself by default (or something similar), rather than
> > enable
> > it separately for each grid?
> > 
> > [@Gilad - keep in mind that IIUC - each column in each grid still needs to
> > have
> > its width explicitly set]
> 
> We probably should just replace enableColumnResizing() with
> disbaleColumnResizing()
> in favor of the problematic sub-tabs. But I think it can wait till we're done
> adding the support (i.e. set width explicitly for each column).
> 
> What do you think?

indeed - no matter how we decide to exactly approach this: if the general 
approach is to enable 
column resizing by default in the base widget class, we need to take care of 
preparing the widget 
instances first, i.e., set their columns' widths.

I suggest that we will proceed with the patches that were already written as 
they are, and 
also write the upcoming patches in the same manner (i.e. explicitly 
"enableColumnResizing()" 
each one of the widget instances + set the widths), verify that everything is 
working/looking OK, 
and only then - create the patch that replaces the "enableColumnResizing()" 
method with the 
"disbaleColumnResizing()" method (and removes all calls to 
"enableColumnResizing()"), which will 
actually be only a code-change/refactoring patch (i.e. will have no affect on 
the application behavior).

acks/nacks/comments?

> 
> > 
> > ----
> > Thanks,
> > Einav
> > 
> > ----- Original Message -----
> > > From: gchap...@redhat.com
> > > To: "Daniel Erez" <de...@redhat.com>
> > > Cc: "Einav Cohen" <eco...@redhat.com>, "Vojtech Szocs"
> > > <vsz...@redhat.com>,
> > > "Alona Kaplan" <alkap...@redhat.com>,
> > > "Tal Nisan" <tni...@redhat.com>
> > > Sent: Thursday, April 25, 2013 3:23:56 AM
> > > Subject: Change in ovirt-engine[master]: webadmin: DataCenters main-tab:
> > > sub-tabs resizable columns
> > > 
> > > Gilad Chaplik has posted comments on this change.
> > > 
> > > Change subject: webadmin: DataCenters main-tab: sub-tabs resizable
> > > columns
> > > ......................................................................
> > > 
> > > 
> > > Patch Set 4:
> > > 
> > > why we need to invoke getTable().enableColumnResizing() on each sub-tab?
> > > don't we want all grid subtabs to have column resizing?
> > > 
> > > --
> > > To view, visit http://gerrit.ovirt.org/14105
> > > To unsubscribe, visit http://gerrit.ovirt.org/settings
> > > 
> > > Gerrit-MessageType: comment
> > > Gerrit-Change-Id: I71465d36e5c18ecb8eb6dbca436feaeea1c902a9
> > > Gerrit-PatchSet: 4
> > > Gerrit-Project: ovirt-engine
> > > Gerrit-Branch: master
> > > Gerrit-Owner: Daniel Erez <de...@redhat.com>
> > > Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
> > > Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
> > > Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
> > > Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
> > > Gerrit-Reviewer: Tal Nisan <tni...@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
> 
> 
> 
_______________________________________________
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel

Reply via email to