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

On Wed, Apr 25, 2018 at 12:13 PM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:

> Hi Dave,
>
> On Wed, Apr 25, 2018 at 3:36 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> All,
>>
>> We just had a brief discussion in our EDB sprint planning meeting about
>> this. There is a non-zero chance that we're going to have to fork wcDocker
>> in the near future, in order to update it to work with jQuery 3. If we do
>> that, then it may be significantly easier to fix this issue in that fork
>> (perhaps by adding a single lockLayout(bool) function, rather than trying
>> to do so from pgAdmin.
>>
>> I think (unless Murtuza believes that won't help), that we're better off
>> holding on this for now until we know if we've had to do that.
>>
>
> ​I don't have any objection forking the code and adding the flag to lock
> the panel, But I'm certain that
> we will use the same inbuilt method *panel.moveable(false)* which we have
> used right now in the patch to prevent a panel from floating and will face
> the same issue again which Akshay mentioned in his last email.
>
> Let me know if you want me to attach latest patch onto the ticket for
> future reference and update the ticket accordingly​.
>

That's probably a good idea - thanks.

>
>
>> On Wed, Apr 25, 2018 at 10:23 AM, Murtuza Zabuawala <
>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Akshay,
>>>
>>>
>>> On Wed, Apr 25, 2018 at 2:37 PM, Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Joao/Murtuza
>>>>
>>>> It break's the functionality, I am able to move "Data output",
>>>> "Explain" etc.. panel of Query Tool, even if "Lock layout?" is set to True.
>>>>
>>>
>>> ​It's working properly in v5 patch, Something went wrong while
>>> refactoring.​
>>>
>>> ​
>>>
>>> Apart from above I have found more issue. Below are the steps to
>>>> reproduce:
>>>>
>>>> - Set the "Lock layout?" flag to False.
>>>> - Move out Dashboard panel.
>>>> - Set the "Lock layout?" flag to True.
>>>> - Close the Dashboard panel, as layout is locked and empty
>>>> Dashboard panel is still visible. (Refer attached screenshot)
>>>>
>>>> ​That's because we have set the Panel moveable property to False, they
>>> won't auto resize, As discussed earlier if user drag any panel out of panel
>>> group it gets render in seprate wcFrame. I think that needs to be taken
>>> care by user before they decide to lock the layout, We can not expilcitly
>>> set panel's closeable property to False when layout is locked, If we do so
>>> user will not be able to close any Query tool, Debugger panels.​
>>> ​
>>>
>>>
>>> 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(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>*
>>>>>>>>
>>>>>>>
>>>>
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>>
>>>> *Sr. Software Architect *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>
>>
>> --
>> 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