From: | Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com> |
---|---|
To: | Khushboo Vashi <khushboo(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:42:13 |
Message-ID: | CAG7mmowuW7oP1w=VFLLRdp+mJ-GF1WLVac57Rqn96LB7seU=kw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
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.
-- 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
>>
>
From | Date | Subject | |
---|---|---|---|
Next Message | Khushboo Vashi | 2019-01-31 11:43:01 | Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields |
Previous 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 |