Re: Issue with SlickGrid

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Dave Page <[email protected]>
To: Joao Pedro De Almeida Pereira <[email protected]>
Cc: Murtuza Zabuawala <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Cc: Matthew Kleiman <[email protected]>
Subject: Re: Issue with SlickGrid
Date: Thu, 27 Apr 2017 16:53:25 +0100
Message-ID: <CA+OCxoxq0SOTGXdCP_LnO3b_gQaXQM7WLjLkB3yJpMebvns0Gw@mail.gmail.com> (raw)
In-Reply-To: <CAE+jjamV-ofoRkA4z2Rr21+zujLbabsJUF3iqcWE_8EAuULkpA@mail.gmail.com>
References: <CAE+jjakiOhWWPTUxLMGGCuVhL=3pKuZjFfya5WVj0snegCZ-kQ@mail.gmail.com>
	<CAKKotZQNrt0v2Mp+1bPSu67GS6fvqo7DwaBjvw9KYK-hDzSbwQ@mail.gmail.com>
	<CAKKotZSwGWmb7zK5=s_MsvtCa3Zd-0KKHps61VpvDFKOrLJV0Q@mail.gmail.com>
	<CAE+jja=xFhROQ7krsZxq5Ziqt09jNAy2xqssNKaavBoJSD+nLQ@mail.gmail.com>
	<CAKKotZSOd3q7xhPPNVGDf-c0jfCW6jxAEBSUnQZMSw58N5=hiw@mail.gmail.com>
	<CAE+jjamV-ofoRkA4z2Rr21+zujLbabsJUF3iqcWE_8EAuULkpA@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

Thanks Joao. Murtuza, can you review this please?

On Thu, Apr 27, 2017 at 4:49 PM, Joao Pedro De Almeida Pereira <
[email protected]> 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 <
>> [email protected]> 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 <
>>> [email protected]> 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 <
>>>> [email protected]> 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 <
>>>>> [email protected]> 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 ([email protected])
> 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

view thread (9+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Issue with SlickGrid
  In-Reply-To: <CA+OCxoxq0SOTGXdCP_LnO3b_gQaXQM7WLjLkB3yJpMebvns0Gw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox