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

Hi,

PFA updated patch.

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

On Mon, Aug 29, 2016 at 1:18 PM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
wrote:

> Hi Murtuza,
>
> All the review comments given by Akshay are fixed. Below are some minor
> observations.
>
>
> - Add two new rows with valid data. Select one row and delete that
> row. Now "Save" button should be enabled because we still have one valid
> new row to save the data.
>
> Done

>
> - When we click on new blank row and select "By Selection" then it
> throws 'javascript' exception.
>
> Uncaught TypeError: Cannot read property 'id' of undefined
>
Done

>
> Thanks,
> Neel patel
>
> On Mon, Aug 29, 2016 at 10:43 AM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> ---------- Forwarded message ----------
>> From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
>> Date: Fri, Aug 26, 2016 at 8:27 PM
>> Subject: Re: [pgadmin-hackers] PATCH: SlickGrid integration in query tool
>> (pgAdmin4)
>> To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
>> Cc: Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers <
>> pgadmin-hackers(at)postgresql(dot)org>
>>
>>
>> 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(dot)zabuawala(at)enterprisedb(dot)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)or
>>>>>>> g)
>>>>>>> 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*
>>>
>>
>>
>>
>