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-01-31 11:43:01
Message-ID: CAFOhELc2_KrQ7JaWmj4RkNeCnc13dtJ-e9pua8f9ihjP+DTVsA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>>>
>>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2019-01-31 12:19:49 pgAdmin 4 commit: Remove hacky way to open/close the dropdown submenu
Previous Message Ashesh Vashi 2019-01-31 11:42:13 Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields