Hi Joao, On Wed, Apr 25, 2018 at 6:55 PM, Joao De Almeida Pereira < jdealmeidapere...@pivotal.io> wrote:
> Hi, > @Murtuza: We didn't notice the issue, can you please advise on what need > to change to make it work? The only change we did was to make one test pass. > I moved to another project so I didn't get a chance to look into the code but a s you are aware that we are no longer considering given the patch as a fix for the issue instead someone from the team might fork the code and add the option in the library itself. Regards, Murtuza > @Hackers: In our point of view it is never good to fork a library. But if > he really have to do it, then we should fork it in Github, make our code > accessible to other people, and we should add it as a dependency on > package.json > > > Thanks > Anthony & Joao > > > On Wed, Apr 25, 2018 at 7:14 AM Dave Page <dp...@pgadmin.org> wrote: > >> On Wed, Apr 25, 2018 at 12:13 PM, Murtuza Zabuawala < >> murtuza.zabuaw...@enterprisedb.com> wrote: >> >>> Hi Dave, >>> >>> On Wed, Apr 25, 2018 at 3:36 PM, Dave Page <dp...@pgadmin.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.zabuaw...@enterprisedb.com> wrote: >>>> >>>>> Hi Akshay, >>>>> >>>>> >>>>> On Wed, Apr 25, 2018 at 2:37 PM, Akshay Joshi < >>>>> akshay.jo...@enterprisedb.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 < >>>>>> 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.zabuaw...@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>* >>>>>>>>>> >>>>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> *Akshay Joshi* >>>>>> >>>>>> *Sr. Software Architect * >>>>>> >>>>>> >>>>>> >>>>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 >>>>>> 976-788-8246 <+91%2097678%2088246>* >>>>>> >>>>> >>>>> >>>> >>>> >>>> -- >>>> 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 >> >
