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