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 1fBINs-0000cA-VZ for pgadmin-hackers@arkaria.postgresql.org; Wed, 25 Apr 2018 11:14:49 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fBINr-0004mt-Vg for pgadmin-hackers@arkaria.postgresql.org; Wed, 25 Apr 2018 11:14:48 +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 1fBINr-0004mi-KM for pgadmin-hackers@lists.postgresql.org; Wed, 25 Apr 2018 11:14:47 +0000 Received: from mail-wm0-x236.google.com ([2a00:1450:400c:c09::236]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1fBINm-0002CP-Rl for pgadmin-hackers@postgresql.org; Wed, 25 Apr 2018 11:14:47 +0000 Received: by mail-wm0-x236.google.com with SMTP id o78so6116757wmg.0 for ; Wed, 25 Apr 2018 04:14:42 -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=0kl0WTB6u5OEHVpWBiKeTqT/DD4z0dB7H3zFRIScC4w=; b=vE/nEgcbP4U7XbzXFSF+CAaIyx8Us+c7k0a85TBkenbkiX3F2MLGNmwUVhEmzBb7bQ NSyXBndyiTXU4wwLS0VNz9MpxbpNtBEgHZg5WtAsE5r58mGw1/VKqxtvymsCI7O0z5Ce 06Bo0KgqET3s0+tyYF4qq9YBG/0iEsJYNw2ATGV4cM32cXjGvlIN8XyS1r/jHtGNcCMp vQD242WCxpJz00ax1rOVYCtoBCbIzwaVEoT4iQl3V+TJsFpKXZ+otTqpPh5QQnQ5h3Ir lOvh7Xq1vr6EZPma7GW1XLZhDZxH4yZgQcVQpWpPFvYgTrUH/MR6XznoUhWyKN4rdraz zO6g== 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=0kl0WTB6u5OEHVpWBiKeTqT/DD4z0dB7H3zFRIScC4w=; b=NWDCqpLEthGtJnVj4cctY+/5zlFdtsPZnLQ4JSQwnEgj73Kn5lmUWX9NtN3EyxkKrs t3oS4aXYbTTEyOAbE3qwB2NV+qALBG5khkHpozOU3pG91iYrfj8jo4C1HAYoY8/Ql+lE 9x+bTW6bY5mqn5gonvvOJyQS7qMd0LHMufpmB86q6sfcAGp7RZ4TCLDeRZV+9lvPEKiF zQjF/GSHqXln6E6Tn5eXF8oMAL3h16wajDaB5JB8IIvLSu6Xa7XpaFmcwvEeYTorc2aY BTYwRCun3/GfsBCUb+UaVlBh9c40BYMdxyLSSmmoRlqz9XconMBkUQKDvlwFDm6nhypj c1cg== X-Gm-Message-State: ALQs6tCEcyHvhTC4fJn1VWi0nQRPJXzKTi+p/JoOpyAQ9MTqRoarRmFo Q//5YoFp6jPjc94OvIlz9kCIui9/GFn2z9SORSynJQ== X-Google-Smtp-Source: AB8JxZo0vJgeOBk2c8EwnbyG3PM3qPdTRq7EM9anqJ9gwfYDl1gnQjpf4bypg8JR0X4h9HdcpyAX8QMLAMGK5M6FnUM= X-Received: by 10.28.136.199 with SMTP id k190mr15025243wmd.154.1524654880891; Wed, 25 Apr 2018 04:14:40 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.69.220 with HTTP; Wed, 25 Apr 2018 04:14:39 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Wed, 25 Apr 2018 12:14:39 +0100 Message-ID: Subject: Re: [pgAdmin4][RM#3155] Allow user to lock the Layout To: Murtuza Zabuawala Cc: Akshay Joshi , Joao De Almeida Pereira , pgadmin-hackers Content-Type: multipart/alternative; boundary="001a11444b4294b84d056aaa6047" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --001a11444b4294b84d056aaa6047 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Apr 25, 2018 at 12:13 PM, Murtuza Zabuawala < murtuza.zabuawala@enterprisedb.com> wrote: > Hi Dave, > > On Wed, Apr 25, 2018 at 3:36 PM, Dave Page wrote: > >> All, >> >> We just had a brief discussion in our EDB sprint planning meeting about >> this. There is a non-zero chance that we're going to have to fork wcDock= er >> in the near future, in order to update it to work with jQuery 3. If we d= o >> that, then it may be significantly easier to fix this issue in that fork >> (perhaps by adding a single lockLayout(bool) function, rather than tryin= g >> to do so from pgAdmin. >> >> I think (unless Murtuza believes that won't help), that we're better off >> holding on this for now until we know if we've had to do that. >> > > =E2=80=8BI don't have any objection forking the code and adding the flag = to lock > the panel, But I'm certain that > we will use the same inbuilt method *panel.moveable(false)* which we have > used right now in the patch to prevent a panel from floating and will fac= e > the same issue again which Akshay mentioned in his last email. > > Let me know if you want me to attach latest patch onto the ticket for > future reference and update the ticket accordingly=E2=80=8B. > That's probably a good idea - thanks. > > >> On Wed, Apr 25, 2018 at 10:23 AM, Murtuza Zabuawala < >> murtuza.zabuawala@enterprisedb.com> wrote: >> >>> Hi Akshay, >>> >>> >>> On Wed, Apr 25, 2018 at 2:37 PM, Akshay Joshi < >>> akshay.joshi@enterprisedb.com> wrote: >>> >>>> Hi Joao/Murtuza >>>> >>>> It break's the functionality, I am able to move "Data output", >>>> "Explain" etc.. panel of Query Tool, even if "Lock layout?" is set to = True. >>>> >>> >>> =E2=80=8BIt's working properly in v5 patch, Something went wrong while >>> refactoring.=E2=80=8B >>> >>> =E2=80=8B >>> >>> Apart from above I have found more issue. Below are the steps to >>>> reproduce: >>>> >>>> - Set the "Lock layout?" flag to False. >>>> - Move out Dashboard panel. >>>> - Set the "Lock layout?" flag to True. >>>> - Close the Dashboard panel, as layout is locked and empty >>>> Dashboard panel is still visible. (Refer attached screenshot) >>>> >>>> =E2=80=8BThat's because we have set the Panel moveable property to Fal= se, they >>> won't auto resize, As discussed earlier if user drag any panel out of p= anel >>> group it gets render in seprate wcFrame. I think that needs to be taken >>> care by user before they decide to lock the layout, We can not expilcit= ly >>> set panel's closeable property to False when layout is locked, If we do= so >>> user will not be able to close any Query tool, Debugger panels.=E2=80= =8B >>> =E2=80=8B >>> >>> >>> On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira < >>>> jdealmeidapereira@pivotal.io> wrote: >>>> >>>>> haha, >>>>> Just joking, now we have a good version that passes tests and all. >>>>> >>>>> We found out that a test was failing in the patch version 5: >>>>> >>>>> HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and u= ser created panel without defining isMoveable then it should be moveable it= should call moveable method with true as argument FAILED >>>>> Expected false to be true. >>>>> at UserContext. (regression/javascript/browser/pan= el_spec.js:12886:38) >>>>> >>>>> =E2=80=8B >>>>> To solve this problem we decided to change the Panel class to match >>>>> what the test say. >>>>> >>>>> Thanks >>>>> Victoria & Joao >>>>> >>>>> >>>>> On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira < >>>>> jdealmeidapereira@pivotal.io> wrote: >>>>> >>>>>> Hi, >>>>>> Apparently the last version was not applying, here is the new versio= n >>>>>> that should apply correctly >>>>>> >>>>>> Thanks >>>>>> Victoria & Joao >>>>>> >>>>>> On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira < >>>>>> jdealmeidapereira@pivotal.io> wrote: >>>>>> >>>>>>> Hi Murtuza, >>>>>>> >>>>>>> We tested the patch and everything looks fine. We also refactors >>>>>>> some parts to include things like strict equality and using let/con= st >>>>>>> instead of var. The updated patch is attached. >>>>>>> In the future, it will be more valuable to have the translation to >>>>>>> ES6 and the feature work in separate commits so it is easier to und= erstand >>>>>>> what changed. >>>>>>> >>>>>>> Sincerely, >>>>>>> >>>>>>> Joao and Victoria >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi < >>>>>>> akshay.joshi@enterprisedb.com> wrote: >>>>>>> >>>>>>>> On Tue, Apr 24, 2018 at 1:17 PM, Dave Page >>>>>>>> wrote: >>>>>>>> >>>>>>>>> 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 note= s. >>>>>>>>> >>>>>>>> >>>>>>>> Sure >>>>>>>> >>>>>>>>> >>>>>>>>> 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 >>>>>>>>>> EnterpriseDB: http://www.enterprisedb.com >>>>>>>>>> The Enterprise PostgreSQL Company >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt < >>>>>>>>>> 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 >>>>>>>>>>>> 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 < >>>>>>>>>>>>>>>> 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 Page < >>>>>>>>>>>>>>>>>> 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. >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> PFA updated patch. >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereir= a >>>>>>>>>>>>>>>>>>>> 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 o= r >>>>>>>>>>>>>>>>>>>>>> preference cache available when the page loads and p= anels 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 pre= ference 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/4a= 5630c2 >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> ./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 AP= I 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 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. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> =E2=80=8BThat is expected behaviour =E2=80=8Bbecause onc= e you drag the >>>>>>>>>>>>>>>>>> panel out of the group of Panels then it becomes individ= ual 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 >>>>>>>>>>>>>>>>>> different arrangement of elements depending. This featur= e should only ever >>>>>>>>>>>>>>>>>> be used within the onCreate method of the panel. I shoul= d probably have >>>>>>>>>>>>>>>>>> been more clear about this limitation in the documentati= on."* >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> So does it become a panel again if a second panel is adde= d >>>>>>>>>>>>>>>>> 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 itsel= f >>>>>>>>>>>>>>>> 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 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 warn= ing 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 lik= e >>>>>>>>>>>>> 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 ha= s >>>>>>>>>>> to do with locking panels into a certain layout. I can also con= vince myself >>>>>>>>>>> that the same request is simple because users are frustrated wi= th the fact >>>>>>>>>>> that 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 >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Dave Page >>>>>>>>> Blog: http://pgsnake.blogspot.com >>>>>>>>> Twitter: @pgsnake >>>>>>>>> >>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>>>>>> The Enterprise PostgreSQL Company >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> *Akshay Joshi* >>>>>>>> >>>>>>>> *Sr. Software Architect * >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 >>>>>>>> 976-788-8246 <+91%2097678%2088246>* >>>>>>>> >>>>>>> >>>> >>>> >>>> -- >>>> *Akshay Joshi* >>>> >>>> *Sr. Software Architect * >>>> >>>> >>>> >>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246* >>>> >>> >>> >> >> >> -- >> 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 --001a11444b4294b84d056aaa6047 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Wed, Apr 25, 2018 at 12:13 PM, Murtuza Zabuawala &= lt;= murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Wed, Apr 25, 2018 at 3:3= 6 PM, Dave Page <dpage@pgadmin.org> wrote:
All,

We just had a bri= ef discussion in our EDB sprint planning meeting about this. There is a non= -zero chance that we're going to have to fork wcDocker in the near futu= re, in order to update it to work with jQuery 3. If we do that, then it may= be significantly easier to fix this issue in that fork (perhaps by adding = a single lockLayout(bool) function, rather than trying to do so from pgAdmi= n.

I think (unless Murtuza believes that won't= help), that we're better off holding on this for now until we know if = we've had to do that.

=E2=80=8BI don't h= ave any objection forking the code and adding the flag to lock the panel,= =C2=A0 But I'm certain that=C2=A0
we will = use the same inbuilt method = panel.moveable(false) which we have used right now in the patch to prev= ent a panel from floating and will face the same issue again which Akshay m= entioned in his last email.

Let me know if you want me to attach latest patch onto the ti= cket for future reference and update the ticket accordingly=E2=80=8B= .

That's = probably a good idea - thanks.
=C2=A0

<= div dir=3D"ltr">

On Wed, Apr 25, 2018 at 10:23 AM, Murtuza Zabuawala &= lt;= murtuza.zabuawala@enterprisedb.com> wrote:
Hi Akshay,


On Wed, Apr 25, 2018 at 2:37 PM, Aksha= y Joshi <akshay.joshi@enterprisedb.com> wro= te:
Hi Joao/Murtuza
=
It break's the functionality, I am able to move "Da= ta output", "Explain" etc.. panel of Query Tool, even if &qu= ot;Lock layout?" is set to True.
=C2=A0
=E2=80=8BIt's working properly in v5 patch, Somet= hing went wrong while refactoring.=E2=80=8B
=C2=A0
=E2=80=8B
Apart fro= m above I have found more issue. Below are the steps to reproduce:
  • Set the "Lock layout?" flag to False.
  • Move out = Dashboard panel.
  • Set the "Lock layout= ?" flag to True.
  • Close the Dashboard panel, as layout i= s locked and empty Dashboard panel is still visible. (Refer attached screen= shot)=C2=A0=C2=A0
= =E2=80=8BThat's because we have set the Panel moveable property to Fals= e, they won't auto resize, As discussed earlier if user drag any panel = out of panel group it gets render in seprate wcFrame. I think that needs to= be taken care by user before they decide to lock the layout, We can not ex= pilcitly set panel's closeable property to False when layout is locked,= If we do so user will not be able to close any Query tool, Debugger panels= .=E2=80=8B
=C2=A0=E2=80=8B
=

On Tue, Apr 24, 2018 at 9:11 PM, Joao De= Almeida Pereira <jdealmeidapereira@pivotal.io> w= rote:
haha,
Just jok= ing, now we have a good version that passes tests and all.

We found out that a test was failing in the patch version 5:
= 
HeadlessChrome 0.0.0 (Linux 0.0.0) P=
anel when we create a panel and user created panel without defining isMovea=
ble then it should be moveable it should call moveable method with true as =
argument FAILED
    Expected false to be true.
        at UserContext.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
=E2=80=8B
To solve this problem we decided to change the Panel cl= ass to match what the test say.

Thanks
V= ictoria & Joao


On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira <= jdealmeid= apereira@pivotal.io> wrote:
=
Hi,
Apparently the last version was not applying, here= is the new version that should apply correctly

Th= anks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <<= a href=3D"mailto:jdealmeidapereira@pivotal.io" target=3D"_blank">jdealmeida= pereira@pivotal.io> wrote:
<= div dir=3D"ltr">Hi Murtuza,

We tested the patch and ever= ything looks fine.=C2=A0 We also refactors some parts to include things lik= e strict equality and using let/const instead of var.=C2=A0 The updated pat= ch is attached.
In the future, it will be more valuable to have t= he translation to ES6 and the feature work in separate commits so it is eas= ier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, = Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com>= wrote:
On Tue, Apr 24, 2018 at 1:17 PM= , Dave Page <dpage@pgadmin.org> wrote:
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 not= es.

=C2=A0 =C2=A0= Sure=C2=A0

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <<= a href=3D"mailto:murtuza.zabuawala@enterprisedb.com" target=3D"_blank">murt= uza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

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

<= div class=3D"m_-9025730651829308527m_556779113377951736m_-13553935446343116= 73m_-3273487579039060307m_-7590550199736533260m_-622418930833992900m_713730= 5888967977126m_1156207684282842045m_-9161428906115566089m_77038578753455323= 1m_-6739333857610869662m_-7492286660467830201m_-3274200323325296947gmail_si= gnature" data-smartmail=3D"gmail_signature">
=
--
Regards,
Murtuza Zabuawala
EnterpriseDB:=C2=A0h= ttp://www.enterprisedb.com
The Enterprise PostgreSQL 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.za= buawala@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@enterprisedb.com> wrote:<= br>
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org><= span style=3D"font-family:arial,sans-serif;color:rgb(34,34,34)"> 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@e= nterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.<= /div>

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,
<= br>
Please find upda= ted patch,=C2=A0
Now layout will b= e locked after user updates its preferences, w
e have used =E2=80=8B
<= /div>
templated variable in the javascript file<= /div>
=E2=80=8B because 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 te= sts 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.
<= br>

All the tests are working, but he linter is fa= iling:
=

/tmp/build/4a5630c2/pivota=
l-rm-3155/web /tmp/build/4a5630c2

./pgadmin/mis=
c/__init__.py:78: [E303] too many blank lines (2)

1       E3=
03 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 fine for all the Tabs/P= anels (all the panels from main window as well as from Query tool and Debug= ger) but I'm facing an issue while handling the Browser tree section, I= t is a wcDocer frame and not a wcDocker panel.=C2=A0Lik= e wcDocker panel, wcDocker frame do not provide any API so that a developer= can prevent drag-drop functionality on it.

It's not w= orking fine for me. For example, if I put the SQL Panel on it's own bel= ow the properties/stats panels (so it looks like pgAdmin 3 used to by defau= lt), 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
=E2=80=8BThat is expected behaviour =E2= =80=8Bbecause once you drag the panel out of the group of Panels then it be= comes individual Frame, That is what the author of the wcDocker replied on = my question,=C2=A0
"A panel mu= st either be initialized as movable or non-movable from the beginning and n= ever changed because it generates a different arrangement of elements depen= ding. This feature should only ever be used within the onCreate method of t= he panel. I should probably have been more clear about this limitation in t= he 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 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 questi= on becomes: do we include the lock feature, but rename it to "Lock Tab= s" or something similar, or leave it out altogether? It clearly doesn&= #39;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 default layout only, An= d 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 m= ain source code to make the Frame lock.

Anyone else have any thoughts on thi= s? 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 use= rs.=C2=A0

100% agre= e. I can convince my self that this feature request has to do with locking = panels into a certain layout. I can also convince myself that the same requ= est is simple because users are frustrated with the fact that the tabs and = panes move around and they find that behavior annoying.=C2=A0
-- Rob
=C2=A0
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Ent= erpriseDB UK: htt= p://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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



--
Akshay Joshi<= /div>
Sr. Softwar= e Architect




--
Akshay Joshi
Sr. Software Archit= ect

=




--
Dave Page
Blog: http://pgsnake.blogspot.comTwitter: @pgsnake

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



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

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