public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dave Page <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: 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 11:01:56 +0000
Message-ID: <CA+OCxowV9D9kWoTU+pOV5y3j5n4y7A-2gUNNfiUQdnZt8uqSRQ@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDdmK=UKFhHQpC+Pw1knyeNCHf2VgZ5xnXRfeQ7izc=cTA@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>
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.
>
>
> 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]
Subject: Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
In-Reply-To: <CA+OCxowV9D9kWoTU+pOV5y3j5n4y7A-2gUNNfiUQdnZt8uqSRQ@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