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

Hi Dave,

On Mon, Jan 15, 2018 at 2:57 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> Unfortunately there's variation in the spec of the images. Can you please
> check with Chethana on the correct sizing and resolution etc? For example,
> query_sql_editor.png was previously 755x209, but now is 2042x402.
>
​Fixed.

PFA updated patch.
​

>
> Thanks!
>
> On Mon, Jan 15, 2018 at 7:21 AM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> PFA patch to update docs & screenshots for the new connection status
>> feature.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Fri, Jan 12, 2018 at 8:08 PM, Murtuza Zabuawala <
>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>
>>> Sure, I'll do that.
>>>
>>> On Fri, Jan 12, 2018 at 8:05 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Thanks - patch applied!
>>>>
>>>> Can you please update the docs for the new config options, and any
>>>> screenshot updates that are required?
>>>>
>>>> On Fri, Jan 12, 2018 at 2:10 PM, Murtuza Zabuawala <
>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> ​Hi Dave,
>>>>>
>>>>> PFA updated patch with additional checks to prevent unnecessary
>>>>> ​polling added as suggested.
>>>>> Please review.
>>>>>
>>>>> --
>>>>> Regards,
>>>>> Murtuza Zabuawala
>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>>
>>>>> On Thu, Jan 11, 2018 at 2:33 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Jan 11, 2018 at 7:00 AM, Harshal Dhumal <
>>>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>
>>>>>> This is a great suggestion (the idea in general). We need to make
>>>>>> this happen - it'll go a long way to minimising our concerns about the
>>>>>> amount of polling.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 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
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> 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
>>>>
>>>
>>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>