Subject: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields

From: Khushboo Vashi <[email-masked]>
Date: Mon, 14 Jan 2019 16:28:10 +0530

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

---

From: Akshay Joshi <[email-masked]>
Date: Mon, 21 Jan 2019 16:42:27 +0530

Hi Aditya

Can you please review it.

On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <
[email-masked]> wrote:

> Hi,
>
> Please find the attached patch to fix #3051 - Tables > Properties >
> Columns tab is slow on tables with a lot of fields
>

-- 
*Akshay Joshi*

*Sr. Software Architect *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

---

From: Aditya Toshniwal <[email-masked]>
Date: Tue, 22 Jan 2019 11:33:05 +0530

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?
2) In Login roles, Create trigger dialogs switch control colors are
different. Below is screenshot,
3) In Create cast dialog switch control is smaller and so clipping text.
Below is screenshot,
4) You've removed unnecessary switch control template codes at most places.
I would suggest doing the same for Backform.CustomSwitchControl in
trigger.js
5) Feature tests are still using bootstrap-switch classes and so failing.

Apart from above, everything looks good to me.

-- 
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB Software Solutions | Pune
"Don't Complain about Heat, Plant a tree"

---

From: Murtuza Zabuawala <[email-masked]>
Date: Tue, 22 Jan 2019 12:21:01 +0530

Apart from that, Due to API based approach taken in rendering toggle, so I
can still reproduce the issue raised in
https://redmine.postgresql.org/issues/3568 which is linked to RM#3051,
I have had updated the RM with my finding on the weekend, I think we need
to use HTML based approach for rendering the toggle when not in edit mode.

-- Murtuza

On Tue, Jan 22, 2019 at 11:33 AM Aditya Toshniwal <
[email-masked]> wrote:

> 4) You've removed unnecessary switch control template codes at most
> places. I would suggest doing the same for Backform.CustomSwitchControl in
> trigger.js
> On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <
> [email-masked]> wrote:
>
>> Hi Aditya
>>

---

From: Khushboo Vashi <[email-masked]>
Date: Tue, 29 Jan 2019 09:42:51 +0530

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

> does not remove editor class. I think we can skip adding editor, what
This issue was old, not due to my patch but I have fixed it.

> different. Below is screenshot,
Fixed

> 3) In Create cast dialog switch control is smaller and so clipping text.
Fixed

> places. I would suggest doing the same for Backform.CustomSwitchControl in
Done

> 5) Feature tests are still using bootstrap-switch classes and so failing.
Fixed

---

From: Murtuza Zabuawala <[email-masked]>
Date: Tue, 29 Jan 2019 10:39:23 +0530

> https://redmine.postgresql.org/issues/3664.

I agreed to that, that's why I wrote when not in edit mode : )

With API mode, we are performing DOM operation on each individual instance
(specially in Subnode/Backgrid) which I think we should avoid when we are
just displaying to the user.

Regrads,
Murtuza

>>>> The root cause of the issue is bootstrap switch, which has been
>>>> replaced with bootstrap4-toggle application wide.
>>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>
>>>
>>> --
>>> *Akshay Joshi*
>>>

---

From: Ashesh Vashi <[email-masked]>
Date: Tue, 29 Jan 2019 10:47:04 +0530

On Tue, Jan 29, 2019 at 10:39 AM Murtuza Zabuawala <
[email-masked]> wrote:

>> 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.
I agree - it was my concern with the earlier switch control as well.

I was thinking of using switch control based on complete CSS.
How about the following?
https://github.com/ghinda/css-toggle-switch
https://ghinda.net/css-toggle-switch/bootstrap.html

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
<http://www.enterprisedb.com/>

*http://www.linkedin.com/in/asheshvashi*
<http://www.linkedin.com/in/asheshvashi>

---

From: Khushboo Vashi <[email-masked]>
Date: Wed, 30 Jan 2019 15:17:08 +0530

Hi All,

As per our discussion, I have implemented the CSS Switch.  Reference:
https://codepen.io/personable/pen/stpwD?editors=1100#0
Please find the attached patch for the same.

Please check the performance for the Login/Group roles Properties tab.

Thanks,
Khushboo

On Tue, Jan 29, 2019 at 10:47 AM Ashesh Vashi <[email-masked]>
wrote:

>>> 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,
>> With API mode, we are performing DOM operation on each individual
>> instance (specially in Subnode/Backgrid) which I think we should avoid when
>> we are just displaying to the user.
>>
>>>> 3) In Create cast dialog switch control is smaller and so clipping
>>>> text. Below is screenshot,
>>>>
>>> Fixed
>>>

---

From: Ashesh Vashi <[email-masked]>
Date: Wed, 30 Jan 2019 15:22:22 +0530

> Please check the performance for the Login/Group roles Properties tab.
Do you see any performance benefit with the CSS approach?

-- Thanks, Ashesh

>
> Thanks,
> Khushboo
>
>
>
>
> On Tue, Jan 29, 2019 at 10:47 AM Ashesh Vashi <
> [email-masked]> wrote:
>

---

From: Khushboo Vashi <[email-masked]>
Date: Wed, 30 Jan 2019 15:25:38 +0530

On Wed, Jan 30, 2019 at 3:22 PM Ashesh Vashi <[email-masked]>
wrote:

> I have tested with 250 to 300 records and didn't realise much performance
benefit.
I think maximum people should test this, so we can have better idea.

>>>>>> 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?
>>>>>>

---

From: Khushboo Vashi <[email-masked]>
Date: Thu, 31 Jan 2019 14:37:27 +0530

Hi,

Please find the attached rebased patch.

Thanks,
Khushboo

---

From: Khushboo Vashi <[email-masked]>
Date: Thu, 31 Jan 2019 15:33:36 +0530

Hi,

Please find attached rebased patch.

Thanks,
Khushboo

---

From: Akshay Joshi <[email-masked]>
Date: Thu, 31 Jan 2019 16:20:32 +0530

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.

-- 
*Akshay Joshi*

---

From: Dave Page <[email-masked]>
Date: Thu, 31 Jan 2019 11:01:56 +0000

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-masked]>
wrote:

> 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.
>

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Khushboo Vashi <[email-masked]>
Date: Thu, 31 Jan 2019 17:10:56 +0530

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-masked]> wrote:

> On Thu, Jan 31, 2019 at 11:50 AM Akshay Joshi <

---

From: Ashesh Vashi <[email-masked]>
Date: Thu, 31 Jan 2019 17:12:13 +0530

On Thu, Jan 31, 2019 at 5:11 PM Khushboo Vashi <
[email-masked]> wrote:

>>>    - 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

>>>>>> In our case, Initialisation with HTML is not possible as we render
>>>>>> the backform controls runtime, So, I have used the other option.
>>>>>> Please, review this patch, we need your approval for the toggle
>>>>>> design changes.
>>>>>>
>>>>>> Thanks,
>>>>>> Khushboo
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>> Please find the attached patch to fix #3051 - Tables > Properties

---

From: Khushboo Vashi <[email-masked]>
Date: Thu, 31 Jan 2019 17:13:01 +0530

On Thu, Jan 31, 2019 at 5:12 PM Ashesh Vashi <[email-masked]>
wrote:

>>>> Login Roles. Basic problem is with rendering of controls in Backgrid
>>>> it self, that took a lot of time.
>>>>
Sure.

---

From: Khushboo Vashi <[email-masked]>
Date: Mon, 4 Feb 2019 10:18:28 +0530

Hi,

Please find the attached updated patch which also includes the keyboard
accessibility for the Switch control and cell.
This patch does not include the fix for the slow rendering of the
properties and dependents tabs, I will send that patch separately.

Thanks,
Khushboo

On Thu, Jan 31, 2019 at 5:13 PM Khushboo Vashi <
[email-masked]> wrote:

>
>
> On Thu, Jan 31, 2019 at 5:12 PM Ashesh Vashi <
>>> Also, I will try to apply the same logic used in statistics tab to
>>> render the properties tab faster.
>>>
>>>> I think Bootstrap toggle is a nicer and clearer design. Not showing
>>>> both value labels makes it much clearer what the current setting is.
>>>>
>>>>> I have applied both(Bootstrap-Toggle and CSS Approach) the patches
>>>>> and performance wise no major difference found. I have tested it for 1000+
>>>>>>>> Also, the main issue of slow rendering which has been solved
>>>>>>>> through this implementation. The browser hanging issue is due to Backbone
>>>>>>>>> 2) In Login roles, Create trigger dialogs switch control colors
>>>>>>>>> are different. Below is screenshot,
>>>>>>>>>
>>>>>>>> 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
>>>>>>>>
>>>>>>>>>>> Please find the attached patch to fix #3051 - Tables >
>>>>>>>>>>> Properties > Columns tab is slow on tables with a lot of fields
>>>>>>>>>>>

---

From: Akshay Joshi <[email-masked]>
Date: Mon, 4 Feb 2019 11:33:16 +0530

Thanks patch applied.

On Mon, Feb 4, 2019 at 10:18 AM Khushboo Vashi <
[email-masked]> wrote:

>>>>>>    - In CSS switch both the options (Yes/No) is visible. Please
>>>>>>    refer attached screenshot.
>>>>>>    - When we scroll down the Roles in the properties tab rendering
>>>>>>    of CSS switch is very slow compare to the Bootstrap Toggle.
>>>>>>
>>>>>>>>>> 3) In Create cast dialog switch control is smaller and so
>>>>>>>>>> clipping text. Below is screenshot,
>>>>>>>>>>
>>>>>>>>> Fixed
>>>>>>>>>

-- 
*Akshay Joshi*