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

From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Chethana Kumar <chethana(dot)kumar(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: Adding connection status in Query tool
Date: 2018-01-10 08:25:13
Message-ID: CAKKotZR3CuZa7bHxeQ3-7OGdvoyJHHA-ng-S0bQ3rraPW67N+w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>

Attachment Content-Type Size
RM_2475_v6.diff text/plain 30.4 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Khushboo Vashi 2018-01-10 09:10:00 [pgAdmin4][Patch]: RM #2993 : - [Web Based] User can not view data for View and MATERIALIZED View object.
Previous Message Murtuza Zabuawala 2018-01-10 07:15:47 [pgAdmin4][Patch#2903] To add ALT tags for images in pgAdmin4 help files