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

Sure, I'll do that.

On Fri, Jan 12, 2018 at 8:05 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Thanks - patch applied!
>
> Can you please update the docs for the new config options, and any
> screenshot updates that are required?
>
> On Fri, Jan 12, 2018 at 2:10 PM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> ​Hi Dave,
>>
>> PFA updated patch with additional checks to prevent unnecessary ​polling
>> added as suggested.
>> Please review.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Thu, Jan 11, 2018 at 2:33 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Thu, Jan 11, 2018 at 7:00 AM, Harshal Dhumal <
>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>
>>>> On Thu, Jan 11, 2018 at 12:06 PM, Murtuza Zabuawala <
>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)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
>>>> <http://www.thefutureoftheweb.com/blog/detect-browser-window-focus>.
>>>> 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(dot)dhumal(at)enterprisedb(dot)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(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> PFA updated patch.
>>>>>>>
>>>>>>> On Tue, Jan 9, 2018 at 7:57 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Tue, Jan 9, 2018 at 6:33 AM, Murtuza Zabuawala <
>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)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(at)pgadmin(dot)org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Mon, Jan 8, 2018 at 1:24 PM, Murtuza Zabuawala <
>>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Dave,
>>>>>>>>>>>
>>>>>>>>>>> PFA updated patch.
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Jan 8, 2018 at 5:11 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi
>>>>>>>>>>>>
>>>>>>>>>>>> On Fri, Jan 5, 2018 at 8:49 AM, Murtuza Zabuawala <
>>>>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Dave,
>>>>>>>>>>>>>
>>>>>>>>>>>>> PFA updated patch,
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, Jan 3, 2018 at 10:44 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Thu, Dec 28, 2017 at 9:38 AM, Murtuza Zabuawala <
>>>>>>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)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
>>>
>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>