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

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, Dave Page <dpage(at)pgadmin(dot)org>, 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 06:03:16
Message-ID: CANxoLDcqUtntoPwLGOsmv8p1=YNCNN4m9VJsmAsn8otL63FbRg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks patch applied.

On Mon, Feb 4, 2019 at 10:18 AM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

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

--
*Akshay Joshi*

*Sr. Software Architect *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Aditya Toshniwal 2019-02-04 08:48:37 Re: [pgAdmin4][RM3936] Feature tests failing intermittently for SQL editor related test cases
Previous Message Akshay Joshi 2019-02-04 06:02:55 pgAdmin 4 commit: Replace Bootstrap switch with Bootstrap4 toggle to im