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

Hi Dave,

PFA patch to update docs & screenshots for the new connection status
feature.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Jan 12, 2018 at 8:08 PM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:

> 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(dot)zabuawala(at)enterprisedb(dot)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
>>
>
>