Re: [pgAdmin4][Patch]: Adding connection status in Query tool

On Thu, Jan 11, 2018 at 7:00 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

On Thu, Jan 11, 2018 at 12:06 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

User can open Query tool in new browser window where we'll not have wcDocker panel.

In that case we can use window onfocus and onblur events.

In sqleditor there are cases where we've written conditional code based on whether sqleditor is opened in new window or not.

This is a great suggestion (the idea in general). We need to make this happen - it'll go a long way to minimising our concerns about the amount of polling.

 

On Thu, Jan 11, 2018 at 12:00 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Murtuza, I think we should only poll if sqleditor/datagrid is visible.

We've wcDocker.EVENT.VISIBILITY_CHANGED event when panel visibility changes.

-- 

Harshal Dhumal

Sr. Software Engineer

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

On Wed, Jan 10, 2018 at 1:55 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

PFA updated patch.

On Tue, Jan 9, 2018 at 7:57 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Jan 9, 2018 at 6:33 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

Please find updated patch.

I turned off the status option, but polling is still happening. This should definitely stop! :-)

​Fixed typo in variable.

Can you also reverse the enable/disable switch and the interval setting on the preferences page? I think Enable/Disable should be at the top, and be followed by the interval.

​

 Fixed.

​

But just a heads up we won't be able to put '?' after 'Connection status'​  eg: '

Connection status

​?'​

​

​Then string sorting again puts it after 'Connection status refresh rate' ​:)

​

Thanks.

 

On Mon, Jan 8, 2018 at 7:21 PM, Dave Page <dpage@pgadmin.org> wrote:

On Mon, Jan 8, 2018 at 1:24 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

PFA updated patch.

On Mon, Jan 8, 2018 at 5:11 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Fri, Jan 5, 2018 at 8:49 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

PFA updated patch,

On Wed, Jan 3, 2018 at 10:44 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thu, Dec 28, 2017 at 9:38 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi,

PFA updated patch based on new design suggested by Chethana.

The patch also includes some misc fixes related to object validation.

RM#2475

This seems much nicer, but I still think there are some tweaks to make:

1) If I open a query tool, and then stop the application server, the icon is updated to show the broken connection. However, unless I execute a query in the query tool before the server is shut down, the connection status won't recover when the server is restarted. If I do run a query first (SELECT 1; will do), then the connection status will recover.

I have logged​ 

​

https://redmine.postgresql.org/issues/2983

2) I think the "connected" icon should be in $primary-blue (#2c76b4). The green is ugly and not overly easy to read. It's also distracting as it catches the eye, which the default, non-error state should not do.

​Fixed​

 

Much better.

 

3) I'm not overly happy with the the status text popover. After some thought, I think it's because there are no visual clues that you should click on the icon to see it - and Karen seems to be of a similar opinion. Can we put a small marker there, perhaps a triangle on the bottom-right, like you get on a spreadsheet cell if you add a comment/note? We should also have a hotkey and I guess a tooltip, e.g. "Connection status Ctrl+Alt+S" or similar.

​Fixed​

, added accesskey 'T' for TX status tooltip shortcut as 'S' is already taken for Save file option

Hmm - having seen it, I don't think the marker helps us.

Can you remove it, and fix the tooltip (which doesn't seem to work)? If we always have the tooltip say "Connection status Ctrl+Alt+T" (or whatever is appropriate for the platform/browser), then that should give the user enough hint to click.

​

Fixed​.

I have removed the marker & added tooltip instead but it is not possible to add specific shortcut keys in tooltip because accesskey may vary depending on OS & browser.

​Ref: ​

https://www.w3schools.com/tags/att_global_accesskey.asp

That's better - though I think the tool tip is better as something like:

Connection status (click for details) (<accesskey>+T) 

I'm still not overly happy with all the polling that's going on though. It's a lot of requests, especially with multiple QTs open. I think we need to be able to disable the feature entirely through a switch in the Preferences. In that case, no icon would be shown, and polling would be disabled - i.e. everything would be as it is now.

What do you think?

​Fixed

Made it configurable & set default polling time to 10sec.​

 

​Please review.​

--

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

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

--

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

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

--

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

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