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

Поиск
Список
Период
Сортировка
От Khushboo Vashi
Тема Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields
Дата
Msg-id CAFOhELc9410UoQak5AWdPahwGLM2e2H0B=oA87mRRk3pRZJcgQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields  (Khushboo Vashi <khushboo.vashi@enterprisedb.com>)
Ответы Re: [pgAdmin4][Patch]: RM - 3051 - ables > Properties > Columns tab is slow on tables with a lot of fields  (Akshay Joshi <akshay.joshi@enterprisedb.com>)
Список pgadmin-hackers
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,
Screenshot 2019-01-22 at 11.04.36 AM.png
Fixed 
3) In Create cast dialog switch control is smaller and so clipping text. Below is screenshot,
Screenshot 2019-01-22 at 11.07.14 AM.png
Fixed 
4) You've removed unnecessary switch control template codes at most places. I would suggest doing the same for Backform.CustomSwitchControl in trigger.js
Done 
5) Feature tests are still using bootstrap-switch classes and so failing.
Fixed 

Apart from above, everything looks good to me.


On Mon, Jan 21, 2019 at 4:42 PM Akshay Joshi <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
Вложения

В списке pgadmin-hackers по дате отправления:

Предыдущее
От: Aditya Toshniwal
Дата:
Сообщение: [pgAdmin4][RM3941] Dashboard graphs needs optimizations
Следующее
От: Akshay Joshi
Дата:
Сообщение: Re: [pgAdmin4][RM3941] Dashboard graphs needs optimizations