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 1fB0gV-0006Jq-U1 for pgadmin-hackers@arkaria.postgresql.org; Tue, 24 Apr 2018 16:20: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 1fB0gU-00063Y-Sb for pgadmin-hackers@arkaria.postgresql.org; Tue, 24 Apr 2018 16:20: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 1fB0gU-00063M-7H for pgadmin-hackers@lists.postgresql.org; Tue, 24 Apr 2018 16:20:50 +0000 Received: from mail-ot0-x22a.google.com ([2607:f8b0:4003:c0f::22a]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1fB0gQ-0000mO-4F for pgadmin-hackers@postgresql.org; Tue, 24 Apr 2018 16:20:48 +0000 Received: by mail-ot0-x22a.google.com with SMTP id p2-v6so21927558otf.3 for ; Tue, 24 Apr 2018 09:20:45 -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=x+zWW0D/jZ6/eo/V0bxNXJQIwmyJgoZ49A5yggeJyzM=; b=nCJ2fzMorjcnOW7O77hOqxlayEY/h+9lrgE6iTIHN7PYN2tbQFZtN1cEGg0RbvpyyB SuBHip99/eEpT72wsWDMtpWICYAh4H/uhH6d4QDYG7CBZVbnTDfd0DXGR7+kx6b5Na2o PLdVG7S6JnTK2Wx+2QYqBrGLr319VM5lP7nYZ2FoSf41xwxv9Q0Z82ZHIA5yoaI/URtl y7sr+gGCGOyO4k0qh2BOw2Vx9BUtfIBcxV2gcwZ+3PbZuioa+16donB4K6UjQPppXwKx scfJ/EvNvbH3NpMVzziZOk3vuncDAy1T27S5EYunWm4cQHnz//JOCpkb7m/NKespUNss 4lww== 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=x+zWW0D/jZ6/eo/V0bxNXJQIwmyJgoZ49A5yggeJyzM=; b=p51SJs3t7F6NRRFtjbTlyE7TCeAUQt+2eAB19HFquIEYgBjnUoCV0d3JZ/PLAHfGV+ ISeJopiyT+qhNvIkTcUY7aafNx+28UdsbqbezEo0vqwuEkjD61YVPucPoJBNA3V4Az4s 0S0VDPz00L9h2pCliEZgeMEb6ptz6TvkPsa8HBtDupEFgG1moXEBPt+J/rW+CPmjHzh9 cJsxOCXao9iPZ/ZH9u0vtIef+CSZSNrHJ5h6pwwNNP+iDDcMtYDSqj4BRnP4zMjDUD8v NTADedWZ+g+PyWZFhYy+/XbqZfvfGBAMbP5My8JsvJoVT5fRFWfA90i1MRvIsVgJq921 +/QA== X-Gm-Message-State: ALQs6tDXCqMh/ZwT9Jr4vN20jfQlNEhkFxxp70AwpZpwMnx8QNvekgTK TO7+qWyqVrXdJZtOwlYVXTt9S0c3rKMnaZCtDV2s+A== X-Google-Smtp-Source: AB8JxZoQanq628aL0lN9YlT9EleXwVS39jKpUknCIMyHlj4oR3IIBrGKHTXjH1B3ZhYoxDXsWnd8Rcr1HNXp2HDDF7w= X-Received: by 2002:a9d:2842:: with SMTP id h2-v6mr12703096otd.210.1524586845121; Tue, 24 Apr 2018 09:20:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.201.89.5 with HTTP; Tue, 24 Apr 2018 09:20:24 -0700 (PDT) In-Reply-To: References: From: Murtuza Zabuawala Date: Tue, 24 Apr 2018 21:50:24 +0530 Message-ID: Subject: Re: [pgAdmin4][RM#3155] Allow user to lock the Layout To: Joao De Almeida Pereira Cc: Akshay Joshi , Dave Page , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000556a4c056a9a89d1" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000556a4c056a9a89d1 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thank you Joao for reviewing. 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 sho= uld call moveable method with true as argument FAILED > Expected false to be true. > at UserContext. (regression/javascript/browser/panel_s= pec.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/const instea= d 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 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 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 notes. >>>>> >>>> >>>> Sure >>>> >>>>> >>>>> Thanks. >>>>> >>>>> On Tue, Apr 24, 2018 at 8:45 AM, 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 >>>>> > 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 pr= eference >>>>>>>>>>>>>>>>>> cache available when the page loads and panels gets rend= ered, >>>>>>>>>>>>>>>>>> =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 a= t >>>>>>>>>>>>>>>>> least 2 commits, one that is related to the preference is= sue 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/4a5630= c2 >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> ./pgadmin/misc/__init__.py:78: [E303] too many blank line= s (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 t= ool and Debugger) >>>>>>>>>>>>>>>>>> but I'm facing an issue while handling the Browser tree = section, It is a wcDocer >>>>>>>>>>>>>>>>>> frame >>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>> not a wcDocker panel >>>>>>>>>>>>>>>>>> . Li= ke >>>>>>>>>>>>>>>>>> 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-dock it. I ca= n 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 yo= u 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, >>>>>>>>>>>>>> *"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 sh= ould only ever >>>>>>>>>>>>>> be used within the onCreate method of the panel. I should pr= obably 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 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 m= ost 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 also convince m= yself >>>>>>> that the same request is simple because users are frustrated with t= he fact >>>>>>> that the tabs and panes move around and they find that behavior ann= oying. >>>>>>> >>>>>>> -- 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>* >>>> >>> --000000000000556a4c056a9a89d1 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thank you Joao for reviewing.
=


On Tue, Apr 24, 2018 at 9:11 PM, Joao De Alm= eida 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 withou=
t defining isMoveable then it should be moveable it should call moveable me=
thod with true as argument FAILED
    Expected false to be true.
        at UserContext.<anonymous> (regression/javascript/browse=
r/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 <jdealmeidap= ereira@pivotal.io> wrote:
Hi,
Apparently the last version was not applying, here i= s the new version that should apply correctly

Than= ks
Victoria & Joao

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

We tested the patch and everyt= hing looks fine.=C2=A0 We also refactors some parts to include things like = strict 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= translation to ES6 and the feature work in separate commits so it is easie= r to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Ap= r 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> w= rote:
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 A= M, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb= .com> wrote:
Hi Dave,

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

<= div>
--
Regards,
Murtuza Zabuawala
EnterpriseDB:=C2=A0http://www.enterprisedb.com
The Enterprise PostgreSQL Company<= /div>

<= /div>

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:

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


On Wed, Apr 4, 2018 at 12:54 PM, Mu= rtuza Zabuawala <murtuza.zabuawala@enterprisedb.com<= /a>> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Z= abuawala <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 <= ;mu= rtuza.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 reviewi= ng.
=
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 patch,=C2=A0

Now layout will be locked after user updates its preferences, w
<= /font>
e have used =E2=80=8B
templ= ated variable in the javascript file
=E2=80=8B because we do not have preference module or prefer= ence 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 comments.
Looks like everything is workin= g when we change the lock.
As a personal preferences I would pref= er 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.

<= 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 many blank lines (2)=C2=A0
=



=
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 ou= t of the box :(
=E2=80=8B



=


=C2=A0





--
Dave Page
Blog: http://pgsnake.blogspot.com
Tw= itter: @pgsnake

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


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

--000000000000556a4c056a9a89d1--