Re: Issue with SlickGrid

From: Joao Pedro De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Matthew Kleiman <mkleiman(at)pivotal(dot)io>
Subject: Re: Issue with SlickGrid
Date: 2017-04-27 15:49:07
Message-ID: CAE+jjamV-ofoRkA4z2Rr21+zujLbabsJUF3iqcWE_8EAuULkpA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
<https://github.com/6pac/SlickGrid/pull/100>.

Thanks,
Joao & Matt

On Thu, Apr 27, 2017 at 8:13 AM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)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(at)pivotal(dot)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(dot)zabuawala(at)enterprisedb(dot)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(dot)zabuawala(at)enterprisedb(dot)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(at)pivotal(dot)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
>>>>> <https://github.com/6pac/SlickGrid/pull/100> 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
>>>>>
>>>>
>>>>
>>>
>

Attachment Content-Type Size
0001-Update-SlickGrid-to-latest-version.patch application/octet-stream 317.2 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-04-27 15:53:25 Re: Issue with SlickGrid
Previous Message Robert Eckhardt 2017-04-27 15:44:43 Re: Declarative partitioning in pgAdmin4