Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d3ljf-0005YF-1r for pgadmin-hackers@arkaria.postgresql.org; Thu, 27 Apr 2017 15:53:39 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d3lje-0001k6-L8 for pgadmin-hackers@arkaria.postgresql.org; Thu, 27 Apr 2017 15:53:38 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1d3ljd-0001jw-OD for pgadmin-hackers@postgresql.org; Thu, 27 Apr 2017 15:53:38 +0000 Received: from mail-it0-x235.google.com ([2607:f8b0:4001:c0b::235]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d3ljW-0003Zs-1a for pgadmin-hackers@postgresql.org; Thu, 27 Apr 2017 15:53:36 +0000 Received: by mail-it0-x235.google.com with SMTP id x188so14551709itb.0 for ; Thu, 27 Apr 2017 08:53:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=6c7jmUyy0OgGGBQap0p5uVpILKpUFmboNesXQqplhU8=; b=s7K2gcUEzKyvKUJY+u4fK/Z+vKIEs8rBvsLIUcCMJ7FEW1rNebusJ8JMYfWiVuhzKk w/RzPyOn2kgxNQXUqMHygeAcoSJQO0CZaaSFhGTUL7wnxr6XISuneGln3l/d5TzjXQLo HQw2zUGb6Yu+qMAC6qz9VcoedMiAiqZiTqMM7DLFqhdWamWIpyf1QljTWjzMfhK7VvVH GknrNXc19to2S0oG66KjPO1wPyIInjonu+Vf261Uxjt0LCaz4Fi5aQ2p873/sAeMt5/a pK48OgtDk8GKrUsrsKquZ9057j+StNUzoPq5YOnjTKsuXIDAdzpDhqzRIzBZDjft2/4T p9kg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=6c7jmUyy0OgGGBQap0p5uVpILKpUFmboNesXQqplhU8=; b=M3z10qJCbimQUS4rrM5KBhMOe9UNN2n+DsMeeZsVkU7vrsk5RArkmVkKS4NWTe+9M0 9Uad2kTyITXY+xw2Ep4wjLBGru88Nc0/0PWtvO1/M5X2Mau4B1TgxfUaGUqcoPyiuP0T M5pc5ykFliQ/LMTlHOjjj/Bl5ZqMQSYjtdxQUW0c/n1WYex3SgGp8g0X/sAhs8qLh86D tailRVuU2pBrKk2ViDuT1sV4D85E55GfOmlzfEnSYs/A4CTDyHYNnYxonY090p2tXBEo k3nuN5DKJ6JaeSNro4fkC6pQfQ1+92QS45QsYrUV1JglC5DNRRCWiFW6yq0mowDoY1T/ tlBg== X-Gm-Message-State: AN3rC/6S54u71wKXrIRcnKI+BBFkqDR+j7t1uJLpRaLhK4hlOvE+/1tA Uuy2bvBhF05ashsm3FujmQTP3Ur3Iw== X-Received: by 10.36.152.196 with SMTP id n187mr4126151itd.28.1493308406581; Thu, 27 Apr 2017 08:53:26 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.174.167 with HTTP; Thu, 27 Apr 2017 08:53:25 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Thu, 27 Apr 2017 16:53:25 +0100 Message-ID: Subject: Re: Issue with SlickGrid To: Joao Pedro De Almeida Pereira Cc: Murtuza Zabuawala , pgadmin-hackers , Matthew Kleiman Content-Type: multipart/alternative; boundary=94eb2c05e9a21dde7b054e27f55e X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --94eb2c05e9a21dde7b054e27f55e Content-Type: text/plain; charset=UTF-8 Thanks Joao. Murtuza, can you review this please? On Thu, Apr 27, 2017 at 4:49 PM, Joao Pedro De Almeida Pereira < jdealmeidapereira@pivotal.io> wrote: > Hi Hackers, > > We found that the latest version of SlickGrid fixes the scrollbar issue. > We have upgraded it to the latest version in our vendor directory and > updated the tests accordingly in the attached patch. > > We didn't apply any of the custom changes that were previously added. > Please validate that the memory issues that were referenced in the README > file are solved with the latest version of SlickGrid. If we can avoid > changing the code of the libraries that we use, it will be far easier to > continue to upgrade in the future. We will need to upgrade the version of > SlickGrid again soon, once they approve our pull request > . > > Thanks, > Joao & Matt > > > On Thu, Apr 27, 2017 at 8:13 AM, Murtuza Zabuawala enterprisedb.com> wrote: > >> No, we didn't. >> >> -- >> Regards, >> Murtuza Zabuawala >> EnterpriseDB: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> >> On Thu, Apr 27, 2017 at 4:42 PM, Joao Pedro De Almeida Pereira < >> jdealmeidapereira@pivotal.io> wrote: >> >>> Hello Murtuza, >>> Thanks for the explanation. Based on what you said it looks like a bug >>> in the library, have you guys considered sending a PR to it? >>> >>> Thanks >>> >>> On Thu, Apr 27, 2017, 2:46 AM Murtuza Zabuawala < >>> murtuza.zabuawala@enterprisedb.com> wrote: >>> >>>> +++ >>>> Reference: https://www.postgresql.org/message-id/CAKKotZRjqb >>>> KAZev81Zk78nikDVXqLKEDV5r%2BsW8Me31Gpzrm_A%40mail.gmail.com >>>> >>>> -- >>>> Regards, >>>> Murtuza Zabuawala >>>> EnterpriseDB: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>>> On Thu, Apr 27, 2017 at 12:09 PM, Murtuza Zabuawala < >>>> murtuza.zabuawala@enterprisedb.com> wrote: >>>> >>>>> Hello Joao, >>>>> >>>>> Yes, We made some changes in SlickGrid library when we integrated it >>>>> into Query tool. >>>>> >>>>> *Issue:* Last row from the query result set was not displaying >>>>> correctly in query tool when we have scrollbar in grid. >>>>> >>>>> The row hight/width pixel size calculations is done inside SlickGrid >>>>> javascript code, Though we tried solve it through CSS but we had no luck, >>>>> so we had no other choice but to do it in library it self. >>>>> >>>>> The changes were, >>>>> 1) "getDataLengthIncludingAddNew()" function (slick.grid.js) to add >>>>> two new rows instead of one when user add values into row (one row is dummy >>>>> & not visible to user so that it displays last row correctly) >>>>> 2) Other change was done into "appendRowHtml()" function to >>>>> calculating the correct number of rows in SlickGrid result as we have added >>>>> our own custom row as mentioned earlier. >>>>> 3) Abbreviated long CSS classes as mentioed in README file. >>>>> >>>>> Apologies we missed to update this change in README. >>>>> >>>>> >>>>> -- >>>>> Regards, >>>>> Murtuza Zabuawala >>>>> EnterpriseDB: http://www.enterprisedb.com >>>>> The Enterprise PostgreSQL Company >>>>> >>>>> On Thu, Apr 27, 2017 at 2:23 AM, Joao Pedro De Almeida Pereira < >>>>> jdealmeidapereira@pivotal.io> wrote: >>>>> >>>>>> Hello Hackers, >>>>>> >>>>>> While doing some changes to the Query Results we found out that there >>>>>> was a issue with Slick grid. >>>>>> >>>>>> The issue that we found was with the CellSelectModel, behaved >>>>>> differently when pressing Ctrl and Command(Mac). We created a PR >>>>>> with the change to >>>>>> changes the behavior of the plugin. >>>>>> >>>>>> When this PR is applied to the SlickGrid library we need to apply it >>>>>> to the current version of SlickGrid that we have vendorized. >>>>>> According to the libraries.txt file we are in version 2.2.4 of the >>>>>> library but a diff between our code and the libraries version 2.2.4 shows >>>>>> differences in the code. >>>>>> >>>>>> Did we do any change to SlickGrid library that is vendorized? Or is >>>>>> just the information in libraries.txt that is incorrect? >>>>>> Does anyone know any problem if we bump the version of SlickGrid to >>>>>> the newer version after the PR is applied? >>>>>> >>>>>> Thanks >>>>>> Joao >>>>>> >>>>> >>>>> >>>> >> > > > -- > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgadmin-hackers > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --94eb2c05e9a21dde7b054e27f55e Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Thanks Joao. Murtuza, can you review this please?

On Thu, Apr 27, 2017= at 4:49 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@p= ivotal.io> wrote:
Hi Hackers,

We found that the latest version of= SlickGrid fixes the scrollbar issue. We have upgraded it to the latest ver= sion in our vendor directory and updated the tests accordingly in the attac= hed patch.

We didn't apply any of the custom c= hanges that were previously added. Please validate that the memory issues t= hat were referenced in the README file are solved with the latest version o= f SlickGrid. If we can avoid changing the code of the libraries that we use= , it will be far easier to continue to upgrade in the future. We will need = to upgrade the version of SlickGrid again soon, once they approve our pull re= quest.

Thanks,
Joao & Matt
=


On Thu, Apr 27, 2017 at 8:13 AM= , Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.= com> wrote:
No, we didn't.=C2=A0

--=
Regards,
Murtuza ZabuawalaE= nterpriseDB:=C2=A0http://www.enter= prisedb.com
The Enterprise PostgreSQL Company

On Thu, Apr 27, 2017 at 4:42 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:

Hello Murtuza,
Thanks for the explanation. Based on what you said it looks like a bug in t= he library, have you guys considered sending a PR to it?

Thanks


On Thu, Apr 27, 2017, 2:46 = AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote= :

=
--
Regards,
Murtuza Zabuawala
EnterpriseDB:=C2=A0http://www.enterprisedb.com
The Enterprise PostgreSQL= Company

On Thu, Apr= 27, 2017 at 12:09 PM, Murtuza Zabuawala <murtuza.zabuawa= la@enterprisedb.com> wrote:
Hello Joao,

Yes, We made some cha= nges in SlickGrid library when we integrated it into Query tool.
=
Issue: Last row from the query result set was not dis= playing correctly in query tool when we have scrollbar in grid.

The row hight/width pixel size calculations is done = inside SlickGrid javascript code, Though we tried solve it through CSS but = we had no luck, so we had no other choice but to do it in library it self.<= /div>
=C2=A0=C2=A0
The changes were,
1) &= quot;getDataLengthIncludingAddNew()" function (slick.grid.js) to = add two new rows instead of one when user add values into row (one row is d= ummy & not visible to user so that it displays last row correctly)
2) Other change was done into "appendRowHtml()" function to= calculating the correct number of rows in SlickGrid result as we have adde= d our own custom row as mentioned earlier.
3) Abbreviated long CS= S classes as mentioed in README file.
=C2=A0
Apolo= gies we missed to update this change in README.

<= /div>

--
Regards,
Murtuza Zabuawala
EnterpriseDB:=C2=A0http://www.enterprisedb.com
The Enterprise PostgreSQ= L Company

On Thu, Apr 27, 2017 at 2:23 AM, Joao Pedro = De Almeida Pereira <jdealmeidapereira@pivotal.io>= wrote:
Hello Hackers,
While doing some changes to the Query Results we found ou= t that there was a issue with Slick grid.=C2=A0

Th= e issue that we found was with the CellSelectModel, behaved differently whe= n pressing Ctrl and Command(Mac). We created a PR with the change to chan= ges the behavior of the plugin.

When this PR is ap= plied to the SlickGrid library we need to apply it to the current version o= f SlickGrid that we have vendorized.
According to the libraries.t= xt file we are in version 2.2.4 of the library but a diff between our code = and the libraries version 2.2.4 shows differences in the code.
Did we do any change to SlickGrid library that is vendorized? = Or is just the information in libraries.txt that is incorrect?
Do= es anyone know any problem if we bump the version of SlickGrid to the newer= version after the PR is applied?

Thanks
<= div>Joao






--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-ha= ckers




--
--94eb2c05e9a21dde7b054e27f55e--