Thanks, patch applied. On Mon, Aug 29, 2016 at 3:30 PM, Murtuza Zabuawala < murtuza.zabuaw...@enterprisedb.com> wrote:
> 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.pa...@enterprisedb.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.zabuaw...@enterprisedb.com> wrote: >> >>> >>> -- >>> Regards, >>> Murtuza Zabuawala >>> EnterpriseDB: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >>> ---------- Forwarded message ---------- >>> From: Murtuza Zabuawala <murtuza.zabuaw...@enterprisedb.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.jo...@enterprisedb.com> >>> Cc: Dave Page <dp...@pgadmin.org>, pgadmin-hackers < >>> pgadmin-hackers@postgresql.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.jo...@enterprisedb.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.zabuaw...@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.zabuaw...@enterprisedb.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 <dp...@pgadmin.org> wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> On Wed, Aug 17, 2016 at 11:19 AM, Murtuza Zabuawala < >>>>>>> murtuza.zabuaw...@enterprisedb.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@postgresql.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@postgresql.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* >>>> >>> >>> >>> >> > -- *Akshay Joshi* *Principal Software Engineer * *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
