Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d6l1p-0005FX-S6 for pgadmin-hackers@arkaria.postgresql.org; Fri, 05 May 2017 21:44:46 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d6l1p-0001u4-Em for pgadmin-hackers@arkaria.postgresql.org; Fri, 05 May 2017 21:44:45 +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 1d6l1o-0001qj-If for pgadmin-hackers@postgresql.org; Fri, 05 May 2017 21:44:44 +0000 Received: from mail-io0-x22d.google.com ([2607:f8b0:4001:c06::22d]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d6l1k-00039W-QW for pgadmin-hackers@postgresql.org; Fri, 05 May 2017 21:44:43 +0000 Received: by mail-io0-x22d.google.com with SMTP id p24so23752234ioi.0 for ; Fri, 05 May 2017 14:44:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pivotal-io.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=8NDJ4pQz2yZQWZkxGmMul0Hb2fIfqKzZByDudWPnPss=; b=hsyd+KP4tLWszEMv5tK2CQgfnWtcKnIDE2J1kkKtrbJ8bZqT1yVV6gTAlGfnbQCfAE gdNW5dXmHQ4u4Kb8BJ0Lm3MZJTP4oO7k2D1ZqE/EZd1fFG7tYclME6i4Dyp818WrM0GR N460zemBywZFxCo/EmmLZKOMSa1FFgqMS8evFYjCnhIXCm9iZ8cFr4BYteTmsnMiwI7X F3c5szvGNp7trizSArX8+PSYcuWqYYaItPr1Smb8IbvV1Ney+ipSSIrZZ3PO0jFjJoSc t8bAvuN9Zdyo2U8O0fKWt9DfdYDWRkNlWff709ZtlDxbdW5ed9PGL87IHlNmMdRs/UIY U0fw== 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=8NDJ4pQz2yZQWZkxGmMul0Hb2fIfqKzZByDudWPnPss=; b=PQZ8z9kMI1skFFRMbQaSpCn36PuQaJUCw6WkstpSGNERK9Jz3PuJehFAz44wbnpCbc IJ6+MWhnCCJP02XdSKoOpjKH9arTnTP2ldpYNVOkmy6ySHSihrUgU8q58KPoZ8iyeusV EG5U7RfPaEgK0UY6L+koYa2WANcJ5JrFXcI7j4+twIoXvGXgZMH4cUcc0sQc2doFPAz8 D1odsRl+7Ip5ZR2ln4whoaJUYMDwOKEi620yg7OHHz0wyE3XDIyukgTU6EtACA2+yOyU CHlskrvK9a3+bWHpwkWfnmSwd66SWU0s1GtkOMz9qBPqzt3TTyp0dh3a08GKe/vQ/Fgn u84A== X-Gm-Message-State: AN3rC/4bo8KS8joOKyd39VNI+W4B1bGj7FTH28hAC/5p7JlUnXH6Zaq+ +1GQLmVM4x9XhBoOP+l2hBuvj/FOWOwP X-Received: by 10.107.130.99 with SMTP id e96mr47825078iod.38.1494020679365; Fri, 05 May 2017 14:44:39 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.169.216 with HTTP; Fri, 5 May 2017 14:44:18 -0700 (PDT) In-Reply-To: References: From: Sarah McAlear Date: Fri, 5 May 2017 17:44:18 -0400 Message-ID: Subject: Re: Issue with SlickGrid To: Murtuza Zabuawala Cc: Dave Page , pgadmin-hackers , Matthew Kleiman Content-Type: multipart/alternative; boundary=001a113f26a6e190fe054ecdcbc8 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 --001a113f26a6e190fe054ecdcbc8 Content-Type: text/plain; charset=UTF-8 Hi Murtuza! 1) > Individual cell selection is not visible, > This will be useful to user when user wants to use "By Selection" & > "Exclude Selection" filter option. > (Even if active cell is not visible to user, above filters are working > though when we click on any cell) Good catch. We'll fix that. 2) Column header does not look good due bottom padding. > Refer screenshot. We think that we actually fixed that with a patch we are currently working on and will be submitting next week. 3) Select All color selection issue, > try running, > select * from pg_class > Now scroll horizontally till the last column and again scroll back to > first column, > you will see that one of the column gets the blue color as if it is > selected by user. > Refer screenshot. Good catch with this one too! We actually think this is a bug that may have been introduced before this. We were able to replicate the same issue without the patch being applied. We're going to take this bug on in our backlog and include the fix in the upcoming patch we just mentioned. This only seems to be a bug when the table has more columns than fits on the viewport. We think we have an idea of how to fix this. Thanks! Matt & Sarah On Wed, May 3, 2017 at 12:26 AM, Murtuza Zabuawala < murtuza.zabuawala@enterprisedb.com> wrote: > Hi Jao, > > I found few minor issues, > > 1) > Individual cell selection is not visible, > This will be useful to user when user wants to use "By Selection" & > "Exclude Selection" filter option. > (Even if active cell is not visible to user, above filters are working > though when we click on any cell) > > 2) > Column header does not look good due bottom padding. > Refer screenshot. > > 3) > Select All color selection issue, > > try running, > select * from pg_class > > Now scroll horizontally till the last column and again scroll back to > first column, > you will see that one of the column gets the blue color as if it is > selected by user. > Refer screenshot. > > Rest of the patch looks good to me. > > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > On Thu, Apr 27, 2017 at 9:23 PM, Dave Page wrote: > >> 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 < >>> 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 >> > > > > -- > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgadmin-hackers > > --001a113f26a6e190fe054ecdcbc8 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Murtuza!

1)
Individual cell selection is not visible,
T= his will be useful to user when user wants to use "By Selection" = & "Exclude Selection" filter option.
(Even if active cell = is not visible to user, above filters are working though when we click on a= ny cell)

Good catch. We'll fix that.

2)=C2=A0
Column header does not look good due bot= tom padding.
Refer screenshot.

We think that we actuall= y fixed that with a patch we are currently working on and will be submittin= g next week.

3)=C2=A0=
Select All c= olor selection issue,
try running,=C2=A0
select * from pg_class
No= w scroll horizontally till the last column and again scroll back to first c= olumn,=C2=A0
you will see that one of the column gets the blue color as = if it is selected by user.
Refer screenshot.

Good catch= with this one too! We actually think this is a bug that may have been intr= oduced before this. We were able to replicate the same issue without the pa= tch being applied. We're going to take this bug on in our backlog and i= nclude the fix in the upcoming patch we just mentioned.=C2=A0
This only seems to be a bug when the table has more= columns than fits on the viewport. We think we have an idea of how to fix = this.=C2=A0

Thanks!
Matt & Sarah


On Wed, May 3, 2017 at 12:26 AM, Murtuza Zabuawala &= lt;= murtuza.zabuawala@enterprisedb.com> wrote:
Hi Jao,

I found few minor issues,
<= br>
1)
Individual cell selection is not visible,
<= div>This will be useful to user when user wants to use "By Selection&q= uot; & "Exclude Selection" filter option.
(Even if = active cell is not visible to user, above filters are working though when w= e click on any cell)

2)
Column header do= es not look good due bottom padding.
Refer screenshot.
=
3)=C2=A0
Select All color selection issue,

try running,=C2=A0
select * from pg_class
=

Now scroll horizontally till the last column and again = scroll back to first column,=C2=A0
you will see that one of the c= olumn gets the blue color as if it is selected by user.
Refer scr= eenshot.

Rest of the patch looks good to me.
=


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

On Thu, Apr 27= , 2017 at 9:23 PM, Dave Page <dpage@pgadmin.org> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px= #ccc solid;padding-left:1ex">
Thanks Joao. Murtuza, can yo= u review this please?

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

We fo= und 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&#= 39;t apply any of the custom changes that were previously added. Please val= idate that the memory issues that were referenced in the README file are so= lved with the latest version of SlickGrid. If we can avoid changing the cod= e of the libraries that we use, it will be far easier to continue to upgrad= e in the future. We will need to upgrade the version of SlickGrid again soo= n, once they approve our pull request.

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 Zabuawala
EnterpriseDB:=C2=A0http://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 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
Enter= priseDB:=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 ZabuawalaEn= terpriseDB:=C2=A0http://www.enter<= wbr>prisedb.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 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
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




--
<= div class=3D"m_7018031743816182203m_3327124699761151941gmail_signature" dat= a-smartmail=3D"gmail_signature">Dave Page
Blog: http://pgsnake.blogspot.com
Twitte= r: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgr= eSQL Company



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


--001a113f26a6e190fe054ecdcbc8--