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

Hi,

PFA updated patch for SlickGrid.

*Note:* This patch contains our own custom Slickgrid changes in SlickGrid
no need to apply patch sent in last email.

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

On Wed, Aug 24, 2016 at 6:27 PM, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com
> wrote:

> Hi Murtuza
>
> I have tested the functionality before reviewing the actual code, below
> are my review comments
>
> *Functionality related comments*:
>
> - Filter "By Selection" and "Exclude Selection" not working.
>
> Done

>
> - Increase the font size from 8pt to 9pt and also make the table
> header's in bold.
>
> Done

>
> - User won't be able to enter/copy the data directly from the cell.
> With current implementation user has to double click on cell then one data
> editor popped up where user can enter/copy data and will have to press
> "save" or "cancel" button. Then will have to double click again on the cell
> where he/she want's to paste and will have to press "save" or "cancel"
> button.
>
> Copying from selected Cell is done. But for now you have edit target cell
& paste the data in editor.
I will create TODO ticket for this pasting data without opening editor in
to cell as it requires some time.

>
> - Slick grid not rendered properly when changing tabs or resize "Data
> output" docker panel. Please refer "Error-1.png"
> - *Steps to reproduce*: View table data with more than 100 rows,
> scroll down a bit, now either shift tab and back again to "Data output"
> panel OR resize the "Data output" docker panel.
>
> Done

>
> - Slick grid is not refreshing properly when deleted multiple rows.
>
> Done

>
> - Error Message is not cleared from "Messages" tab even after
> successful transaction.
> - *Steps to reproduce*: Generate error by saving some wrong data
> and check "Messages" tab, now correct the value and save the data again.
> Data gets saved, but "Messages" tab still show error message.
>
> Done

>
> - Paste rows button gets disabled once it is clicked, as per me it
> should be enabled if some rows are copied to clipboard.
>
> Done

>
> - Facing weird issue for some tables, data is not being saved even if
> I have given the correct values. Mostly it happens with NOT NULL columns.
> - *Steps to reproduce*: View Data of the table where there are
> columns with NOT NULL constraint. I have tested it for "public.jobhist"
> table of PPAS-9.5. Add new row and insert values for primary key columns,
> but don't insert any value for NOT NULL column. Now click on Save button it
> will throw an error "null value in column <xyz> violates not-null
> constraint". Now insert the proper value for that column and try to save
> it, still it will give the same error.
>
> Done

>
> - Extra space has been added for the drop down menu of query tool
> toolbar. Please refer "Before_Slickgrid.png" and "After_Slickgrid.png"
>
> Done

>
> On Tue, Aug 23, 2016 at 4:00 PM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA minor add-on patch on previous Slickgrid v2 patch to remove console
>> log messages from JS..
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Tue, Aug 23, 2016 at 3:47 PM, Murtuza Zabuawala <
>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>
>>> 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,
>>>>>
>>>>> 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
>>>>
>>>
>>>
>>
>>
>> --
>> 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
>>
>>
>
>
> --
> *Akshay Joshi*
> *Principal Software Engineer *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>