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

Hi Dave,

Please find updated patch.

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
>