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 1fAsg8-00032F-Lu for pgadmin-hackers@arkaria.postgresql.org; Tue, 24 Apr 2018 07:47:56 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fAsg7-0007u2-Mu for pgadmin-hackers@arkaria.postgresql.org; Tue, 24 Apr 2018 07:47:55 +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 1fAsg7-0007ts-A5 for pgadmin-hackers@lists.postgresql.org; Tue, 24 Apr 2018 07:47:55 +0000 Received: from mail-wr0-x244.google.com ([2a00:1450:400c:c0c::244]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1fAsg3-0006zW-0z for pgadmin-hackers@postgresql.org; Tue, 24 Apr 2018 07:47:54 +0000 Received: by mail-wr0-x244.google.com with SMTP id p18-v6so28681173wrm.1 for ; Tue, 24 Apr 2018 00:47:50 -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=LBLeLDQraKUSjuldRuCxjZaaEct5KSin6aUDzSwWYvA=; b=F3+6Id9ksJb4erRcfSFpWQFTa/8XHGbh/CroD4SC5nqWMlo7Wys6gdB9iSU7WHjb2r W4bMpeGEEi43UInu9d7irWPE6IufN2DpxxtGVHD2NNKckI+DQGMZEbvWb1qAIo1765Sj 0yfUweAfFUwewlVO9x0D0ZP+f31bPXkz1PUxVB+yZDcSj/45C+FfSvpZ+FfTcvlcxxgS PQdSnHZap4llsfILOc4J7woULPVbA49q/ZgUw+mZ9tSGacBuRufLH9LmbwWpkdv0/0MS eTnUKe2wpv6SCcPBBYBcGxlMTYyrbVhnHNftRD5tHRyKv4NsyEpR5LoKS/lAx+yuiILB Ei3Q== 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=LBLeLDQraKUSjuldRuCxjZaaEct5KSin6aUDzSwWYvA=; b=JLOQn6iYQ1hUHrzyV6V1KUqZ48P0K2KFf06jvGX0OMQ+NJlK+vTJiTzZ1abxTY+fWG yMUHYVGdCsXcabzBgZ2uDvFTc96x7p5v3m7tu3oQx9/F2Xr/vZW2q7+ksm4qKhvoUwyl aS0Qxzhg6pJ0Mzj7vGs5hSzRe+Ogw7ZOtY4kxUEw89N7KDpTYAMHO+mjmxEieQhWCoId 1sW4RUqT9JkJL5RVAebUR6U7hbdTcQH9jdyem2IFGDOPXHzcHLIiyjq1bMCRDjm72XcS j4lbczmBx2+GA3ySqRxAJ3ixZ3UmnBFoMmES2PPVBUC/MtUq3zhItPnnKKkEZJaryIla ZTrg== X-Gm-Message-State: ALQs6tA1EKt+ilXhpabyKiw4xM+7tsF7LeEKClmzAn3h0xAXxkQqxsnV bQssuoc2Yg9EAjsy7ya/dinOdCuEuCCFgmsJsIV+Rw== X-Google-Smtp-Source: AB8JxZpQUTYSSy1eGe/5RGRTeDWWTwO/bIFUetrEuuB9UsdzGTKKMlvha1cmTutkCJTS7/OhEjuRf3ZojiCiyhKUAIY= X-Received: by 10.28.68.137 with SMTP id r131mr1106511wma.140.1524556069804; Tue, 24 Apr 2018 00:47:49 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.69.220 with HTTP; Tue, 24 Apr 2018 00:47:48 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Tue, 24 Apr 2018 08:47:48 +0100 Message-ID: Subject: Re: [pgAdmin4][RM#3155] Allow user to lock the Layout To: Murtuza Zabuawala Cc: pgadmin-hackers , Akshay Joshi Content-Type: multipart/alternative; boundary="001a1148d7befb314f056a935e56" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --001a1148d7befb314f056a935e56 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Akshay, could you review/commit this please? Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes. Thanks. On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala < murtuza.zabuawala@enterprisedb.com> wrote: > Hi Dave, > > Please find the updated patch, Now we are able to lock wcFrame and wcPane= l > both. > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt > wrote: > >> >> >> On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi < >> khushboo.vashi@enterprisedb.com> wrote: >> >>> >>> >>> On Wed, Apr 4, 2018 at 8:09 PM, Dave Page wrote: >>> >>>> >>>> >>>> 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 prefere= nce 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 a= nd 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 De= bugger) 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 Pane= l >>>>>>>>>> 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 thin= gs 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 dra= g the panel out >>>>>>>>> of the group of Panels then it becomes individual Frame, That is = what the >>>>>>>>> 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 diffe= rent >>>>>>>>> arrangement of elements depending. This feature should only ever = be used >>>>>>>>> within the onCreate method of the panel. I should probably have b= een 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 i= s >>>>>>> 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 grou= p. >>>>>>>> >>>>>>> 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 altog= ether? >>>>>> 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 w= ill >>>>> 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. >>> >> >> 100% agree. I can convince my self that this feature request has to do >> with locking panels into a certain layout. I can also convince myself th= at >> the same request is simple because users are frustrated with the fact th= at >> the tabs and panes move around and they find that behavior annoying. >> >> -- Rob >> >> >>> -- >>>> Dave Page >>>> Blog: http://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>> >>>> EnterpriseDB UK: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>> >>> >> > --=20 Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --001a1148d7befb314f056a935e56 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit use= r-visible changes, to make it easier to build the release notes.
=
Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock= wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
Enterp= riseDB:=C2=A0http://www.enterprisedb.com
The Enterprise Post= greSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckha= rdt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

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


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:<= br>
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadm= in.org> wrote:
<= div class=3D"gmail_quote">


On Wed, Apr= 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawal= a@enterprisedb.com> wrote:
= On Wed, Ap= r 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, Murtuza Zabuawala <murtuza= .zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.
<= div style=3D"font-family:verdana,sans-serif;color:rgb(68,68,68)">
=
PFA updat= ed 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 dir=3D"ltr">
=E2=80=8BHello,
<= /div>

Pl= ease find updated patch,=C2=A0

No= w layout will be locked after user updates its preferences, w
<= /font>e have used =E2=80=8B
templated variable i= n the javascript file
=
=E2=80=8B b= ecause we do not have preference module or preference cache available when = the page loads and panels gets rendered,=C2=A0
=E2=80=8BI
=E2=80= =8B also=C2=A0
made changes in JS tests as per Joao's review comme= nts.
Looks like everything is working when we change the loc= k.
As a personal preferences I would prefer to see this in at lea= st 2 commits, one that is related to the preference issue and another one t= hat is related to this story.


All t= he 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)=C2=A0
1=C2=A0
=E2=80=8BFixed=E2=80=8B
=C2=A0

=

@Dave/Pivotal team,
The given patch is working f= ine for all the Tabs/Panels (all the panels from main window as well as fro= m Query tool and Debugger) but I'm facing an issue while handling the B= rowser tree section, It is a wcDocer frame and not a wcDock= er panel.=C2=A0Like wcDocker panel, wcDocker frame do not provide any A= PI 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 Pan= el on it's own below the properties/stats panels (so it looks like pgAd= min 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 wi= th the browser tree as well, probably because it's a frame as you say.<= /div>
=C2=A0
=E2=80=8BThat is ex= pected behaviour =E2=80=8Bbecause once you drag the panel out of the group = of Panels then it becomes individual Frame, That is what the author of the = wcDocker replied on my question,=C2=A0
"A panel must either be initialized as movable or non-movable from= the beginning and never changed because it generates a different arrangeme= nt 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=C2=A0
As far as I understand Panel needs a Frame to rende= r itself if it is not attached to the main docker instance.=E2=80=8B
<= /div>

There must be som= e 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 :(
<= div style=3D"font-family:verdana,sans-serif;color:rgb(68,68,68);display:inl= ine">=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 altogethe= r? It clearly doesn't do everything we want right now.=C2=A0
=E2=80=8BI would say lets = include the feature by adding warning note that this feature works with def= ault layout only, And I don't think most user will try to drag drop Bro= wser panel =E2=80=8B
anyway, meanwhile I'll check what chang= es 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 fea= tures.=C2=A0

+1, but we need to find out the way as this feature is r= equested by many users.=C2=A0

=
100% agree. I can convince my self that this feature request has= to do with locking panels into a certain layout. I can also convince mysel= f that the same request is simple because users are frustrated with the fac= t that the tabs and panes move around and they find that behavior annoying.= =C2=A0

-- Rob
=C2=A0
= --
Dave Page
Blog: http://pgsnake.blogs= pot.com
Twitter: @pgsnake

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






--
Dave Page
Blog: = http://pgsnake.bl= ogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com<= br>The Enterprise PostgreSQL Company
--001a1148d7befb314f056a935e56--