public inbox for [email protected]
help / color / mirror / Atom feedFrom: Akshay Joshi <[email protected]>
To: Khushboo Vashi <[email protected]>
Cc: Aditya Toshniwal <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
Date: Thu, 31 Jan 2019 16:20:32 +0530
Message-ID: <CANxoLDdmK=UKFhHQpC+Pw1knyeNCHf2VgZ5xnXRfeQ7izc=cTA@mail.gmail.com> (raw)
In-Reply-To: <CAFOhELdu5iEJ0uWfd8n8ycw7RjnggBA9uhrQSsz4+9C9h1yhog@mail.gmail.com>
References: <CAFOhELfosrL2Y_2_gwHyzef5JeSSzNSyicSNj6uv1B1286yKzg@mail.gmail.com>
<CANxoLDc1q3KmpmO=SZw69J9on-TUUBVj-QoN7aYAtgYjP3HBsQ@mail.gmail.com>
<CAM9w-_=RLYvwz4HJ5YrDPSdX80B7kLvt0Ti41F6r0_deAt-wHg@mail.gmail.com>
<CAFOhELe+UJqC-4vyBe2yk_Mz27DKfJFFyU1UbURZ9ycrQs9SYA@mail.gmail.com>
<CAFOhELfkdK5X_c1bOCsEHHGFEKYNjjjO4Aus-EH7h7zLSSYmvg@mail.gmail.com>
<CAFOhELdu5iEJ0uWfd8n8ycw7RjnggBA9uhrQSsz4+9C9h1yhog@mail.gmail.com>
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 <
[email protected]> wrote:
> Hi,
>
> Please find attached rebased patch.
>
> Thanks,
> Khushboo
>
> On Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <
> [email protected]> wrote:
>
>> Hi,
>>
>> Please find the attached rebased patch.
>>
>> Thanks,
>> Khushboo
>>
>> On Tue, Jan 29, 2019 at 9:42 AM Khushboo Vashi <
>> [email protected]> 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 <
>>> [email protected]> 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 <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Aditya
>>>>>
>>>>> Can you please review it.
>>>>>
>>>>> On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <
>>>>> [email protected]> 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*
Attachments:
[image/png] Screenshot 2019-01-22 at 11.04.36 AM.png (107.2K, 3-Screenshot%202019-01-22%20at%2011.04.36%20AM.png)
download | view image
[image/png] Screenshot 2019-01-22 at 11.07.14 AM.png (49.9K, 4-Screenshot%202019-01-22%20at%2011.07.14%20AM.png)
download | view image
[image/png] Bootstrap_Toggle_Small.png (289.3K, 5-Bootstrap_Toggle_Small.png)
download | view image
[image/png] Switch_Using _CSS.png (281.7K, 6-Switch_Using%20_CSS.png)
download | view image
view thread (19+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
In-Reply-To: <CANxoLDdmK=UKFhHQpC+Pw1knyeNCHf2VgZ5xnXRfeQ7izc=cTA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox