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

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(dot)zabuawala(at)enterprisedb(dot)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>*
>>>
>>