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

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.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
> <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