Обсуждение: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab isslow on tables with a lot of fields

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

Hi Aditya 

Can you please review it.

On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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-9517
Mobile: +91 976-788-8246

On Tue, Jan 29, 2019 at 10:39 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

On Tue, Jan 29, 2019 at 9:43 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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.

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.

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.linkedin.com/in/asheshvashi 

Regrads,

Murtuza

@ 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 <aditya.toshniwal@enterprisedb.com> 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,

Fixed 

3) In Create cast dialog switch control is smaller and so clipping text. 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 

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 <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya 

Can you please review it.

On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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-9517
Mobile: +91 976-788-8246

--

Thanks and Regards,

Aditya Toshniwal

Software Engineer | EnterpriseDB Software Solutions | Pune

"Don't Complain about Heat, Plant a tree"

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 <aditya.toshniwal@enterprisedb.com> 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,

Fixed 

3) In Create cast dialog switch control is smaller and so clipping text. 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 

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 <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya 

Can you please review it.

On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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-9517
Mobile: +91 976-788-8246

--

Thanks and Regards,

Aditya Toshniwal

Software Engineer | EnterpriseDB Software Solutions | Pune

"Don't Complain about Heat, Plant a tree"

Hi,

Please find the attached rebased patch.

Thanks,

Khushboo

On Tue, Jan 29, 2019 at 9:42 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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 <aditya.toshniwal@enterprisedb.com> 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,

Fixed 

3) In Create cast dialog switch control is smaller and so clipping text. 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 

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 <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya 

Can you please review it.

On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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-9517
Mobile: +91 976-788-8246

--

Thanks and Regards,

Aditya Toshniwal

Software Engineer | EnterpriseDB Software Solutions | Pune

"Don't Complain about Heat, Plant a tree"

Hi,

Please find attached rebased patch.

Thanks,

Khushboo

On Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find the attached rebased patch.

Thanks,

Khushboo

On Tue, Jan 29, 2019 at 9:42 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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 <aditya.toshniwal@enterprisedb.com> 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,

Fixed 

3) In Create cast dialog switch control is smaller and so clipping text. 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 

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 <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya 

Can you please review it.

On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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-9517
Mobile: +91 976-788-8246

--

Thanks and Regards,

Aditya Toshniwal

Software Engineer | EnterpriseDB Software Solutions | Pune

"Don't Complain about Heat, Plant a tree"

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 <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find attached rebased patch.

Thanks,

Khushboo

On Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find the attached rebased patch.

Thanks,

Khushboo

On Tue, Jan 29, 2019 at 9:42 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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 <aditya.toshniwal@enterprisedb.com> 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,

Fixed 

3) In Create cast dialog switch control is smaller and so clipping text. 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 

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 <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya 

Can you please review it.

On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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-9517
Mobile: +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-9517
Mobile: +91 976-788-8246

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 <akshay.joshi@enterprisedb.com> 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 <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find attached rebased patch.

Thanks,

Khushboo

On Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find the attached rebased patch.

Thanks,

Khushboo

On Tue, Jan 29, 2019 at 9:42 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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 <aditya.toshniwal@enterprisedb.com> 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,

Fixed 

3) In Create cast dialog switch control is smaller and so clipping text. 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 

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 <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya 

Can you please review it.

On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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-9517
Mobile: +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-9517
Mobile: +91 976-788-8246

--

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

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

On Thu, Jan 31, 2019 at 5:12 PM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Thu, Jan 31, 2019 at 5:11 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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 <dpage@pgadmin.org> 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 <akshay.joshi@enterprisedb.com> 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 <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find attached rebased patch.

Thanks,

Khushboo

On Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find the attached rebased patch.

Thanks,

Khushboo

On Tue, Jan 29, 2019 at 9:42 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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 <aditya.toshniwal@enterprisedb.com> 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,

Fixed 

3) In Create cast dialog switch control is smaller and so clipping text. 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 

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 <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya 

Can you please review it.

On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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-9517
Mobile: +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-9517
Mobile: +91 976-788-8246

--

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

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

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 <khushboo.vashi@enterprisedb.com> wrote:

On Thu, Jan 31, 2019 at 5:12 PM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Thu, Jan 31, 2019 at 5:11 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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 <dpage@pgadmin.org> 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 <akshay.joshi@enterprisedb.com> 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 <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find attached rebased patch.

Thanks,

Khushboo

On Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find the attached rebased patch.

Thanks,

Khushboo

On Tue, Jan 29, 2019 at 9:42 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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 <aditya.toshniwal@enterprisedb.com> 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,

Fixed 

3) In Create cast dialog switch control is smaller and so clipping text. 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 

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 <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya 

Can you please review it.

On Mon, Jan 14, 2019 at 4:28 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> 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-9517
Mobile: +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-9517
Mobile: +91 976-788-8246

--

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

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