Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields

From: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
Date: 2019-02-04 04:48:28
Message-ID: CAFOhELc9410UoQak5AWdPahwGLM2e2H0B=oA87mRRk3pRZJcgQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

Please find the attached updated patch which also includes the keyboard
accessibility for the Switch control and cell.
This patch does not include the fix for the slow rendering of the
properties and dependents tabs, I will send that patch separately.

Thanks,
Khushboo

On Thu, Jan 31, 2019 at 5:13 PM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

>
>
> On Thu, Jan 31, 2019 at 5:12 PM Ashesh Vashi <
> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> On Thu, Jan 31, 2019 at 5:11 PM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Thanks Dave and Akshay for the review.
>>> Should I go ahead with the Bootstrap toggle and work on the changes
>>> proposed in the review meeting?
>>>
>>> Also, I will try to apply the same logic used in statistics tab to
>>> render the properties tab faster.
>>>
>>> On Thu, Jan 31, 2019 at 4:32 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> I think Bootstrap toggle is a nicer and clearer design. Not showing
>>>> both value labels makes it much clearer what the current setting is.
>>>>
>>>> On Thu, Jan 31, 2019 at 11:50 AM Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Khushboo
>>>>>
>>>>> I have applied both(Bootstrap-Toggle and CSS Approach) the patches
>>>>> and performance wise no major difference found. I have tested it for 1000+
>>>>> Login Roles. Basic problem is with rendering of controls in Backgrid
>>>>> it self, that took a lot of time.
>>>>>
>>>>> *User experience differences:*
>>>>>
>>>>> - Look & Feel wise I would prefer Bootstrap Toggle.
>>>>> - In CSS switch both the options (Yes/No) is visible. Please refer
>>>>> attached screenshot.
>>>>> - CSS switch toggles even if we click outside the control, but
>>>>> within the same row.
>>>>> - When we scroll down the Roles in the properties tab rendering of
>>>>> CSS switch is very slow compare to the Bootstrap Toggle.
>>>>>
>>>>> Properties, and dependents as well.
>>
> Sure.
>
>>
>> -- Thanks, Ashesh
>>
>>>
>>>>> On Thu, Jan 31, 2019 at 3:33 PM Khushboo Vashi <
>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Please find attached rebased patch.
>>>>>>
>>>>>> Thanks,
>>>>>> Khushboo
>>>>>>
>>>>>> On Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <
>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Please find the attached rebased patch.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Khushboo
>>>>>>>
>>>>>>> On Tue, Jan 29, 2019 at 9:42 AM Khushboo Vashi <
>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Aditya,
>>>>>>>>
>>>>>>>> Thanks for the review.
>>>>>>>>
>>>>>>>> Please find the attached updated patch.
>>>>>>>>
>>>>>>>> @ Murtuza,
>>>>>>>>
>>>>>>>> Regarding your concern, I have not used the API. As per the
>>>>>>>> documentation, there are 2 ways to initialise the bootstrap toggle, First
>>>>>>>> Initialise with HTML and second with Code.
>>>>>>>> In our case, Initialisation with HTML is not possible as we render
>>>>>>>> the backform controls runtime, So, I have used the other option.
>>>>>>>> Also, the main issue of slow rendering which has been solved
>>>>>>>> through this implementation. The browser hanging issue is due to Backbone
>>>>>>>> collection reset method and I am working on that part with another RM,
>>>>>>>> https://redmine.postgresql.org/issues/3664.
>>>>>>>>
>>>>>>>> @ Dave,
>>>>>>>>
>>>>>>>> Please, review this patch, we need your approval for the toggle
>>>>>>>> design changes.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Khushboo
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Jan 22, 2019 at 11:33 AM Aditya Toshniwal <
>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi Khushboo,
>>>>>>>>>
>>>>>>>>> I have few suggestions/review:
>>>>>>>>> 1) Do we need to add "editor" class to switch control in backgrid
>>>>>>>>> when changing. For eg. in tables->columns if I change not null switch, it
>>>>>>>>> adds editor class which makes hover background white. Plus, leaving the
>>>>>>>>> switch does not remove editor class. I think we can skip adding editor,
>>>>>>>>> what do you think?
>>>>>>>>>
>>>>>>>> This issue was old, not due to my patch but I have fixed it.
>>>>>>>>
>>>>>>>>> 2) In Login roles, Create trigger dialogs switch control colors
>>>>>>>>> are different. Below is screenshot,
>>>>>>>>> [image: Screenshot 2019-01-22 at 11.04.36 AM.png]
>>>>>>>>>
>>>>>>>> Fixed
>>>>>>>>
>>>>>>>>> 3) In Create cast dialog switch control is smaller and so clipping
>>>>>>>>> text. Below is screenshot,
>>>>>>>>> [image: Screenshot 2019-01-22 at 11.07.14 AM.png]
>>>>>>>>>
>>>>>>>> Fixed
>>>>>>>>
>>>>>>>>> 4) You've removed unnecessary switch control template codes at
>>>>>>>>> most places. I would suggest doing the same for
>>>>>>>>> Backform.CustomSwitchControl in trigger.js
>>>>>>>>>
>>>>>>>> Done
>>>>>>>>
>>>>>>>>> 5) Feature tests are still using bootstrap-switch classes and so
>>>>>>>>> failing.
>>>>>>>>>
>>>>>>>> Fixed
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Apart from above, everything looks good to me.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <
>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Aditya
>>>>>>>>>>
>>>>>>>>>> Can you please review it.
>>>>>>>>>>
>>>>>>>>>> On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <
>>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Please find the attached patch to fix #3051 - Tables >
>>>>>>>>>>> Properties > Columns tab is slow on tables with a lot of fields
>>>>>>>>>>>
>>>>>>>>>>> The root cause of the issue is bootstrap switch, which has been
>>>>>>>>>>> replaced with bootstrap4-toggle application wide.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Khushboo
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> *Akshay Joshi*
>>>>>>>>>>
>>>>>>>>>> *Sr. Software Architect *
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Thanks and Regards,
>>>>>>>>> Aditya Toshniwal
>>>>>>>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>>>>>>>> "Don't Complain about Heat, Plant a tree"
>>>>>>>>>
>>>>>>>>
>>>>>
>>>>> --
>>>>> *Akshay Joshi*
>>>>>
>>>>> *Sr. Software Architect *
>>>>>
>>>>>
>>>>>
>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>

Attachment Content-Type Size
RM_3051_v2.patch application/octet-stream 454.6 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2019-02-04 05:15:36 Re: [pgAdmin4][RM3941] Dashboard graphs needs optimizations
Previous Message Aditya Toshniwal 2019-02-01 13:33:47 [pgAdmin4][RM3941] Dashboard graphs needs optimizations