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 1f3vct-0000MH-MP for pgadmin-hackers@arkaria.postgresql.org; Thu, 05 Apr 2018 03:31:52 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1f3vcs-0001wv-CT for pgadmin-hackers@arkaria.postgresql.org; Thu, 05 Apr 2018 03:31:50 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1f3vcr-0001wj-P9 for pgadmin-hackers@lists.postgresql.org; Thu, 05 Apr 2018 03:31:50 +0000 Received: from mail-oi0-x241.google.com ([2607:f8b0:4003:c06::241]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1f3vcj-0000vz-Lc for pgadmin-hackers@postgresql.org; Thu, 05 Apr 2018 03:31:48 +0000 Received: by mail-oi0-x241.google.com with SMTP id u84-v6so21259666oie.10 for ; Wed, 04 Apr 2018 20:31:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=pAPAu5iToO4lJRylXkz1UJXrl3UBhtd/ahoOvIpT5fo=; b=IKQ9IjIrn55kHPFl029GMLiovfIjrybHi2XFNSrJY8+xQu5tWDxOYxAEpqksxHxBLU dUZAOoliYz2qAjvzGSHVdQs8RadVr0nnk0r62x4gXIlmZI9MN+DIkjwM63Ujd/dovxjk +Z9vgEVgIKTr0UrVEZbHwHcoRdP6eu9if/YxSv/Q2QWP9QMok4/1IZN1FMhH60goSEBQ IvOondWzZc6LAin6zzJSYZ6mcIQRoJUEGW4pHN/+alEuuAsk6Kd4ijskflBPkJIo5PAI SlnF8RZTBI2z/jEHfXTClKdYbvsSzOIW1o3OsKpbOlAwBvmKzhbE5UqCD2rVA1dgX9pP j2Jg== 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=pAPAu5iToO4lJRylXkz1UJXrl3UBhtd/ahoOvIpT5fo=; b=GzSXpL24pH0Wgxf1tGjAfnmauU07lbnE/so1IldoEEkzNAfBd0zlCX5w6LiZVUx+CR gNHBzrihN+h5G83AWGtFaL3YABhXh87DsfJeje9gTXW7nE23KXazxEAeRuWABsh9s1KK Lss5Asd6tsWgfA/vIFTIVVnekdTNt67aSE0tWbSEsrUbHgCtAOHFtmxDhhN1vJLog9EA NCv19o44Th6Oy0F/LK8lc4mwLzyZ3mHggMVYnBgOJhA5EhhgqfY3c+F4GtJqjBphuR5l eZscxui8/TDObNfAYCIgei+A5RafrQZ/HDkcFFaG1JxgZtL/SckYpQ1dsxseIByMiquJ /xrg== X-Gm-Message-State: ALQs6tBAEoUbYs1AWwAwmVv9dKOKKnri5Yp8ZknN5mIrbeFkgy276z4U O5oVkWcxjdkF7nZgavbLAHWf1nda0PJjEOScYc5Wsw== X-Google-Smtp-Source: AIpwx4/4KjgEaS6eFRN55tyGlls3Mu2ovagW61d5sQCn8UYF2zCcryi6/5PjTdp1drQCf8Hm8oT1IMfRaNYAZ0epSPg= X-Received: by 2002:aca:aa56:: with SMTP id t83-v6mr12317975oie.113.1522899100155; Wed, 04 Apr 2018 20:31:40 -0700 (PDT) MIME-Version: 1.0 Received: by 10.74.138.251 with HTTP; Wed, 4 Apr 2018 20:31:39 -0700 (PDT) In-Reply-To: References: From: Khushboo Vashi Date: Thu, 5 Apr 2018 09:01:39 +0530 Message-ID: Subject: Re: [pgAdmin4][RM#3155] Allow user to lock the Layout To: Dave Page Cc: Murtuza Zabuawala , Joao De Almeida Pereira , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000e4db8005691193af" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000e4db8005691193af Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Apr 4, 2018 at 8:09 PM, Dave Page wrote: > > > On Wed, Apr 4, 2018 at 12:54 PM, 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 = one 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 = developer >>>>>>>>>> can prevent drag-drop functionality on it. >>>>>>>>>> >>>>>>>>> >>>>>>> It's not working fine for me. For example, if I put the SQL Panel o= n >>>>>>> it's own below the properties/stats panels (so it looks like pgAdmi= n 3 used >>>>>>> to by default), and then lock the layout, I can un-dock the SQL pan= el into >>>>>>> a dialogue, but then cannot re-dock it. I can do weird things with = the >>>>>>> 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 t= he panel out of >>>>>> the group of Panels then it becomes individual Frame, That is what t= he >>>>>> 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 = used >>>>>> 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 >>>> not 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 renam= e >>> 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 tha= t 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. > > +1, but we need to find out the way as this feature is requested by many users. > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > --000000000000e4db8005691193af Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


O= n Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuz= a.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5= :00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at= 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterpris= edb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <= dpage@pgadmin.org> 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 P= age <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Mur= tuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
<= div dir=3D"ltr">
Hi,

Thanks Joao for 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:
=

<= div><= div style=3D"font-family:verdana,sans-serif;color:rgb(68,68,68);display:inl= ine">=E2=80=8BHello,

Please find updated patch,=C2=A0
=

<= /div>
Now layout will be locked after user upd= ates its preferences, w
e have use= d =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 r= endered,=C2=A0
=E2=80=8BI
=E2=80=8B also=C2=A0
made changes i= n JS tests as per Joao's review comments.
Looks like eve= rything is working when we change the lock.
As a personal prefere= nces 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 linte= r is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c=
2

./pgadmin/misc/__init__.py:78: [E30=
3] too many blank lines (2)

1       E303 too many blank line=
s (2)=C2=A0
1= =C2=A0
=E2=80=8BFixe= d=E2=80=8B
=C2=A0


@Dave/Pivotal team,
The given patch is = working fine for all the Tabs/Panels (all the panels from main window as we= ll as from Query tool and Debugger) but I'm facing an issue while handl= ing the Browser tree section, It is a wcDocer frame and not a wcDocker panel.=C2=A0Like wcDocker panel, wcDocker frame do not prov= ide any API so that a developer can prevent drag-drop functionality on it.<= /div>

It's not working fine for me. For example, if I put th= e 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 panel into a dialogue, but then cannot re-dock it. I can do weird = things with the browser tree as well, probably because it's a frame as = you say.
=C2=A0
<= div style=3D"font-family:verdana,sans-serif;color:rgb(68,68,68)">=E2=80=8BT= hat is expected behaviour =E2=80=8Bbecause once you drag the panel out of t= he group of Panels then it becomes individual Frame, That is what the autho= r of the wcDocker replied on my question,=C2=A0
"A panel must either be initialized as movable or non-mo= vable from the beginning and never changed because it generates a different= arrangement of elements depending. This feature should only ever be used w= ithin the onCreate method of the panel. I should probably have been more cl= ear about this limitation in the documentation."


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

Ther= e must be some way we can lock a tab that's not part of a group.
<= /div>
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.=C2= =A0
=E2=80=8BI w= ould 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 ch= eck what changes are required in main source code to make the Frame lock.

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

+1, but we need to find out the way as this= feature is requested by many users.=C2=A0
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake=

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Compan= y

--000000000000e4db8005691193af--