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

Thanks, applied.

On Mon, Jan 15, 2018 at 11:35 AM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:

> 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(dot)zabuawala(at)enterprisedb(dot)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
>>
>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company