public inbox for [email protected]
help / color / mirror / Atom feedFrom: Khushboo Vashi <[email protected]>
To: Ashesh Vashi <[email protected]>
Cc: Dave Page <[email protected]>
Cc: Akshay Joshi <[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 17:13:01 +0530
Message-ID: <CAFOhELc2_KrQ7JaWmj4RkNeCnc13dtJ-e9pua8f9ihjP+DTVsA@mail.gmail.com> (raw)
In-Reply-To: <CAG7mmowuW7oP1w=VFLLRdp+mJ-GF1WLVac57Rqn96LB7seU=kw@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>
<CANxoLDdmK=UKFhHQpC+Pw1knyeNCHf2VgZ5xnXRfeQ7izc=cTA@mail.gmail.com>
<CA+OCxowV9D9kWoTU+pOV5y3j5n4y7A-2gUNNfiUQdnZt8uqSRQ@mail.gmail.com>
<CAFOhELd=LneQKhExpOzi1yu1Kpcc6Zpkz0eR=K4Cn_BF1kKH_Q@mail.gmail.com>
<CAG7mmowuW7oP1w=VFLLRdp+mJ-GF1WLVac57Rqn96LB7seU=kw@mail.gmail.com>
On Thu, Jan 31, 2019 at 5:12 PM Ashesh Vashi <[email protected]>
wrote:
> On Thu, Jan 31, 2019 at 5:11 PM Khushboo Vashi <
> [email protected]> 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 <[email protected]> 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 <
>>> [email protected]> 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 <
>>>> [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*
>>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
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
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], [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: <CAFOhELc2_KrQ7JaWmj4RkNeCnc13dtJ-e9pua8f9ihjP+DTVsA@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