public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ashesh Vashi <[email protected]>
To: Khushboo 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:12:13 +0530
Message-ID: <CAG7mmowuW7oP1w=VFLLRdp+mJ-GF1WLVac57Rqn96LB7seU=kw@mail.gmail.com> (raw)
In-Reply-To: <CAFOhELd=LneQKhExpOzi1yu1Kpcc6Zpkz0eR=K4Cn_BF1kKH_Q@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>
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.
-- 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: <CAG7mmowuW7oP1w=VFLLRdp+mJ-GF1WLVac57Rqn96LB7seU=kw@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