Thread: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab isslow on tables with a lot of fields
[pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab isslow on tables with a lot of fields
From
Khushboo Vashi
Date:
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
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tabis slow on tables with a lot of fields
From
Akshay Joshi
Date:
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 fieldsThe 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
Mobile: +91 976-788-8246
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Aditya Toshniwal
Date:
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.
On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +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"
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Murtuza Zabuawala
Date:
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 <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?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.js5) Feature tests are still using bootstrap-switch classes and so failing.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 AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Khushboo Vashi
Date:
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 AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Murtuza Zabuawala
Date:
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.
Regrads,
Murtuza
@ Dave,Please, review this patch, we need your approval for the toggle design changes.Thanks,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Ashesh Vashi
Date:
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?
--
Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
Regrads,Murtuza@ Dave,Please, review this patch, we need your approval for the toggle design changes.Thanks,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Khushboo Vashi
Date:
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 <ashesh.vashi@enterprisedb.com> wrote:
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?
--Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
Regrads,Murtuza@ Dave,Please, review this patch, we need your approval for the toggle design changes.Thanks,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Ashesh Vashi
Date:
On Wed, Jan 30, 2019 at 3:17 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi All,As per our discussion, I have implemented the CSS Switch. Reference: https://codepen.io/personable/pen/stpwD?editors=1100#0Please find the attached patch for the same.Please check the performance for the Login/Group roles Properties tab.
Do you see any performance benefit with the CSS approach?
-- Thanks, Ashesh
Thanks,KhushbooOn Tue, Jan 29, 2019 at 10:47 AM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote: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?
--Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
Regrads,Murtuza@ Dave,Please, review this patch, we need your approval for the toggle design changes.Thanks,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Khushboo Vashi
Date:
On Wed, Jan 30, 2019 at 3:22 PM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Wed, Jan 30, 2019 at 3:17 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi All,As per our discussion, I have implemented the CSS Switch. Reference: https://codepen.io/personable/pen/stpwD?editors=1100#0Please find the attached patch for the same.Please check the performance for the Login/Group roles Properties tab.Do you see any performance benefit with the CSS approach?
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.
-- Thanks, AsheshThanks,KhushbooOn Tue, Jan 29, 2019 at 10:47 AM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote: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?
--Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
Regrads,Murtuza@ Dave,Please, review this patch, we need your approval for the toggle design changes.Thanks,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Khushboo Vashi
Date:
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,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Khushboo Vashi
Date:
Hi,
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,KhushbooOn 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,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Akshay Joshi
Date:
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,KhushbooOn Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find the attached rebased patch.Thanks,KhushbooOn 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,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware 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
Mobile: +91 976-788-8246
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Dave Page
Date:
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 KhushbooI 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,KhushbooOn Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find the attached rebased patch.Thanks,KhushbooOn 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,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"--Akshay JoshiSr. Software ArchitectPhone: +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
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Khushboo Vashi
Date:
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 KhushbooI 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,KhushbooOn Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find the attached rebased patch.Thanks,KhushbooOn 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,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"--Akshay JoshiSr. Software ArchitectPhone: +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
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Ashesh Vashi
Date:
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 KhushbooI 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 <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find attached rebased patch.Thanks,KhushbooOn Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find the attached rebased patch.Thanks,KhushbooOn 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,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"--Akshay JoshiSr. Software ArchitectPhone: +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
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Khushboo Vashi
Date:
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 KhushbooI 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, AsheshOn Thu, Jan 31, 2019 at 3:33 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find attached rebased patch.Thanks,KhushbooOn Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find the attached rebased patch.Thanks,KhushbooOn 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,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"--Akshay JoshiSr. Software ArchitectPhone: +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
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Khushboo Vashi
Date:
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 KhushbooI 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, AsheshOn Thu, Jan 31, 2019 at 3:33 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find attached rebased patch.Thanks,KhushbooOn Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find the attached rebased patch.Thanks,KhushbooOn 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,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"--Akshay JoshiSr. Software ArchitectPhone: +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
Attachment
Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
From
Akshay Joshi
Date:
Thanks patch applied.
On Mon, Feb 4, 2019 at 10:18 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
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,KhushbooOn 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 KhushbooI 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, AsheshOn Thu, Jan 31, 2019 at 3:33 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find attached rebased patch.Thanks,KhushbooOn Thu, Jan 31, 2019 at 2:37 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi,Please find the attached rebased patch.Thanks,KhushbooOn 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,KhushbooOn 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,Fixed3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,Fixed4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.jsDone5) Feature tests are still using bootstrap-switch classes and so failing.FixedApart from above, everything looks good to me.On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi AdityaCan 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 fieldsThe root cause of the issue is bootstrap switch, which has been replaced with bootstrap4-toggle application wide.Thanks,Khushboo--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB Software Solutions | Pune"Don't Complain about Heat, Plant a tree"--Akshay JoshiSr. Software ArchitectPhone: +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
Akshay Joshi
Sr. Software Architect

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