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

From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Joao De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][RM#3155] Allow user to lock the Layout
Date: 2018-04-04 06:20:07
Message-ID: CAKKotZTu93swLkwLS_B87Sq0iZs6MkkLRsLhu-X8zjRipm2yPQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.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(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."*

>
>>
>>>> By visiting wcDocker github page <https://github.com/WebCabin/wcDocker> It
>>>> looks like it not actively maintained.
>>>> What do you suggest how should we tackle this issue?
>>>>
>>>
> It may not have been updated recently, but the lead developer answered
> your questions pretty quickly. Maybe he'll be open to a pull request if we
> can figure out how to lock the layout of the frame as well.
>
>
>>
>>>>
>>> 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 out of my head.
>>> (ACITree, Backbone and wcDocker). I might even add to the mix jquery 1.11.2
>>> because it stopped being actively developed and supported after May 20 of
>>> 2016.
>>>
>> ​Sure, I'll send separate email.​
>>
>>
>>>
>>>
>>>> For time being, I've created subtask for this issue
>>>> https://redmine.postgresql.org/issues/3243
>>>>
>>>> Thanks,
>>>> Murtuza
>>>> ​
>>>> On Thu, Mar 29, 2018 at 8:57 PM, Joao De Almeida Pereira <
>>>> jdealmeidapereira(at)pivotal(dot)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 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 take effect
>>>>> 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
>>>>> for the panels, you can reuse it, because the beforeEach will run for every
>>>>> test
>>>>>
>>>>> Thanks
>>>>> Joao
>>>>>
>>>>> On Thu, Mar 29, 2018 at 9:48 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <
>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)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 have
>>>>>> 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
>>>>>>
>>>>>
>>>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Attachment Content-Type Size
Screen Shot 2018-04-04 at 11.40.23 am.png image/png 421.3 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Murtuza Zabuawala 2018-04-04 07:24:44 Re: [pgAdmin4][RM#3154] Update modules to latest version
Previous Message Khushboo Vashi 2018-04-04 05:26:29 Re: [pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations