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: Dave Page <dpage(at)pgadmin(dot)org>
Cc: 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:40:56
Message-ID: CAFOhELd=LneQKhExpOzi1yu1Kpcc6Zpkz0eR=K4Cn_BF1kKH_Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.
>>
>>
>> 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 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
Previous Message Dave Page 2019-01-31 11:01:56 Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields