Re: PATCH: SlickGrid integration in query tool (pgAdmin4)

Hi,

PFA updated patch & comments inline.

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

On Thu, Aug 18, 2016 at 9:16 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi,
>
> On Wed, Aug 17, 2016 at 11:19 AM, Murtuza Zabuawala <
> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> PFA initial patch for SlickGrid integration in query tool.
>>
>
> This is looking awesome! I think there are some tweaks to make, but
> performance-wise, it's blowing away the old code. My tests show it's even
> faster than you're seeing; I'm seeing ~3x performance rendering 21K rows
> for example, vs. the first 2K of the same dataset on page 1 with the old
> code - plus, the rendered HTML isn't so huge that it slows everything else
> down to being unusable!
>
> Can you look at the following issues please?
>
> 1) The colour for a selected row should be #eeeeee
>
Done

> 2) If there's a horizontal scroll bar (in Chrome on OSX, possibly others),
> then it partially eclipses the new blank row in View Data mode. The
> vertical scroll needs to have the height of the horizontal scrollbar added.
>
This issue needed small code hack in SlickGrid code itself. (Ashesh help me
with issue as all the row sizes were dynamic (in terms of pixels)
& calculated as we display rows on grid canvas, so we can not use CSS/JS to
tweak it)
I have attached separate patch for Slickgrid changes, we have to apply this
patch whenever we update SlickGrid version in future.

>
>
3) If the window is resized, the grid doesn't redraw to the new size.
>
Done

> 4) I cannot leave a Primary Key field blank so that it picks up a default
> value (I get "Primary key columns cannot be null.")
>
Done

> 5) When editing text fields with multiple lines, the CR/LFs are lost. Do
> we need a custom editor for this (textarea?).
>
Added custom detached editor [ which supports all keyboard navigation :-) ].

>
> 6) The tooltips should probably include <pre> tags around the content
> otherwise the formatting is messed up and becomes unreadable.
>
This is not permitted, Only text to be displayed is allowed, it will not
parse html tags in and show them as is.
Ref: http://www.w3schools.com/tags/att_global_title.asp

Thanks!
>
>
>>
>> My finding on grid rendering performance with each grid,
>> [image: Inline image 1]
>> Tested on PG9.5 (sql attached).
>>
>> Please review.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> --
>> 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
>