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 1fBhlL-0002mT-9r for pgadmin-hackers@arkaria.postgresql.org; Thu, 26 Apr 2018 14:20:43 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fBhlJ-0008Sm-HO for pgadmin-hackers@arkaria.postgresql.org; Thu, 26 Apr 2018 14:20:41 +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 1fBhlJ-0008SZ-0D for pgadmin-hackers@lists.postgresql.org; Thu, 26 Apr 2018 14:20:41 +0000 Received: from mail-ot0-x22b.google.com ([2607:f8b0:4003:c0f::22b]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1fBhlA-0005br-PN for pgadmin-hackers@postgresql.org; Thu, 26 Apr 2018 14:20:39 +0000 Received: by mail-ot0-x22b.google.com with SMTP id 77-v6so23492455otd.4 for ; Thu, 26 Apr 2018 07:20:32 -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=VswBtI7YfQ7vdz3VKpzRTXQgyRd450XvqUpPbzfIghE=; b=J8bkptUxCpbA101SxmVqZkWljAhysDDx22RTcql1X1OE1TMJWXna/jm7PzjCa4MhqZ ElwoXWT6Cy9uWoNFPAkL2Eri5/J7bzntWUuF4v5KZQ9mPjRziljpH6wnL4Qg3sXPqSmI t2r5YfD5molduS7zqkkxvld2lWRi9uYhCGWD4P9xYaNycj0f3ScUYXsBSS93X78GloZg /XQoTLcEz12Kv8mSa9di8n9ZDOZBq3shKGdjUKrDiSqXuVv16y7hooyxFlgOaWV2q2/Q 16xLKGmjIH/W4IYrSRcHQB/qHGoXQ0CzoQMQLetwLrDyvSFEG9VOgpQPPICt4kyy+GPT 66qQ== 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=VswBtI7YfQ7vdz3VKpzRTXQgyRd450XvqUpPbzfIghE=; b=GRHONpcOaewq33AH7GuaYu2teERU/gUyZI5ZYz2cn9aLEhWaEL9ZnrUglNFjYcKhBA T+nU8nsAvBj3BytG4OsSfqIuMrLr5bh8ZMFpg2rPj3kgr8KkGVccnJy5M2sBVBfB0QEA hmcIMNwRSN9zBFbnunJZpp22XjfqZrQIHfOcstr6HpJ/9iDC6FOALQh5znGlHlhw33Wn P3LQha39sYikosSzOWsktwGi+8it2EeiqnHMeaw8A+B3dQ3CyU/c/W0bKBaSkr+CGbgd +yWrQamD4JmP9wlt0kqOEoFVIXyLn+h+Zfj+Bbq1j62bPhcgD5u2stcLj1iq2j2PSjmU rORw== X-Gm-Message-State: ALQs6tCLF/Nvd51Bt45OAosglzuYxBxwtuxgottcVLV0ScxlLnrWUVcE 2WTFF+qd6+hDWuqO00tBcTcvsYf9gxxbLc2VtfRaQg== X-Google-Smtp-Source: AIpwx4+XOVEIVREdeSGpmCJa/oiEUtdPKtohxI+sk4C/Cs52Vq3VdyomQ6wiIEloHSHg4KVG3ZribfqpwASW/wSrFw4= X-Received: by 2002:a9d:700c:: with SMTP id k12-v6mr21233268otj.178.1524752430787; Thu, 26 Apr 2018 07:20:30 -0700 (PDT) MIME-Version: 1.0 Received: by 10.201.89.5 with HTTP; Thu, 26 Apr 2018 07:20:10 -0700 (PDT) In-Reply-To: References: From: Murtuza Zabuawala Date: Thu, 26 Apr 2018 19:50:10 +0530 Message-ID: Subject: Re: [pgAdmin4][RM#3155] Allow user to lock the Layout To: Joao De Almeida Pereira Cc: Dave Page , Akshay Joshi , pgadmin-hackers Content-Type: multipart/alternative; boundary="00000000000002198f056ac11758" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --00000000000002198f056ac11758 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Joao, On Wed, Apr 25, 2018 at 6:55 PM, Joao De Almeida Pereira < jdealmeidapereira@pivotal.io> wrote: > Hi, > @Murtuza: We didn't notice the issue, can you please advise on what need > to change to make it work? The only change we did was to make one test pa= ss. > I moved to another project so I didn't get a chance to look into the code but a s you are aware that =E2=80=8Bwe are no longer considering given the patch = as a fix for the issue instead someone from the team might fork the =E2=80=8B =E2=80=8Bcode and add the option in the library itself.=E2=80=8B Regards, Murtuza > @Hackers: In our point of view it is never good to fork a library. But if > he really have to do it, then we should fork it in Github, make our code > accessible to other people, and we should add it as a dependency on > package.json > > > Thanks > Anthony & Joao > > > On Wed, Apr 25, 2018 at 7:14 AM Dave Page wrote: > >> 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 abou= t >>>> this. There is a non-zero chance that we're going to have to fork wcDo= cker >>>> in the near future, 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 fo= rk >>>> (perhaps by adding a single lockLayout(bool) function, rather than try= ing >>>> 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 fla= g 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 w= ill >>> face 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 t= o True. >>>>>> >>>>> >>>>> =E2=80=8BIt's working properly in v5 patch, Something went wrong whil= e >>>>> 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 F= alse, >>>>> they won't auto resize, As discussed earlier if user drag any panel o= ut 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 >>>>> expilcitly set panel's closeable property to False when layout is loc= ked, >>>>> If we do so user will not be able to close any Query tool, Debugger p= anels.=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= user 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/p= anel_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 >>>>>>>> version 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/c= onst >>>>>>>>> instead of var. The updated patch is attached. >>>>>>>>> In the future, it will be more valuable to have the translation t= o >>>>>>>>> ES6 and the feature work in separate commits so it is easier to u= nderstand >>>>>>>>> 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 rel= ease notes. >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> 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 < >>>>>>>>>>>>>>>> dpage@pgadmin.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@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 >>>>>>>>>>>>>>>>>>>>>> Pereira 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 thi= s >>>>>>>>>>>>>>>>>>>>>>> in at least 2 commits, one that is related to the p= reference 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 blan= k 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 we= ll as from Query tool and >>>>>>>>>>>>>>>>>>>>>>>> Debugger) but I'm facing an issue while handling t= he 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 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-doc= k it. I can do weird >>>>>>>>>>>>>>>>>>>>> things with the browser tree as well, probably becaus= e it's a frame as you >>>>>>>>>>>>>>>>>>>>> say. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> =E2=80=8BThat is expected behaviour =E2=80=8Bbecause o= nce 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= , >>>>>>>>>>>>>>>>>>>> *"A panel must either be initialized as movable or >>>>>>>>>>>>>>>>>>>> non-movable from the beginning and never changed becau= se it generates a >>>>>>>>>>>>>>>>>>>> different arrangement of elements depending. This feat= ure should only ever >>>>>>>>>>>>>>>>>>>> be used within the onCreate method of the panel. I sho= uld probably have >>>>>>>>>>>>>>>>>>>> been more clear about this limitation in the documenta= tion."* >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> 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 par= t >>>>>>>>>>>>>>>>>>> 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 simila= r, or leave it out >>>>>>>>>>>>>>>>> altogether? It clearly doesn't do everything we want righ= t now. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> =E2=80=8BI would say lets include the feature by adding wa= rning >>>>>>>>>>>>>>>> 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. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> +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 al= so convince >>>>>>>>>>>>> myself that the same request is simple because users are frus= trated with >>>>>>>>>>>>> the fact that the tabs and panes move around and they find th= at 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-9517 <+91%2020%203058%209517>Mobile: +91 >>>>>> 976-788-8246 <+91%2097678%2088246>* >>>>>> >>>>> >>>>> >>>> >>>> >>>> -- >>>> 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 >> > --00000000000002198f056ac11758 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Joao,

On Wed, Apr 25, 2018 at 6:55 PM, Joao De Alm= eida Pereira <jdealmeidapereira@pivotal.io> wrote= :
Hi,
@Murtuza: We d= idn't notice the issue, can you please advise on what need to change to= make it work? The only change we did was to make one test pass.
=C2=A0
I moved t= o another project so I didn't get a chance to look into the code but a<= /div>
s you are aware that =E2=80=8Bwe are no = longer considering given the patch as a fix for the issue instead someone f= rom the team might fork the =E2=80=8B
=C2=A0
=E2=80=8Bcode and add the option in the library itself.=E2=80=8B
<= /div>

Regards,
Murt= uza


@Hackers: In our= point of view it is never good to fork a library. But if he really have to= do it, then we should fork it in Github, make our code accessible to other= people, and we should add it as a dependency on package.json

Thanks
Anthony & Joao

On Wed, Apr 2= 5, 2018 at 7:14 AM Dave Page <dpage@pgadmin.org> wrote:
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 <dpage@pgadmin.org> wrote:
All,

We just had a brief discussi= on in our EDB sprint planning meeting about this. There is a non-zero chanc= e that we're going to have to fork wcDocker in the near future, in orde= r to update it to work with jQuery 3. If we do that, then it may be signifi= cantly easier to fix this issue in that fork (perhaps by adding a single lo= ckLayout(bool) function, rather than trying to do so from pgAdmin.

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

=E2=80=8BI don't have any obj= ection forking the code and adding the flag to lock the panel,=C2=A0 But I&= #39;m certain that=C2=A0
we will use the same = inbuilt method panel.moveabl= e(false) which we have used right now in the patch to prevent a panel f= rom floating and will face the same issue again which Akshay mentioned in h= is last email.

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

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


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, 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 P= ereira <jdealmeidapereira@pivotal.io> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px= #ccc solid;padding-left:1ex">
haha,
Just joking, now w= e have a good version that passes tests and all.

W= e 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 user created panel without defining isMoveable then it should b=
e 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 <jdealmeidapereira@p= ivotal.io> wrote:
Hi,
Apparently the last version was not applying, here is the = new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereir= a@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything= looks fine.=C2=A0 We also refactors some parts to include things like stri= ct equality and using let/const instead of var.=C2=A0 The updated patch is = attached.
In the future, it will be more valuable to have the tra= nslation to ES6 and the feature work in separate commits so it is easier 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 com= mit user-visible changes, to make it easier to build the release notes.

=C2=A0 =C2=A0Sure=C2= =A0

Thanks.

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

Please find = the updated patch, Now we are able to lock wcFrame and wcPanel both.
<= div class=3D"gmail_extra">
=
--
Regards,
<= font size=3D"2">Murtuz= a Zabuawala
EnterpriseDB:=C2=A0http://www.enterprisedb.comThe 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, D= ave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at = 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprise= db.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Da= ve Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 a= t 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterpri= sedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page = <dpage@p= gadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza= Zabuawala <murtuza.zabuawala@enterprisedb.com&g= t; 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 1= 2:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprised= b.com> wrote:
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:
=

=E2=80=8BHello,

Please find updated patc= h,=C2=A0

<= /font>
Now layout will be locked = after user updates its preferences, w
e have used =E2=80=8B
<= div dir=3D"ltr">
templated variable in the javascript file
=
= =E2=80=8B because we do not have preference module or preference cache avai= lable 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 rev= iew comments.
<= /div>
Looks like everything is working when we chang= e 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 anot= her one that is related to this story.


<= div>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 man=
y blank lines (2)=C2=A0
1=C2=A0
= =E2=80=8BFixed=E2=80=8B
=C2=A0

=

@Dave/Pivota= l 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 faci= ng an issue while handling the Browser tree section, It is a wcDocer = frame and not a wcDocker panel.=C2=A0Like wcDocker panel, wcD= ocker frame do not provide any API so that a developer can prevent drag-dro= p functionality on it.

It's not working fine for me. F= or example, if I put the SQL Panel on it's own below the properties/sta= ts panels (so it looks like pgAdmin 3 used to by default), and then lock th= e layout, I can un-dock the SQL panel into a dialogue, but then cannot re-d= ock it. I can do weird things with the browser tree as well, probably becau= se 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 becomes individual Frame,= That is what the author of the wcDocker replied on my question,=C2=A0
"A panel must either be initialize= d as movable or non-movable from the beginning and never changed because it= generates a different arrangement of elements depending. This feature shou= ld only ever be used within the onCreate method of the panel. I should prob= ably 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 g= roup?
=E2=80=8BN= o, it stays Frame.=E2=80=8B
=C2=A0
As far as I underst= and Panel needs a Frame to render itself if it is not attached to the main = docker instance.=E2=80=8B
<= div dir=3D"ltr">
=
There must be some way we can lock a tab that's not part= of a group.
At a mom= ent there is no way of =E2=80=8B
locking= frames out of the box :(
=E2=80=8B

Hmm, so the question becomes: do we inclu= de the lock feature, but rename it to "Lock Tabs" or something si= milar, or leave it out altogether? 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 default layout only, And I don't think mos= t user will try to drag drop Browser panel =E2=80=8B
anyway, mea= nwhile I'll check what changes are required in main source code to make= the Frame lock.

<= /div>
Anyone 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

100% agree. I can convince my se= lf that this feature request has to do with locking panels into a certain l= ayout. I can also convince myself that the same request is simple because u= sers are frustrated with the fact that the tabs and panes move around and t= hey find that behavior annoying.=C2=A0

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

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






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twit= ter: @pgsnake

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



--
=
Akshay Joshi
Sr. Software Architect

<= font color=3D"#3333FF">
=



--
=
Akshay Joshi
<= span style=3D"color:rgb(0,0,0);font-family:arial,sans-serif;font-size:13px;= border-collapse:collapse">Sr. Software Architect



=

<= div>
--
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
Twitte= r: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgr= eSQL Company
--00000000000002198f056ac11758--