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.zabuaw...@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
