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

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