From: | Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com> |
---|---|
To: | Dave Page <dpage(at)pgadmin(dot)org> |
Cc: | Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, 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-05 03:31:39 |
Message-ID: | CAFOhELcMaBBKCMN5Tt5=qU_e3Y2VnqeGxsFHkoyCk7JnoA4zAg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
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.zabuawala@
> enterprisedb.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.
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
From | Date | Subject | |
---|---|---|---|
Next Message | Khushboo Vashi | 2018-04-05 03:35:40 | Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree |
Previous Message | Dave Page | 2018-04-04 19:23:44 | Re: [pgAdmin4][Patch] Remake Docker container packaging |