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

From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
To: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, 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-11 06:36:14
Message-ID: CAKKotZRrWpAEvVU0Y6d7FG0ewRCNSCFFygRyrBwuYegc=GgDJQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

User can open Query tool in new browser window where we'll not have
wcDocker panel.

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.zabuawala@
> enterprisedb.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
>>>
>>
>>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Harshal Dhumal 2018-01-11 07:00:10 Re: [pgAdmin4][Patch]: Adding connection status in Query tool
Previous Message Harshal Dhumal 2018-01-11 06:30:57 Re: [pgAdmin4][Patch]: Adding connection status in Query tool