Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1f3jZH-0005nA-Sa for pgadmin-hackers@arkaria.postgresql.org; Wed, 04 Apr 2018 14:39:20 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1f3jZG-0007ZO-EM for pgadmin-hackers@arkaria.postgresql.org; Wed, 04 Apr 2018 14:39:18 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1f3jZG-0007ZE-37 for pgadmin-hackers@lists.postgresql.org; Wed, 04 Apr 2018 14:39:18 +0000 Received: from mail-wr0-x241.google.com ([2a00:1450:400c:c0c::241]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1f3jZB-00085V-Qd for pgadmin-hackers@postgresql.org; Wed, 04 Apr 2018 14:39:17 +0000 Received: by mail-wr0-x241.google.com with SMTP id d17so7698466wre.1 for ; Wed, 04 Apr 2018 07:39:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=RpU3gRMnNaWM/wilgisMneX8HZ/QnTf8oEaoHZrxdEg=; b=z/1IkMs6dyN1VULS6uUvnSPY2giNgz83LhKOUkyKVl/seH+m6BhdmUm5H1bF0vkGOj GZFVvRnRi1putlzLiXsTbEdyxjybgWbX03cJpYZ3NGfatIzCdwW9phplfW6LtJvBBaex kRvBmEgvG9xbBN8rtK7RxBY6ybiummKcuDCjl9/Jl9swF8IMJDo3L4qcRWOeSr3ocnvT WP1gGBp8rd8QlMT88OonjpI5ulTohoDVTtalUJ3RA/v8BRCxYkMhSJqog7pvbtv3/F+S D4nVSJmfYUcnHK+Vjma0FJWBRi7Sc1nvH94e5BCq5em/HF+N/6jFqzoWL5S9Af9xnR2/ 1WMg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=RpU3gRMnNaWM/wilgisMneX8HZ/QnTf8oEaoHZrxdEg=; b=LEm6BE1ktColbe+MwZ/JwTN+1VarTIxVKaOmx4D7asffqCjSDfLIg8aNM9skYpYr8w 1GPNU0nGirHdlt6+pp8x9R6iOXTuCNgSr7UHsGxL2zlTeoTyltClrOIGRoAseSw7Slc8 SVRZa4xvWsAzTE7/4M7QWQZzYnrN8nQVoZJAU2ZCmOcVq/9DWvNZaj1kP8ZRvaW1ojEP V5kqaopWDwSru6jbagoHHLyUpZ2r5uxw90Kj1VWu4OtWMeMuVPFc8zwXo6VS9Kw2JZ4e DKZ8ZydVPUMT+nuPXPSXPhxPZZ4U0v1E6XkMVeB9o2xn0FDbVZFqP3hctb0J/NtdJlK3 xMzg== X-Gm-Message-State: AElRT7F9kOiuIoLxoLUJd1KgJhYznU0bMJebCopNEdFZF6o2K5qZtuj+ 6O14G8B9ag+d1BwN7E7uOC7yecrd0+RV6zg+5f5xAQ== X-Google-Smtp-Source: AIpwx4+kI88h2klLjGpmwq6XTuQfRUKTEsdt91wozgP0qzkQIxWgGbHNtfJ07vgiajoykkQWJ6t3aN756nREzO+61a8= X-Received: by 10.223.176.98 with SMTP id g31mr12860323wra.256.1522852752415; Wed, 04 Apr 2018 07:39:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.69.220 with HTTP; Wed, 4 Apr 2018 07:39:11 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Wed, 4 Apr 2018 15:39:11 +0100 Message-ID: Subject: Re: [pgAdmin4][RM#3155] Allow user to lock the Layout To: Murtuza Zabuawala Cc: Joao De Almeida Pereira , pgadmin-hackers Content-Type: multipart/alternative; boundary="001a113c9c405a723f056906c9f1" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --001a113c9c405a723f056906c9f1 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala < murtuza.zabuawala@enterprisedb.com> wrote: > On Wed, Apr 4, 2018 at 5:00 PM, Dave Page wrote: > >> >> >> On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala < >> murtuza.zabuawala@enterprisedb.com> wrote: >> >>> On Wed, Apr 4, 2018 at 2:47 PM, Dave Page wrote: >>> >>>> >>>> >>>> On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala < >>>> murtuza.zabuawala@enterprisedb.com> wrote: >>>> >>>>> Hi Dave, >>>>> >>>>> On Tue, Apr 3, 2018 at 9:03 PM, Dave Page wrote: >>>>> >>>>>> Hi >>>>>> >>>>>> On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala < >>>>>> murtuza.zabuawala@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> Thanks Joao for reviewing. >>>>>>> >>>>>>> PFA updated patch. >>>>>>> >>>>>>> On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira < >>>>>>> jdealmeidapereira@pivotal.io> wrote: >>>>>>> >>>>>>>> Hello, >>>>>>>> >>>>>>>> On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala < >>>>>>>> murtuza.zabuawala@enterprisedb.com> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> =E2=80=8BHello, >>>>>>>>> >>>>>>>>> Please find updated patch, >>>>>>>>> >>>>>>>>> Now layout will be locked after user updates its preferences, w >>>>>>>>> e have used =E2=80=8B >>>>>>>>> templated variable in the javascript file >>>>>>>>> =E2=80=8B because we do not have preference module or preference = cache >>>>>>>>> available when the page loads and panels gets rendered, >>>>>>>>> =E2=80=8BI >>>>>>>>> =E2=80=8B also >>>>>>>>> made changes in JS tests as per Joao's review comments. >>>>>>>>> >>>>>>>> Looks like everything is working when we change the lock. >>>>>>>> As a personal preferences I would prefer to see this in at least 2 >>>>>>>> commits, one that is related to the preference issue and another o= ne that >>>>>>>> is related to this story. >>>>>>>> >>>>>>>> >>>>>>>> All the tests are working, but he linter is failing: >>>>>>>> >>>>>>>> /tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2 >>>>>>>> >>>>>>>> ./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2) >>>>>>>> >>>>>>>> 1 E303 too many blank lines (2) >>>>>>>> >>>>>>>> >>>>>>>> 1 >>>>>>>> >>>>>>> =E2=80=8BFixed=E2=80=8B >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> @Dave/Pivotal team, >>>>>>>>> The given patch is working fine for all the Tabs/Panels (all the >>>>>>>>> panels from main window as well as from Query tool and Debugger) = but I'm >>>>>>>>> facing an issue while handling the Browser tree section, It is a = wcDocer >>>>>>>>> frame and >>>>>>>>> not a wcDocker panel >>>>>>>>> . Like >>>>>>>>> wcDocker panel, wcDocker frame do not provide any API so that a d= eveloper >>>>>>>>> can prevent drag-drop functionality on it. >>>>>>>>> >>>>>>>> >>>>>> It's not working fine for me. For example, if I put the SQL Panel on >>>>>> it's own below the properties/stats panels (so it looks like pgAdmin= 3 used >>>>>> to by default), and then lock the layout, I can un-dock the SQL pane= l into >>>>>> a dialogue, but then cannot re-dock it. I can do weird things with t= he >>>>>> browser tree as well, probably because it's a frame as you say. >>>>>> >>>>> >>>>> =E2=80=8BThat is expected behaviour =E2=80=8Bbecause once you drag th= e panel out of >>>>> the group of Panels then it becomes individual Frame, That is what th= e >>>>> author of the wcDocker replied on my question, >>>>> *"A panel must either be initialized as movable or non-movable from >>>>> the beginning and never changed because it generates a different >>>>> arrangement of elements depending. This feature should only ever be u= sed >>>>> within the onCreate method of the panel. I should probably have been = more >>>>> clear about this limitation in the documentation."* >>>>> >>>>> >>>> So does it become a panel again if a second panel is added to the new >>>> tab group? >>>> >>> =E2=80=8BNo, it stays Frame.=E2=80=8B >>> >>> As far as I understand Panel needs a Frame to render itself if it is no= t >>> attached to the main docker instance.=E2=80=8B >>> >>>> >>>> There must be some way we can lock a tab that's not part of a group. >>>> >>> At a moment there is no way of =E2=80=8B >>> locking frames out of the box :( >>> =E2=80=8B >>> >> >> Hmm, so the question becomes: do we include the lock feature, but rename >> it to "Lock Tabs" or something similar, or leave it out altogether? It >> clearly doesn't do everything we want right now. >> > =E2=80=8BI would say lets include the feature by adding warning note that= this > feature works with default layout only, And I don't think most user will > try to drag drop Browser panel =E2=80=8B > anyway, meanwhile I'll check what changes are required in main source cod= e > to make the Frame lock. > Anyone else have any thoughts on this? Personally I don't like including half-baked features. --=20 Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --001a113c9c405a723f056906c9f1 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala &l= t;m= urtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgad= min.org> wrote:
=


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pga= dmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtu= za.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave P= age <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:<= br>
Hi,

Thanks Joao f= or reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao D= e Almeida Pereira <jdealmeidapereira@pivotal.io> = wrote:
Hello,

On Mon, Apr 2, = 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com= > wrote:
=

=
=E2=80=8BHello,<= /div>

Please find updated patch,=C2=A0
<= div><= div style=3D"display:inline">
Now layout will be locked after user updates its preferences= , w
e have used =E2=80=8B
templated variable in the javascript file
=
=E2=80=8B because we do not hav= e preference module or preference cache available when the page loads and p= anels gets rendered,=C2=A0
=E2=80=8BI
=E2=80=8B also=C2=A0
ma= de changes in JS tests as per Joao's review comments.
Lo= oks like everything is working when we change the lock.
As a pers= onal preferences I would prefer to see this in at least 2 commits, one that= is related to the preference issue and another one that is related to this= story.


All the tests are working, = but he linter is failing:
/tmp/build/4a5630c=
2/pivotal-rm-3155/web /tmp/build/4a5630c2

./pgadmin/misc/__init__.py:78: [E303] too many blank lin=
es (2)

1       E303 too many blank lines (2)=C2=A0
1=C2=A0
=E2=80=8BFixed=E2=80=8B
=C2= =A0

=
<= br>
= @Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (al= l the panels from main window as well as from Query tool and Debugger) but = I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel.=C2=A0Like wcDocke= r panel, wcDocker frame do not provide any API so that a developer can prev= ent drag-drop functionality on it.

It's not working fi= ne for me. For example, if I put the SQL Panel on it's own below the pr= operties/stats panels (so it looks like pgAdmin 3 used to by default), and = then lock the layout, I can un-dock the SQL panel into a dialogue, but then= cannot re-dock it. I can do weird things with the browser tree as well, pr= obably because it's a frame as you say.
=C2=A0
=E2=80=8BThat is expected behaviour =E2=80=8Bbecau= se once you drag the panel out of the group of Panels then it becomes indiv= idual Frame, That is what the author of the wcDocker replied on my question= ,=C2=A0
"A panel must either b= e initialized as movable or non-movable from the beginning and never change= d because it generates a different arrangement of elements depending. This = feature should only ever be used within the onCreate method of the panel. I= should probably have been more clear about this limitation in the document= ation."

So does it become a panel again if a second panel is added to t= he new tab group?
=E2=80=8BNo, it stays Frame.=E2=80=8B
=C2=A0
As far = as I understand Panel needs a Frame to render itself if it is not attached = to the main docker instance.=E2=80=8B

There must be some way we can lock a tab that= 9;s not part of a group.
At a moment there is no way of =E2=80=8B
locking frames out of the box :(
=E2=80=8B

Hmm, so the question becomes:= do we include the lock feature, but rename it to "Lock Tabs" or = something similar, or leave it out altogether? It clearly doesn't do ev= erything we want right now.=C2=A0
=E2=80=8BI would say lets include the feature by adding w= arning note that this feature works with default layout only, And I don'= ;t think most user will try to drag drop Browser panel =E2=80=8B
anyway, meanwhile I'll check what changes are required in main source = code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don'= ;t like including half-baked features.=C2=A0

-- <= br>
Dave P= age
Blog: http= ://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterpri= sedb.com
The Enterprise PostgreSQL Company
--001a113c9c405a723f056906c9f1--