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 1f3No5-0002Mu-Dx for pgadmin-hackers@arkaria.postgresql.org; Tue, 03 Apr 2018 15:25:09 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1f3No4-0006P3-Bz for pgadmin-hackers@arkaria.postgresql.org; Tue, 03 Apr 2018 15:25:08 +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 1f3No3-0006On-Ul for pgadmin-hackers@lists.postgresql.org; Tue, 03 Apr 2018 15:25:08 +0000 Received: from mail-it0-x231.google.com ([2607:f8b0:4001:c0b::231]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1f3Nnz-0002Fj-M3 for pgadmin-hackers@postgresql.org; Tue, 03 Apr 2018 15:25:05 +0000 Received: by mail-it0-x231.google.com with SMTP id e98-v6so23281906itd.4 for ; Tue, 03 Apr 2018 08:25:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pivotal-io.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=d0sbPNhXzC8kXjxyIvfLql5Vn18sBruHoyPGyLKgyJI=; b=fo+XqahvJAc8j9/1+4sfX9ONyBJcoO/s0KdJnQA7yTnwWgl6w9DT4YJbfpisMk4b6W WcWBeCxiMaz4rcSl8aUfxUpQJnhHo+iRyiQqpbO0aYpILDOBw2nM9iJCgpCxGqbLdjyE 9noN511Ai/xB2bjxN2M2ld1pL6UK/KHj6wIJzJ4/56hnMVT3fV+1vX+j8J5Y44A37EP0 g8VZRPKE/r6SbF9Knu2+4pLyNPNOvZOa7PNuSVHs7LMtsAkvV58ADgR+u0gTqQaJQxhF T6w1fbfB9zlyjLhwU2fOoMltx1wkeFcUoaRm6NrQ+zU1Zv/ZH0wFACwZ91fzTq+LdN1h ABtw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=d0sbPNhXzC8kXjxyIvfLql5Vn18sBruHoyPGyLKgyJI=; b=K3TBAv/8nAK0xKWP0gP9O0jRd5fuJeTLgm6zHX4DvVXtvwBEGX9WCDnIsimPnGA3OX H6yrFQgrMtKRuazgN1WbYxrJvGNhBmTukxJQm0Whbb+uqyDQx8SQGJ/3z418gDLLHrcl tpECaIy6KnprrDYt0MU5aoKiAIbaotIFc6B2UUgnC0YuApg5BJVSrXlQDwmSuZGFkoIM OIO0iHAb3QVi9o+vACr/3RtyTgj02vftd2EccEke8ufllM+P+TNK5Zt455ApWFRwum/u LnXpO4f3c6lBhvoCHoEUwn8TYsvYrHD1eY5767rH5P8RZG+vuMfbX5J8RRUPuYB9Mndd G15g== X-Gm-Message-State: AElRT7FZJfM2RMVvf2NjWHlWjPKTGjFy+ER5gN2TSoqRKZ1diJrXixT7 4vy0pDiVuG36pLCK1191uTUZH6Kydk8Ri4g13tlXXw== X-Google-Smtp-Source: AIpwx48R1Mp384biUMp5osnWGEG/gg++7stAghgT0kBHceS1N7m4OctE2srKv48lb6ux67Yx6MJky1ZomfoIhpENSwo= X-Received: by 2002:a24:496a:: with SMTP id z103-v6mr5192023ita.133.1522769102646; Tue, 03 Apr 2018 08:25:02 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Joao De Almeida Pereira Date: Tue, 03 Apr 2018 15:24:52 +0000 Message-ID: Subject: Re: [pgAdmin4][RM#3155] Allow user to lock the Layout To: Murtuza Zabuawala Cc: Dave Page , pgadmin-hackers Content-Type: multipart/alternative; boundary="0000000000007050420568f34fe5" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000007050420568f34fe5 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Murtuza, Everything seems to work, the tests are all green and the linter is fixed. As we stated in an previous email, the direction we would love the application to go is a more robust Javascript Front-End that would rely in the Backend to provide data. Adding more things to templated Javascript files feels like a step back and something that we will in the future have to convert into AJAX calls and JSON responses. As discussed before our idea is to remove all the javascript templated files. How hard do you think it would be to do this implementation without using templates? Thanks Victoria & Joao On Tue, Apr 3, 2018 at 7:57 AM Murtuza Zabuawala < murtuza.zabuawala@enterprisedb.com> wrote: > Hi, > > Thanks Joao for reviewing. > > PFA updated patch. > > On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira < > jdealmeidapereira@pivotal.io> wrote: > >> Hello, >> >> On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala < >> murtuza.zabuawala@enterprisedb.com> wrote: >> >>> >>> =E2=80=8BHello, >>> >>> Please find updated patch, >>> >>> Now layout will be locked after user updates its preferences, w >>> e have used =E2=80=8B >>> templated variable in the javascript file >>> =E2=80=8B because we do not have preference module or preference cache = available >>> when the page loads and panels gets rendered, >>> =E2=80=8BI >>> =E2=80=8B also >>> made changes in JS tests as per Joao's review comments. >>> >> Looks like everything is working when we change the lock. >> As a personal preferences I would prefer to see this in at least 2 >> commits, one that is related to the preference issue and another one tha= t >> is related to this story. >> >> >> All the tests are working, but he linter is failing: >> >> /tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2 >> >> ./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2) >> >> 1 E303 too many blank lines (2) >> >> >> 1 >> > =E2=80=8BFixed=E2=80=8B > > >> >> >>> @Dave/Pivotal team, >>> The given patch is working fine for all the Tabs/Panels (all the panels >>> from main window as well as from Query tool and Debugger) but I'm facin= g 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 develop= er >>> can prevent drag-drop functionality on it. >>> >>> By visiting wcDocker github page = It >>> looks like it not actively maintained. >>> What do you suggest how should we tackle this issue? >>> >>> >> I think this should be moved to a different thread, because at this poin= t >> in time we have 3 of our core libraries that are no longer >> maintained/supported/under active development that I know out of my head= . >> (ACITree, Backbone and wcDocker). I might even add to the mix jquery 1.1= 1.2 >> because it stopped being actively developed and supported after May 20 o= f >> 2016. >> > =E2=80=8BSure, I'll send separate email.=E2=80=8B > > >> >> >>> For time being, I've created subtask for this issue >>> https://redmine.postgresql.org/issues/3243 >>> >>> Thanks, >>> Murtuza >>> =E2=80=8B >>> On Thu, Mar 29, 2018 at 8:57 PM, Joao De Almeida Pereira < >>> jdealmeidapereira@pivotal.io> wrote: >>> >>>> Hi Murtuza, >>>> >>>> After changing the setting in the preferences nothing happened, we had >>>> to reset the layout or refresh the app to see it working. It only look= s the >>>> right side. Was this the intended behavior? >>>> >>>> Not sure if this is the expected behavior or not. I would expect that >>>> any change I do in the preferences would start working after I press t= he >>>> Save button. This also happens with other preferences that only take e= ffect >>>> after refresh on the browser. >>>> This being said, not sure if having the templated variable in the >>>> javascript file is the best approach in this case. >>>> >>>> Do you think you can remove the requirejs tags on the tests? >>>> >>>> At the testing file you do not need to create 3 different variables fo= r >>>> the panels, you can reuse it, because the beforeEach will run for ever= y test >>>> >>>> Thanks >>>> Joao >>>> >>>> On Thu, Mar 29, 2018 at 9:48 AM Dave Page wrote: >>>> >>>>> Hi >>>>> >>>>> On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala < >>>>> murtuza.zabuawala@enterprisedb.com> wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> PFA patch which will allow user to lock the panels and it will not >>>>>> allow user to drag & drop them. >>>>>> >>>>> >>>>> Tests pass, but when I lock the layout, I can still drag panels and >>>>> adjust the splitters etc. After doing so, reset the layout and now h= ave >>>>> the broken layout seen in the attached screenshot. I have rebuilt the >>>>> bundle, reloaded etc. >>>>> >>>>> -- >>>>> Dave Page >>>>> Blog: http://pgsnake.blogspot.com >>>>> Twitter: @pgsnake >>>>> >>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>> The Enterprise PostgreSQL Company >>>>> >>>> >>> --0000000000007050420568f34fe5 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Murtuza,

Everything seems to work, t= he tests are all green and the linter is fixed.

As= we stated in an previous email, the direction we would love the applicatio= n to go is a more robust Javascript Front-End that would rely in the Backen= d to provide data. Adding more things to templated Javascript files feels l= ike a step back and something that we will in the future have to convert in= to AJAX calls and JSON responses.
As discussed before our idea is= to remove all the javascript templated files.

How= hard do you think it would be to do this implementation without using temp= lates?

Thanks
Victoria & Joao
<= /div>
On Tue, Apr 3, 2018 at= 7:57 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

P= FA 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:<= br>

=E2=80=8BHello,

Please find updated pat= ch,=C2=A0

=
Now layout will be locked= after user updates its preferences, w
e have used =E2=80=8B
templated variable in the ja= vascript file
<= /div>
= =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 many blank lines (2)=C2=A0
<=
a href=3D"https://gpdb-dev.bosh.pivotalci.info/teams/main/pipelines/pgadmin=
-feature-branches/jobs/pivotal-rm-3155-python-linter/builds/3#L5ab982d1:11"=
 style=3D"text-decoration-line:none;color:rgb(236,240,241)" target=3D"_blan=
k">
1=C2=A0
=
=E2=80= =8BFixed=E2=80=8B
=C2=A0

=

<= div style=3D"font-family:verdana,sans-serif;color:rgb(68,68,68)">@Dave/Pivo= tal team,
The given patch is working fine for all the Tabs/Panels (all the pane= ls from main window as well as from Query tool and Debugger) but I'm fa= cing an issue while handling the Browser tree section, It is a wcDoce= r frame and not a wcDocker panel.=C2=A0Like wcDocker panel, w= cDocker frame do not provide any API so that a developer can prevent drag-d= rop functionality on it.

By visiting=C2=A0wcDocker github page=C2=A0It looks like = it not actively maintained.
What do you suggest how should we tackle this issue= ?
<= /blockquote>

I think this should be moved to a different= thread, because at this point in time we have 3 of our core libraries that= are no longer maintained/supported/under active development that I know ou= t of my head. (ACITree, Backbone and wcDocker). I might even add to the mix= jquery=C2=A01.11.2 because it stopped being actively developed and support= ed after May 20 of 2016.
<= div class=3D"gmail_default" style=3D"font-family:verdana,sans-serif;color:r= gb(68,68,68);display:inline">=E2=80=8BSure, I'll send separate email.= =E2=80=8B
=C2=A0
=C2=A0
For time being,= I've created subtask for this issue=C2=A0https://redmine.postgresql.org/= issues/3243

Thanks,
Murtuza
=C2=A0=E2=80=8B
On Thu, Mar 29, 2018 at 8:57 PM, Joao De Almei= da Pereira <jdealmeidapereira@pivotal.io> wrote:<= br>
Hi Mu= rtuza,

After changing the setting in the preferences not= hing happened, we had to reset the layout or refresh the app to see it work= ing. It only looks the right side. Was this the intended behavior?

Not sure if this is the expected behavior or not. I would = expect that any change I do in the preferences would start working after I = press the Save button. This also happens with other preferences that only t= ake effect after refresh on the browser.
This being said, not sur= e if having the templated variable in the javascript file is the best appro= ach in this case.

Do you think you can remove the = requirejs tags on the tests?

At the testing file y= ou do not need to create 3 different variables for the panels, you can reus= e it, because the beforeEach will run for every test

Thanks
Joao

On Thu, Mar 29, 2018 at 9:48 AM Dave Page <dpage@pgadmin.org> wrote:
Hi
On Thu,= Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <murtuza.za= buawala@enterprisedb.com> wrote:
Hi,

PFA patch which will allow user to lock t= he panels and it will not allow user to drag & drop them.

Tests pass, but when I lock th= e layout, I can still drag panels and adjust the splitters etc. After doing= so,=C2=A0 reset the layout and now have the broken layout seen in the atta= ched screenshot. I have rebuilt the bundle, reloaded etc.=C2=A0
=
<= div dir=3D"ltr">

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

En= terpriseDB UK: ht= tp://www.enterprisedb.com
The Enterprise PostgreSQL Company

= --0000000000007050420568f34fe5--