Re: [pgAdmin4][Patch]: Using F5 for Query Execution in Query tool

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(dot)kumar(at)enterprisedb(dot)com> wrote:

> On Wed, Jul 27, 2016 at 2:31 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
> >
>
> > Hi
> >
> > On Tue, Jul 26, 2016 at 7:13 PM, Surinder Kumar
> > <surinder(dot)kumar(at)enterprisedb(dot)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
>