Sure, I'll do that. On Fri, Jan 12, 2018 at 8:05 PM, Dave Page <dp...@pgadmin.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.zabuawala@ > enterprisedb.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 <dp...@pgadmin.org> wrote: >> >>> >>> >>> On Thu, Jan 11, 2018 at 7:00 AM, Harshal Dhumal < >>> harshal.dhu...@enterprisedb.com> wrote: >>> >>>> On Thu, Jan 11, 2018 at 12:06 PM, Murtuza Zabuawala < >>>> murtuza.zabuaw...@enterprisedb.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.dhu...@enterprisedb.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.zabuaw...@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi Dave, >>>>>>> >>>>>>> PFA updated patch. >>>>>>> >>>>>>> On Tue, Jan 9, 2018 at 7:57 PM, Dave Page <dp...@pgadmin.org> wrote: >>>>>>> >>>>>>>> Hi >>>>>>>> >>>>>>>> On Tue, Jan 9, 2018 at 6:33 AM, Murtuza Zabuawala < >>>>>>>> murtuza.zabuaw...@enterprisedb.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 <dp...@pgadmin.org> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Mon, Jan 8, 2018 at 1:24 PM, Murtuza Zabuawala < >>>>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>>>>>>>> >>>>>>>>>>> Hi Dave, >>>>>>>>>>> >>>>>>>>>>> PFA updated patch. >>>>>>>>>>> >>>>>>>>>>> On Mon, Jan 8, 2018 at 5:11 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi >>>>>>>>>>>> >>>>>>>>>>>> On Fri, Jan 5, 2018 at 8:49 AM, Murtuza Zabuawala < >>>>>>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hi Dave, >>>>>>>>>>>>> >>>>>>>>>>>>> PFA updated patch, >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On Wed, Jan 3, 2018 at 10:44 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Thu, Dec 28, 2017 at 9:38 AM, Murtuza Zabuawala < >>>>>>>>>>>>>> murtuza.zabuaw...@enterprisedb.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 >
