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

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][RM#3155] Allow user to lock the Layout
Date: 2018-04-24 08:58:30
Message-ID: CANxoLDfBKV0oF42V0d35jQWUwACx05zqv06T_GFz6D75p9joag@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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-9517Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2018-04-24 08:59:12 Re: [PATCH] [RM# 3290] Close button missing for the message window from the backend server
Previous Message Dave Page 2018-04-24 08:17:34 Re: [PATCH] [RM# 3290] Close button missing for the message window from the backend server