Thank you Joao for reviewing.
On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira < jdealmeidapere...@pivotal.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 < > jdealmeidapere...@pivotal.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 < >> jdealmeidapere...@pivotal.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.jo...@enterprisedb.com> wrote: >>> >>>> On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dp...@pgadmin.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 <reckha...@pivotal.io >>>>>> > wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi < >>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dp...@pgadmin.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala < >>>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>>>>>>> >>>>>>>>>> On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala < >>>>>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala < >>>>>>>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi Dave, >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Hi >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala < >>>>>>>>>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Thanks Joao for reviewing. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> PFA updated patch. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira < >>>>>>>>>>>>>>>> jdealmeidapere...@pivotal.io> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Hello, >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala < >>>>>>>>>>>>>>>>> murtuza.zabuaw...@enterprisedb.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>* >>>> >>>
