Thread: Re: [pgAdmin4][Patch]: Using F5 for Query Execution in Query tool
- Execute query --> F5
- Explain query --> F7
- Explain analyze query --> Shift + F7
- Download query result as CSV --> F8
On Wed, Jul 27, 2016 at 2:31 PM, Dave Page <dpage@pgadmin.org> wrote:
>
> Hi
>
> On Tue, Jul 26, 2016 at 7:13 PM, Surinder Kumar
> <surinder.kumar@enterprisedb.com> wrote:
> > Hi Dave
> >
> > I spend some time looking at RM#1412 - Function Keys in Query Tool specific
> > to execute query using F5 key.
>
> Please note that RM1412 was rejected. 1478 is where we're tracking all
> the shortcut issues now.
>
> > As we know F5 key is bound to reload/refresh for browsers on Linux &
> > Windows.
> > But If we use F5 key to execute query, then F5 key event is first captured
> > inside the code and execution works.
>
> OK.
>
> > It will not affect the browser reload behaviour. F5 key for execution will
> > only work if focus is on Query tool.
> >
> > Ashesh's concern is:
> > If focus is not on query tool and user mistakenly pressed Fn+F5 key, the
> > page will reload which shouldn't happened.
> >
> > In such case:
> > The page doesn't reload directly as we always ask user "Whether to Stay on
> > page or Leave page".
>
> Right.
>
> > The other way is to disable reload page on F5 Key only when focus is inside
> > the app.
> > If focus is on browser then F5 for reload will work. (not a good idea)
> >
> > @Dave What do you think?
>
> On Mac it works nicely, both in the runtime and in Chrome, Safari and
> Firefox - and given the number of people complaining about the
> shortcuts here, I think the limitation of focus on the text box is
> acceptable.
>
> > I have tested this attached patch on following platforms without any issue:
> > Windows 7 - Runtime & IE 9 & 10.
> > Mac OSX Yosemite - Safari(8.0.3), Chrome(51) & Firefox(47)
> > Ubuntu-14.04.2 - Chrome & Firefox.
> >
> > Please review the patch.
>
> The patch doesn't update the tooltips/buttons/menus to display the
> correct shortcuts - plus, if we're doing this for F5, then I think we
> should also keep F7 for Explain, Shift+F7 for Explain Analyse and F8
> for Execute to File (CSV Downlaod), like pgAdmin 3.
I will send a patch later today.
>
>
> Please note though that that would only partially resolve RM1478. On
> some browsers/platforms, there is also some CodeMirror related
> shortcut weirdness that needs to be resolved. It may be worth seeing
> if a CodeMirror update would help.
Okay
>
> Thanks!
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
Attachment
Thanks - applied with a couple of fixes: - The menu option for Explain Analyze showed the shortcut as F8, not Shift+F7 - Explain Analyze was broken - the patch checked for Shift+F7 and called on_explain_analyze, but then also checked for F7 and called on_explain. On Wed, Jul 27, 2016 at 6:09 PM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote: > Hi > > Please find attached patch with following fixes: > > 1) This patch adds support for Function Keys(F5, F7, F8) as keyboard > shortcuts. > Following are the keyboard shortcuts for query tool operations such as: > > Execute query --> F5 > Explain query --> F7 > Explain analyze query --> Shift + F7 > Download query result as CSV --> F8 > > 2) Add proper condition to check string in JS array in explain analyze. The > code breaks while running explain analyse. > > This patch partially resolves RM1478. > > Please review. > > On Wed, Jul 27, 2016 at 5:48 PM, Surinder Kumar > <surinder.kumar@enterprisedb.com> wrote: >> >> On Wed, Jul 27, 2016 at 2:31 PM, Dave Page <dpage@pgadmin.org> wrote: >> >> > >> >> > Hi >> > >> > On Tue, Jul 26, 2016 at 7:13 PM, Surinder Kumar >> > <surinder.kumar@enterprisedb.com> wrote: >> > > Hi Dave >> > > >> > > I spend some time looking at RM#1412 - Function Keys in Query Tool >> > > specific >> > > to execute query using F5 key. >> > >> > Please note that RM1412 was rejected. 1478 is where we're tracking all >> > the shortcut issues now. >> > >> > > As we know F5 key is bound to reload/refresh for browsers on Linux & >> > > Windows. >> > > But If we use F5 key to execute query, then F5 key event is first >> > > captured >> > > inside the code and execution works. >> > >> > OK. >> > >> > > It will not affect the browser reload behaviour. F5 key for execution >> > > will >> > > only work if focus is on Query tool. >> > > >> > > Ashesh's concern is: >> > > If focus is not on query tool and user mistakenly pressed Fn+F5 key, >> > > the >> > > page will reload which shouldn't happened. >> > > >> > > In such case: >> > > The page doesn't reload directly as we always ask user "Whether to >> > > Stay on >> > > page or Leave page". >> > >> > Right. >> > >> > > The other way is to disable reload page on F5 Key only when focus is >> > > inside >> > > the app. >> > > If focus is on browser then F5 for reload will work. (not a good idea) >> > > >> > > @Dave What do you think? >> > >> > On Mac it works nicely, both in the runtime and in Chrome, Safari and >> > Firefox - and given the number of people complaining about the >> > shortcuts here, I think the limitation of focus on the text box is >> > acceptable. >> > >> > > I have tested this attached patch on following platforms without any >> > > issue: >> > > Windows 7 - Runtime & IE 9 & 10. >> > > Mac OSX Yosemite - Safari(8.0.3), Chrome(51) & Firefox(47) >> > > Ubuntu-14.04.2 - Chrome & Firefox. >> > > >> > > Please review the patch. >> > >> > The patch doesn't update the tooltips/buttons/menus to display the >> > correct shortcuts - plus, if we're doing this for F5, then I think we >> > should also keep F7 for Explain, Shift+F7 for Explain Analyse and F8 >> > for Execute to File (CSV Downlaod), like pgAdmin 3. > > Tooltips/buttons/menus are updated with correct shortcuts keys in attached > patch. >> >> I will send a patch later today. >> >> >> > >> > >> > Please note though that that would only partially resolve RM1478. On >> > some browsers/platforms, there is also some CodeMirror related >> > shortcut weirdness that needs to be resolved. It may be worth seeing >> > if a CodeMirror update would help. >> Okay >> > >> > Thanks! >> > >> > -- >> > 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