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

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: Khushboo Vashi <khushboo(dot)vashi(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:01:56
Message-ID: CA+OCxowV9D9kWoTU+pOV5y3j5n4y7A-2gUNNfiUQdnZt8uqSRQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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 Khushboo Vashi 2019-01-31 11:40:56 Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
Previous Message Akshay Joshi 2019-01-31 10:50:32 Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields