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

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.

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