Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

Thank you Joao for reviewing.

On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira <
jdealmeidapereira(at)pivotal(dot)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.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
>
> ​
> 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(at)pivotal(dot)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(at)pivotal(dot)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 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 understand what
>>> changed.
>>>
>>> Sincerely,
>>>
>>> Joao and Victoria
>>>
>>>
>>>
>>> On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage(at)pgadmin(dot)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 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(at)pivotal(dot)io
>>>>>> > wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <
>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <
>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <
>>>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <
>>>>>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Dave,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <
>>>>>>>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks Joao for reviewing.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> PFA updated patch.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <
>>>>>>>>>>>>>>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <
>>>>>>>>>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> ​Hello,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Please find updated patch,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Now layout will be locked after user updates its
>>>>>>>>>>>>>>>>>> preferences, w
>>>>>>>>>>>>>>>>>> e have used ​
>>>>>>>>>>>>>>>>>> templated variable in the javascript file
>>>>>>>>>>>>>>>>>> ​ because we do not have preference module or preference
>>>>>>>>>>>>>>>>>> cache available when the page loads and panels gets rendered,
>>>>>>>>>>>>>>>>>> ​I
>>>>>>>>>>>>>>>>>> ​ 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 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
>>>>>>>>>>>>>>>>> <https://gpdb-dev.bosh.pivotalci.info/teams/main/pipelines/pgadmin-feature-branches/jobs/pivotal-rm-3155-python-linter/builds/3#L5ab982d1:9>
>>>>>>>>>>>>>>>>> ./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
>>>>>>>>>>>>>>>>> <https://gpdb-dev.bosh.pivotalci.info/teams/main/pipelines/pgadmin-feature-branches/jobs/pivotal-rm-3155-python-linter/builds/3#L5ab982d1:10>
>>>>>>>>>>>>>>>>> 1 E303 too many blank lines (2)
>>>>>>>>>>>>>>>>> <https://gpdb-dev.bosh.pivotalci.info/teams/main/pipelines/pgadmin-feature-branches/jobs/pivotal-rm-3155-python-linter/builds/3#L5ab982d1:11>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> 1
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> ​Fixed​
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> @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
>>>>>>>>>>>>>>>>>> <http://docker.api.webcabin.org/module-wcFrame.html> and
>>>>>>>>>>>>>>>>>> not a wcDocker panel
>>>>>>>>>>>>>>>>>> <http://docker.api.webcabin.org/module-wcPanel.html>. 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-dock it. I can do weird
>>>>>>>>>>>>>>> things with the browser tree as well, probably because it's a frame as you
>>>>>>>>>>>>>>> say.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> ​That is expected behaviour ​because 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,
>>>>>>>>>>>>>> *"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 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?
>>>>>>>>>>>>>
>>>>>>>>>>>> ​No, it stays Frame.​
>>>>>>>>>>>>
>>>>>>>>>>>> As far as I understand Panel needs a Frame to render itself if
>>>>>>>>>>>> it is not attached to the main docker instance.​
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> 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 ​
>>>>>>>>>>>> locking frames out of the box :(
>>>>>>>>>>>> ​
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 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.
>>>>>>>>>>>
>>>>>>>>>> ​I 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
>>>>>>>>>> will try to drag drop Browser panel ​
>>>>>>>>>> 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
>>>>>>> that the same request is simple because users are frustrated with 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>*
>>>>
>>>