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.
@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 >
