Re: Issue with SlickGrid

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Joao Pedro De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>
Cc: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, 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:53:25
Message-ID: CA+OCxoxq0SOTGXdCP_LnO3b_gQaXQM7WLjLkB3yJpMebvns0Gw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks Joao. Murtuza, can you review this please?

On Thu, Apr 27, 2017 at 4:49 PM, Joao Pedro De Almeida Pereira <
jdealmeidapereira(at)pivotal(dot)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
> <https://github.com/6pac/SlickGrid/pull/100>.
>
> 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(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
>>>>>>
>>>>>
>>>>>
>>>>
>>
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Sarah McAlear 2017-04-27 21:22:45 [Patch] Rename feature test server
Previous Message Joao Pedro De Almeida Pereira 2017-04-27 15:49:07 Re: Issue with SlickGrid